From df951c01ebd89bac53d50fa62fbc6a65d3914d40 Mon Sep 17 00:00:00 2001 From: Michal Fox Date: Thu, 8 Oct 2026 09:46:50 +0000 Subject: [PATCH] partially fix #4012: migrate: mention snapshots referencing a local ISO image When a VM has no local ISO image attached anymore, but one of its snapshots still references it, migration fails with: can't migrate local disk 'local:iso/debian.iso': local cdrom image which does not tell where the image is coming from, since it is not visible in the current configuration of the VM. Add the names of the snapshots referencing the image to the error in that case, like it is already done for a physical CD/DVD drive. The check for the physical drive now reuses the same hint, which also uses the plural form if more than one snapshot references it. Add a migration test with an ISO image on the 'local' storage that is only referenced in a snapshot. This requires the 'iso' content type for the 'local' storage in the test. Signed-off-by: Michal Fox Link: https://lore.proxmox.com/20261008094650.7-3-me@dualfroz.com [FE: note that it's a partial fix as the UI still needs to be improved] Signed-off-by: Fiona Ebner --- src/PVE/QemuMigrate.pm | 15 ++++----- src/test/run_qemu_migrate_tests.pl | 49 +++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm index 8da6f15..6605f5d 100644 --- a/src/PVE/QemuMigrate.pm +++ b/src/PVE/QemuMigrate.pm @@ -412,15 +412,16 @@ sub scan_local_volumes { } my $snaprefs = $attr->{referenced_in_snapshot}; + my $snapshot_hint = ''; + if (defined($snaprefs) && !$attr->{is_attached}) { + my $snapnames = join(', ', sort keys %$snaprefs); + my $plural = scalar(keys %$snaprefs) > 1 ? 's' : ''; + $snapshot_hint = " (referenced in snapshot$plural - $snapnames)"; + } if ($attr->{cdrom}) { if ($volid eq 'cdrom') { - my $msg = "can't migrate local cdrom drive"; - if (defined($snaprefs) && !$attr->{is_attached}) { - my $snapnames = join(', ', sort keys %$snaprefs); - $msg .= " (referenced in snapshot - $snapnames)"; - } - &$log_error("$msg\n"); + &$log_error("can't migrate local cdrom drive$snapshot_hint\n"); return; } return if $volid eq 'none'; @@ -475,7 +476,7 @@ sub scan_local_volumes { $local_volumes->{$volid}->{ref} = 'generated'; return; } - die "local cdrom image\n"; + die "local cdrom image$snapshot_hint\n"; } my ($path, $owner) = PVE::Storage::path($storecfg, $volid); diff --git a/src/test/run_qemu_migrate_tests.pl b/src/test/run_qemu_migrate_tests.pl index 9f4cef6..fc2f137 100644 --- a/src/test/run_qemu_migrate_tests.pl +++ b/src/test/run_qemu_migrate_tests.pl @@ -37,6 +37,7 @@ my $storage_config = { local => { content => { images => 1, + iso => 1, }, path => "/var/lib/vz", type => "dir", @@ -438,7 +439,18 @@ my $source_vdisks = { # Non-image volumes on the source, e.g. ISO images, with the same structure as $source_vdisks. They # are only used for the mocked volume_size_info(), because vdisk_list() only returns images. -my $source_other_volumes = {}; +my $source_other_volumes = { + 'local' => [ + { + 'ctime' => 1589439681, + 'format' => 'iso', + 'parent' => undef, + 'size' => 663748608, + 'used' => 663748608, + 'volid' => 'local:iso/debian.iso', + }, + ], +}; my $default_expected_calls_online = { move_config_to_node => 1, @@ -1240,6 +1252,41 @@ my $tests = [ }, }, }, + { + name => '105_local_iso_in_snapshot', + target => 'pve1', + vmid => 105, + vm_status => { + running => 0, + }, + config_patch => { + snapshots => { + ohsnap => { + ide2 => 'local:iso/debian.iso,media=cdrom', + }, + }, + }, + expected_calls => {}, + expect_die => "can't migrate local disk 'local:iso/debian.iso': local cdrom image" + . " (referenced in snapshot - ohsnap)", + expected => { + source_volids => local_volids_for_vm(105), + target_volids => {}, + vm_config => get_patched_config( + 105, + { + snapshots => { + ohsnap => { + ide2 => 'local:iso/debian.iso,media=cdrom', + }, + }, + }, + ), + vm_status => { + running => 0, + }, + }, + }, { name => '105_cdrom', target => 'pve1',