From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id D386C1FF16B for ; Mon, 16 Sep 2024 18:39:41 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5BBB4BA71; Mon, 16 Sep 2024 18:39:18 +0200 (CEST) From: Daniel Kral To: pve-devel@lists.proxmox.com Date: Mon, 16 Sep 2024 18:38:37 +0200 Message-Id: <20240916163839.236908-8-d.kral@proxmox.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20240916163839.236908-1-d.kral@proxmox.com> References: <20240916163839.236908-1-d.kral@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.002 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [RFC qemu-server 7/9] api: migrate_vm: improve check if target storages support vm images X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" 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 --- 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