Make use of the newly introduced content type assertion helpers in the preconditions check paths and the call to `vdisk_alloc` in the API handler for migrating VMs (and its subcommands and `mtunnel` endpoint).
Since the preconditions existed before, adding a content type safeguard at `vdisk_alloc` does not break the API. Signed-off-by: Daniel Kral <d.k...@proxmox.com> --- changes since v2: * rename helper from *_supported to *_available * lower/remove storage_check_enabled to storage_config where possible due to the helper already checking that * change of vdisk_alloc signature with required $vtype * add comment about why target_storage_check_available cannot use assert_volume_type_available(...) * change back mock in QmMock to only expect 'images' now PVE/API2/Qemu.pm | 7 +------ PVE/QemuMigrate.pm | 17 ++++++++--------- PVE/QemuServer.pm | 17 ++++++++--------- test/MigrationTest/QmMock.pm | 4 ++-- 4 files changed, 19 insertions(+), 26 deletions(-) diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm index 6931aa18..6a882400 100644 --- a/PVE/API2/Qemu.pm +++ b/PVE/API2/Qemu.pm @@ -263,13 +263,8 @@ my $check_storage_access_clone = sub { 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}; + PVE::Storage::assert_content_type_available($storecfg, $storage, 'images', $node); }; my $import_from_volid = sub { diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm index b7bf2aa3..fca53a45 100644 --- a/PVE/QemuMigrate.pm +++ b/PVE/QemuMigrate.pm @@ -159,15 +159,14 @@ sub target_storage_check_available { my ($self, $storecfg, $targetsid, $volid) = @_; 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}; + # check if storage is available on target node and target supports the volume's content type + # cannot use assert_volume_type_available(), because it checks the volume's current storage + eval { + my ($vtype) = PVE::Storage::parse_volname($storecfg, $volid); + PVE::Storage::assert_content_type_available( + $storecfg, $targetsid, $vtype, $self->{node}); + }; + die "$volid: $@" if $@; } } diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index ed207192..f3ce3892 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -2578,14 +2578,12 @@ sub check_storage_availability { my ($sid, $volname) = PVE::Storage::parse_volume_id($volid, 1); 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, $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 is available on both nodes and target supports the volume's content type + eval { + PVE::Storage::storage_check_enabled($storecfg, $sid); + PVE::Storage::assert_volume_type_available($storecfg, $volid, $node); + }; + die "$volid: $@" if $@; }); } @@ -5529,7 +5527,8 @@ 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, 'any', $size); + my $newvolid = PVE::Storage::vdisk_alloc( + $storecfg, $storeid, $vmid, $format, 'images', $size); my $newdrive = $drive; $newdrive->{format} = $format; $newdrive->{file} = $newvolid; diff --git a/test/MigrationTest/QmMock.pm b/test/MigrationTest/QmMock.pm index dfe06667..bea0c33e 100644 --- a/test/MigrationTest/QmMock.pm +++ b/test/MigrationTest/QmMock.pm @@ -89,8 +89,8 @@ $MigrationTest::Shared::storage_module->mock( die "vdisk_alloc (mocked) - name is not expected to be set - implement me\n" if defined($opts->{name}); - die "vdisk_alloc (mocked) - vtype is expected to be either any or images\n" - if $vtype !~ m/(any|images)/; + die "vdisk_alloc (mocked) - vtype is expected to be images\n" + if $vtype ne 'images'; my $name_without_extension = "vm-${vmid}-disk-${disk_counter}"; $disk_counter++; -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel