From 9f887d37382f3c8b21fa34aff3e48d28d33673e7 Mon Sep 17 00:00:00 2001 From: Fiona Ebner Date: Fri, 6 Dec 2024 17:25:31 +0100 Subject: [PATCH] cfg2cmd: print drive commandline: improve format detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For a Proxmox VE managed volume, prefer the format from the storage layer rather than the 'format' option set on the drive. Fail if there is a mismatch between the detected and configured format, because this is not expected for managed volumes. Having this early hard failure protects against undesirable issues with live migration and reboot where the format of a drive would suddenly be different. For a not Proxmox VE managed volume, use the same logic as before, i.e. use the 'format' option for the drive with 'raw' as a fallback: Only root can configure such devices. Both also apply to the case where the 'cdrom' flag is set to avoid autodetection by QEMU. Reported-by: Friedrich Weber Signed-off-by: Fiona Ebner FG: typo fix in comment Signed-off-by: Fabian Grünbichler --- PVE/QemuServer.pm | 23 +++++++++++++++++++---- test/cfg2cmd/ide.conf.cmd | 8 ++++---- test/cfg2cmd/q35-ide.conf.cmd | 8 ++++---- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index f4958d3d..9c8078b2 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -55,7 +55,7 @@ use PVE::QemuServer::Helpers qw(config_aware_timeout min_version windows_version use PVE::QemuServer::Cloudinit; use PVE::QemuServer::CGroup; use PVE::QemuServer::CPUConfig qw(print_cpu_device get_cpu_options get_cpu_bitness is_native_arch get_amd_sev_object); -use PVE::QemuServer::Drive qw(is_valid_drivename drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive); +use PVE::QemuServer::Drive qw(is_valid_drivename checked_volume_format drive_is_cloudinit drive_is_cdrom drive_is_read_only parse_drive print_drive); use PVE::QemuServer::Machine; use PVE::QemuServer::Memory qw(get_current_memory); use PVE::QemuServer::Monitor qw(mon_cmd); @@ -1544,7 +1544,6 @@ sub print_drive_commandline_full { my $path; my $volid = $drive->{file}; - my $format = $drive->{format}; my $drive_id = get_drive_id($drive); my ($storeid, $volname) = PVE::Storage::parse_volume_id($volid, 1); @@ -1556,13 +1555,29 @@ sub print_drive_commandline_full { } else { if ($storeid) { $path = PVE::Storage::path($storecfg, $volid); - $format //= qemu_img_format($scfg, $volname); } else { $path = $volid; - $format //= "raw"; } } + # For PVE-managed volumes, use the format from the storage layer and prevent overrides via the + # drive's 'format' option. For unmanaged volumes, fallback to 'raw' to avoid auto-detection by + # QEMU. For the special case 'none' (get_iso_path() returns an empty $path), there should be no + # format or QEMU won't start. + my $format; + if (drive_is_cdrom($drive) && !$path) { + # no format + } elsif ($storeid) { + $format = checked_volume_format($storecfg, $volid); + + if ($drive->{format} && $drive->{format} ne $format) { + die "drive '$drive->{interface}$drive->{index}' - volume '$volid'" + ." - 'format=$drive->{format}' option different from storage format '$format'\n"; + } + } else { + $format = $drive->{format} // 'raw'; + } + my $is_rbd = $path =~ m/^rbd:/; my $opts = ''; diff --git a/test/cfg2cmd/ide.conf.cmd b/test/cfg2cmd/ide.conf.cmd index 7fd4888c..33c6aadc 100644 --- a/test/cfg2cmd/ide.conf.cmd +++ b/test/cfg2cmd/ide.conf.cmd @@ -23,13 +23,13 @@ -device 'VGA,id=vga,bus=pci.0,addr=0x2' \ -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3,free-page-reporting=on' \ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \ - -drive 'file=/mnt/pve/cifs-store/template/iso/zero.iso,if=none,id=drive-ide0,media=cdrom,aio=threads' \ + -drive 'file=/mnt/pve/cifs-store/template/iso/zero.iso,if=none,id=drive-ide0,media=cdrom,format=raw,aio=threads' \ -device 'ide-cd,bus=ide.0,unit=0,drive=drive-ide0,id=ide0,bootindex=200' \ - -drive 'file=/mnt/pve/cifs-store/template/iso/one.iso,if=none,id=drive-ide1,media=cdrom,aio=threads' \ + -drive 'file=/mnt/pve/cifs-store/template/iso/one.iso,if=none,id=drive-ide1,media=cdrom,format=raw,aio=threads' \ -device 'ide-cd,bus=ide.0,unit=1,drive=drive-ide1,id=ide1,bootindex=201' \ - -drive 'file=/mnt/pve/cifs-store/template/iso/two.iso,if=none,id=drive-ide2,media=cdrom,aio=threads' \ + -drive 'file=/mnt/pve/cifs-store/template/iso/two.iso,if=none,id=drive-ide2,media=cdrom,format=raw,aio=threads' \ -device 'ide-cd,bus=ide.1,unit=0,drive=drive-ide2,id=ide2,bootindex=202' \ - -drive 'file=/mnt/pve/cifs-store/template/iso/three.iso,if=none,id=drive-ide3,media=cdrom,aio=threads' \ + -drive 'file=/mnt/pve/cifs-store/template/iso/three.iso,if=none,id=drive-ide3,media=cdrom,format=raw,aio=threads' \ -device 'ide-cd,bus=ide.1,unit=1,drive=drive-ide3,id=ide3,bootindex=203' \ -device 'virtio-scsi-pci,id=scsihw0,bus=pci.0,addr=0x5' \ -drive 'file=/var/lib/vz/images/100/vm-100-disk-2.qcow2,if=none,id=drive-scsi0,format=qcow2,cache=none,aio=io_uring,detect-zeroes=on' \ diff --git a/test/cfg2cmd/q35-ide.conf.cmd b/test/cfg2cmd/q35-ide.conf.cmd index 20ccc98b..dd4f1bbe 100644 --- a/test/cfg2cmd/q35-ide.conf.cmd +++ b/test/cfg2cmd/q35-ide.conf.cmd @@ -22,13 +22,13 @@ -device 'VGA,id=vga,bus=pcie.0,addr=0x1' \ -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3,free-page-reporting=on' \ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \ - -drive 'file=/mnt/pve/cifs-store/template/iso/zero.iso,if=none,id=drive-ide0,media=cdrom,aio=threads' \ + -drive 'file=/mnt/pve/cifs-store/template/iso/zero.iso,if=none,id=drive-ide0,media=cdrom,format=raw,aio=threads' \ -device 'ide-cd,bus=ide.0,unit=0,drive=drive-ide0,id=ide0,bootindex=200' \ - -drive 'file=/mnt/pve/cifs-store/template/iso/one.iso,if=none,id=drive-ide1,media=cdrom,aio=threads' \ + -drive 'file=/mnt/pve/cifs-store/template/iso/one.iso,if=none,id=drive-ide1,media=cdrom,format=raw,aio=threads' \ -device 'ide-cd,bus=ide.2,unit=0,drive=drive-ide1,id=ide1,bootindex=201' \ - -drive 'file=/mnt/pve/cifs-store/template/iso/two.iso,if=none,id=drive-ide2,media=cdrom,aio=threads' \ + -drive 'file=/mnt/pve/cifs-store/template/iso/two.iso,if=none,id=drive-ide2,media=cdrom,format=raw,aio=threads' \ -device 'ide-cd,bus=ide.1,unit=0,drive=drive-ide2,id=ide2,bootindex=202' \ - -drive 'file=/mnt/pve/cifs-store/template/iso/three.iso,if=none,id=drive-ide3,media=cdrom,aio=threads' \ + -drive 'file=/mnt/pve/cifs-store/template/iso/three.iso,if=none,id=drive-ide3,media=cdrom,format=raw,aio=threads' \ -device 'ide-cd,bus=ide.3,unit=0,drive=drive-ide3,id=ide3,bootindex=203' \ -device 'virtio-scsi-pci,id=scsihw0,bus=pci.0,addr=0x5' \ -drive 'file=/var/lib/vz/images/100/vm-100-disk-2.qcow2,if=none,id=drive-scsi0,format=qcow2,cache=none,aio=io_uring,detect-zeroes=on' \