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>
This commit is contained in:
Fiona Ebner 2023-07-17 16:00:20 +02:00 committed by Fabian Grünbichler
parent 8056ee3030
commit 0d4e8cbde0
2 changed files with 9 additions and 24 deletions

View File

@ -5654,7 +5654,6 @@ __PACKAGE__->register_method({
'disk' => [ 'disk' => [
undef, undef,
$storeid, $storeid,
undef,
$drive, $drive,
0, 0,
$format, $format,

View File

@ -5566,9 +5566,11 @@ sub vm_migrate_get_nbd_disks {
my $scfg = PVE::Storage::storage_config($storecfg, $storeid); my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
return if $scfg->{shared}; return if $scfg->{shared};
my $format = qemu_img_format($scfg, $volname);
# replicated disks re-use existing state via bitmap # replicated disks re-use existing state via bitmap
my $use_existing = $replicated_volumes->{$volid} ? 1 : 0; 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; return $local_volumes;
} }
@ -5579,7 +5581,7 @@ sub vm_migrate_alloc_nbd_disks {
my $nbd = {}; my $nbd = {};
foreach my $opt (sort keys %$source_volumes) { 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) { if ($use_existing) {
$nbd->{$opt}->{drivestr} = print_drive($drive); $nbd->{$opt}->{drivestr} = print_drive($drive);
@ -5588,29 +5590,13 @@ sub vm_migrate_alloc_nbd_disks {
next; next;
} }
# storage mapping + volname = regular migration $storeid = PVE::JSONSchema::map_id($storagemap, $storeid);
# storage mapping + format = remote migration
# order of precedence, filtered by whether storage supports it: # order of precedence, filtered by whether storage supports it:
# 1. explicit requested format # 1. explicit requested format
# 2. format of current volume # 2. default format of storage
# 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); my ($defFormat, $validFormats) = PVE::Storage::storage_default_format($storecfg, $storeid);
if (!$format || !grep { $format eq $_ } @$validFormats) { $format = $defFormat 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);
}
my $size = $drive->{size} / 1024; my $size = $drive->{size} / 1024;
my $newvolid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $format, undef, $size); my $newvolid = PVE::Storage::vdisk_alloc($storecfg, $storeid, $vmid, $format, undef, $size);