From: Daniel Kral <d.kral@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [RFC qemu-server 7/9] api: migrate_vm: improve check if target storages support vm images
Date: Mon, 16 Sep 2024 18:38:37 +0200 [thread overview]
Message-ID: <20240916163839.236908-8-d.kral@proxmox.com> (raw)
In-Reply-To: <20240916163839.236908-1-d.kral@proxmox.com>
Improve the checks when migrating a VM if the target storage supports
the content type 'images' by refactoring it to use `check_storage_alloc`
and `check_volume_alloc` and adding parameter context to the error
message, where it is sensible to do so.
This doesn't change a lot in functionality, except for the allocation of
NBD disks while migrating, which will now do a check if the storage is
enabled and supports vm images before allocating the volume disk image.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
I have only tested migration and remote migration on a surface level for
now, but will follow up with more thorough testing the internal
`mtunnel` API endpoint and setup `qemu-nbd` to test the NBD disk
allocation, as I haven't worked with them before.
PVE/API2/Qemu.pm | 39 +++++++++++++++------------------------
PVE/QemuMigrate.pm | 13 +++++--------
PVE/QemuServer.pm | 11 +++++------
3 files changed, 25 insertions(+), 38 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index a0abe44f..13c1c4e0 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -220,18 +220,6 @@ my $check_storage_access_clone = sub {
return $sharedvm;
};
-my $check_storage_access_migrate = sub {
- my ($rpcenv, $authuser, $storecfg, $storage, $node) = @_;
-
- PVE::Storage::storage_check_enabled($storecfg, $storage, $node);
-
- $rpcenv->check($authuser, "/storage/$storage", ['Datastore.AllocateSpace']);
-
- my $scfg = PVE::Storage::storage_config($storecfg, $storage);
- die "storage '$storage' does not support vm images\n"
- if !$scfg->{content}->{images};
-};
-
my $import_from_volid = sub {
my ($storecfg, $src_volid, $dest_info, $vollist) = @_;
@@ -4697,11 +4685,14 @@ __PACKAGE__->register_method({
if !defined($storagemap->{identity});
foreach my $target_sid (values %{$storagemap->{entries}}) {
- $check_storage_access_migrate->($rpcenv, $authuser, $storecfg, $target_sid, $target);
+ check_storage_alloc($rpcenv, $authuser, $target_sid);
+ check_volume_alloc($storecfg, $target_sid, $target);
}
- $check_storage_access_migrate->($rpcenv, $authuser, $storecfg, $storagemap->{default}, $target)
- if $storagemap->{default};
+ if ($storagemap->{default}) {
+ check_storage_alloc($rpcenv, $authuser, $storagemap->{default});
+ check_volume_alloc($storecfg, $storagemap->{default}, $target);
+ }
PVE::QemuServer::check_storage_availability($storecfg, $conf, $target)
if $storagemap->{identity};
@@ -5652,7 +5643,9 @@ __PACKAGE__->register_method({
my $storecfg = PVE::Storage::config();
foreach my $storeid (PVE::Tools::split_list($storages)) {
- $check_storage_access_migrate->($rpcenv, $authuser, $storecfg, $storeid, $node);
+ check_storage_alloc($rpcenv, $authuser, $storeid);
+ eval { check_volume_alloc($storecfg, $storeid, $node) };
+ raise_param_exc({ storages => "$@" }) if $@;
}
foreach my $bridge (PVE::Tools::split_list($bridges)) {
@@ -5829,7 +5822,9 @@ __PACKAGE__->register_method({
my $storeid = $params->{storage};
my $drive = $params->{drive};
- $check_storage_access_migrate->($rpcenv, $authuser, $state->{storecfg}, $storeid, $node);
+ check_storage_alloc($rpcenv, $authuser, $storeid);
+ eval { check_volume_alloc($state->{storecfg}, $storeid, $node) };
+ raise_param_exc({ storage => "$@" }) if $@;
my $storagemap = {
default => $storeid,
@@ -5856,13 +5851,9 @@ __PACKAGE__->register_method({
'disk-import' => sub {
my ($params) = @_;
- $check_storage_access_migrate->(
- $rpcenv,
- $authuser,
- $state->{storecfg},
- $params->{storage},
- $node
- );
+ check_storage_alloc($rpcenv, $authuser, $params->{storage});
+ eval { check_volume_alloc($state->{storecfg}, $params->{storage}, $node) };
+ raise_param_exc({ storage => "$@" }) if $@;
$params->{unix} = "/run/qemu-server/$state->{vmid}.storage";
diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 6591f3f7..0cc582eb 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -159,14 +159,11 @@ sub target_storage_check_available {
if (!$self->{opts}->{remote}) {
# check if storage is available on target node
- my $target_scfg = PVE::Storage::storage_check_enabled(
- $storecfg,
- $targetsid,
- $self->{node},
- );
- my ($vtype) = PVE::Storage::parse_volname($storecfg, $volid);
- die "$volid: content type '$vtype' is not available on storage '$targetsid'\n"
- if !$target_scfg->{content}->{$vtype};
+ PVE::Storage::storage_check_enabled($storecfg, $targetsid, $self->{node});
+
+ # check if storage supports the volume's content type
+ eval { PVE::QemuServer::Helpers::check_volume_content_type($storecfg, $volid) };
+ die "$volid: $@" if $@;
}
}
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index b40f6f6e..c07dd7aa 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2630,13 +2630,12 @@ sub check_storage_availability {
return if !$sid;
# check if storage is available on both nodes
- my $scfg = PVE::Storage::storage_check_enabled($storecfg, $sid);
+ PVE::Storage::storage_check_enabled($storecfg, $sid);
PVE::Storage::storage_check_enabled($storecfg, $sid, $node);
- my ($vtype) = PVE::Storage::parse_volname($storecfg, $volid);
-
- die "$volid: content type '$vtype' is not available on storage '$sid'\n"
- if !$scfg->{content}->{$vtype};
+ # check if storage supports the volume's content type
+ eval { PVE::QemuServer::Helpers::check_volume_content_type($storecfg, $volid) };
+ die "$volid: $@" if $@;
});
}
@@ -5590,7 +5589,7 @@ sub vm_migrate_alloc_nbd_disks {
$format = $defFormat if !$format || !grep { $format eq $_ } $validFormats->@*;
my $size = $drive->{size} / 1024;
- my $newvolid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $format, undef, $size);
+ my $newvolid = alloc_volume_disk($storecfg, $storeid, $vmid, $format, undef, $size);
my $newdrive = $drive;
$newdrive->{format} = $format;
$newdrive->{file} = $newvolid;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
next prev parent reply other threads:[~2024-09-16 16:39 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-16 16:38 [pve-devel] [RFC qemu-server 0/9] consistent checks for storage content types on volume disk allocation Daniel Kral
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 1/9] test: cfg2cmd: expect error for invalid volume's storage content type Daniel Kral
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 2/9] cfg2cmd: improve error message for invalid volume " Daniel Kral
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 3/9] fix #5284: move_vm: add check if target storage supports vm images Daniel Kral
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 4/9] api: clone_vm: add check if " Daniel Kral
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 5/9] api: create_vm: improve checks if storages for disks support " Daniel Kral
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 6/9] cloudinit: add check if storage for cloudinit disk supports " Daniel Kral
2024-09-16 16:38 ` Daniel Kral [this message]
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 8/9] api: importdisk: improve check if storage " Daniel Kral
2024-09-16 16:38 ` [pve-devel] [RFC qemu-server 9/9] restore_vm: improve checks " Daniel Kral
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240916163839.236908-8-d.kral@proxmox.com \
--to=d.kral@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox