From: Fiona Ebner <f.ebner@proxmox.com>
Date: Mon, 17 Jul 2023 14:00:20 +0000 (+0200)
Subject: migration: alloc nbd disks: fix fall-back for remote live migration
X-Git-Url: https://git.proxmox.com/?p=qemu-server.git;a=commitdiff_plain;h=0d4e8cbde08dd272bcf91318fe67845a58712bec

migration: alloc nbd disks: fix fall-back for remote live migration

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>
---

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 1cff80b..9606e72 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -5654,7 +5654,6 @@ __PACKAGE__->register_method({
 			'disk' => [
 			    undef,
 			    $storeid,
-			    undef,
 			    $drive,
 			    0,
 			    $format,
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 48f9845..1bebe44 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5566,9 +5566,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;
 }
@@ -5579,7 +5581,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);
@@ -5588,29 +5590,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);