* [PATCH qemu-server] fix #4012: migrate: mention snapshots referencing a local ISO image
@ 2026-10-06 8:52 Michal Fox
2026-10-06 10:00 ` Fiona Ebner
0 siblings, 1 reply; 2+ messages in thread
From: Michal Fox @ 2026-10-06 8:52 UTC (permalink / raw)
To: pve-devel
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.
Signed-off-by: Michal Fox <me@dualfroz.com>
---
Tested with a new migration test for an ISO image on local storage
that is only referenced in a snapshot. Without the fix, the error does
not contain the snapshot name. The whole qemu-server test suite passes.
The test needed the ISO in the mocked volume list and 'iso' content on
the mocked 'local' storage.
The GUI still only shows 'Cannot migrate VM with local CD/DVD' in that
case, which could be done as a follow-up in pve-manager.
src/PVE/QemuMigrate.pm | 14 ++++-----
src/test/run_qemu_migrate_tests.pl | 47 ++++++++++++++++++++++++++++++
2 files changed, 54 insertions(+), 7 deletions(-)
diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
index 8da6f15d..a9b69898 100644
--- a/src/PVE/QemuMigrate.pm
+++ b/src/PVE/QemuMigrate.pm
@@ -412,15 +412,15 @@ 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);
+ $snapshot_hint = " (referenced in snapshot - $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 +475,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 05eed1d9..49248ed9 100755
--- 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",
@@ -346,6 +347,17 @@ my $source_vdisks = {
'volid' => 'local-dir:4567/vm-4567-state-snap2.raw',
},
],
+ 'local' => [
+ {
+ 'ctime' => 1589439681,
+ 'format' => 'iso',
+ 'parent' => undef,
+ 'size' => 663748608,
+ 'used' => 663748608,
+ 'vmid' => '0',
+ 'volid' => 'local:iso/debian.iso',
+ },
+ ],
'local-lvm' => [
{
'ctime' => '1589277334',
@@ -1236,6 +1248,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',
--
2.43.0
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH qemu-server] fix #4012: migrate: mention snapshots referencing a local ISO image
2026-10-06 8:52 [PATCH qemu-server] fix #4012: migrate: mention snapshots referencing a local ISO image Michal Fox
@ 2026-10-06 10:00 ` Fiona Ebner
0 siblings, 0 replies; 2+ messages in thread
From: Fiona Ebner @ 2026-10-06 10:00 UTC (permalink / raw)
To: Michal Fox, pve-devel
Hi Michal,
thank you for the contribution!
Am 06.10.26 um 10:53 AM schrieb Michal Fox:
> 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.
>
> Signed-off-by: Michal Fox <me@dualfroz.com>
> ---
> Tested with a new migration test for an ISO image on local storage
> that is only referenced in a snapshot. Without the fix, the error does
> not contain the snapshot name. The whole qemu-server test suite passes.
>
> The test needed the ISO in the mocked volume list and 'iso' content on
> the mocked 'local' storage.
>
> The GUI still only shows 'Cannot migrate VM with local CD/DVD' in that
> case, which could be done as a follow-up in pve-manager.
An additional UI patch would be nice.
>
> src/PVE/QemuMigrate.pm | 14 ++++-----
> src/test/run_qemu_migrate_tests.pl | 47 ++++++++++++++++++++++++++++++
> 2 files changed, 54 insertions(+), 7 deletions(-)
>
> diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
> index 8da6f15d..a9b69898 100644
> --- a/src/PVE/QemuMigrate.pm
> +++ b/src/PVE/QemuMigrate.pm
> @@ -412,15 +412,15 @@ 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);
> + $snapshot_hint = " (referenced in snapshot - $snapnames)";
Nit: Pre-existing, but while we're at it, I'd suggest "snapshot(s)", or
conditionally add the 's' depending on the count.
> + }
>
> 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 +475,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 05eed1d9..49248ed9 100755
> --- 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",
> @@ -346,6 +347,17 @@ my $source_vdisks = {
> 'volid' => 'local-dir:4567/vm-4567-state-snap2.raw',
> },
> ],
> + 'local' => [
> + {
> + 'ctime' => 1589439681,
> + 'format' => 'iso',
> + 'parent' => undef,
> + 'size' => 663748608,
> + 'used' => 663748608,
> + 'vmid' => '0',
Nit: I'd prefer to keep vmid undef here or even better, not specified at
all.
> + 'volid' => 'local:iso/debian.iso',
> + },
> + ],
I'm not sure we should add this to the $source_vdisks, because that is
used for mocking vdisk_list(), which only returns images and not ISOs.
A clean way would probably be to add another file and consider its
contents in the mocked volume_size_info() call in QemuMigrateMock.pm
Or we could rename it to $source_volumes and have non-images carry an
explicit 'vtype' property and have the mocked vdisk_list() filter out
non-images. Images won't need to specify 'vtype' explicitly.
Either way would be nice to have in a separate, preparatory patch.
> 'local-lvm' => [
> {
> 'ctime' => '1589277334',
> @@ -1236,6 +1248,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)",
Style nit: line too long
> + 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',
Best Regards,
Fiona
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-06 10:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-06 8:52 [PATCH qemu-server] fix #4012: migrate: mention snapshots referencing a local ISO image Michal Fox
2026-10-06 10:00 ` Fiona Ebner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox