[pve-devel] [PATCH qemu-server 2/2] migration: alloc nbd disks: fix fall-back for remote live migration

[Fiona Ebner <f.ebner@proxmox.com>]

While the comment sated
>    # order of precedence, filtered by whether storage supports it:
>    # 1. explicit requested format
>    # 2. format of current volume
>    # 3. default format of storage

the code did not fall back to the default format in the case of remote
migration, because the format was already set and the code used
> $format //= $defFormat;

This made remote migration from dir with qcow2 to e.g. LVM-thin fail.

Move extracting the format from the volume name to the call side for
local migration. This allows the logic here to be much simpler.

Reported-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Qemu.pm  |  1 -
 PVE/QemuServer.pm | 32 +++++++++-----------------------
 2 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 59307133..9c3f7216 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -5634,7 +5634,6 @@ __PACKAGE__->register_method({
 			'disk' => [
 			    undef,
 			    $storeid,
-			    undef,
 			    $drive,
 			    0,
 			    $format,
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index a692e32e..4767bad4 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5554,9 +5554,11 @@ sub vm_migrate_get_nbd_disks {
 	my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
 	return if $scfg->{shared};
 
+	my $format = qemu_img_format($scfg, $volname);
+
 	# replicated disks re-use existing state via bitmap
 	my $use_existing = $replicated_volumes->{$volid} ? 1 : 0;
-	$local_volumes->{$ds} = [$volid, $storeid, $volname, $drive, $use_existing];
+	$local_volumes->{$ds} = [$volid, $storeid, $drive, $use_existing, $format];
     });
     return $local_volumes;
 }
@@ -5567,7 +5569,7 @@ sub vm_migrate_alloc_nbd_disks {
 
     my $nbd = {};
     foreach my $opt (sort keys %$source_volumes) {
-	my ($volid, $storeid, $volname, $drive, $use_existing, $format) = @{$source_volumes->{$opt}};
+	my ($volid, $storeid, $drive, $use_existing, $format) = @{$source_volumes->{$opt}};
 
 	if ($use_existing) {
 	    $nbd->{$opt}->{drivestr} = print_drive($drive);
@@ -5576,29 +5578,13 @@ sub vm_migrate_alloc_nbd_disks {
 	    next;
 	}
 
-	# storage mapping + volname = regular migration
-	# storage mapping + format = remote migration
+	$storeid = PVE::JSONSchema::map_id($storagemap, $storeid);
+
 	# order of precedence, filtered by whether storage supports it:
 	# 1. explicit requested format
-	# 2. format of current volume
-	# 3. default format of storage
-	if (!$storagemap->{identity}) {
-	    my $source_scfg = PVE::Storage::storage_config($storecfg, $storeid);
-	    $storeid = PVE::JSONSchema::map_id($storagemap, $storeid);
-	    my ($defFormat, $validFormats) = PVE::Storage::storage_default_format($storecfg, $storeid);
-	    if (!$format || !grep { $format eq $_ } @$validFormats) {
-		if ($volname) {
-		    my $fileFormat = qemu_img_format($source_scfg, $volname);
-		    $format = $fileFormat
-			if grep { $fileFormat eq $_ } @$validFormats;
-		}
-		$format //= $defFormat;
-	    }
-	} else {
-	    # can't happen for remote migration, so $volname is always defined
-	    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
-	    $format = qemu_img_format($scfg, $volname);
-	}
+	# 2. default format of storage
+	my ($defFormat, $validFormats) = PVE::Storage::storage_default_format($storecfg, $storeid);
+	$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);
-- 
2.39.2