The previously introduced approach can fail for pinned versions when a new QEMU release is introduced. The saner approach is to use a mapping that gives one pve-version for each QEMU release.
Fortunately, the old system has not been bumped yet, so we can still change it without too much effort. QEMU versions without a mapping are assumed to be pve0, 4.1 is mapped to pve1 since thats what we had as our default previously. Pinned machine versions (i.e. pc-i440fx-4.1) are always assumed to be pve0, for specific pve-versions they'd have to be pinned as well (i.e. pc-i440fx-4.1+pve1). The new logic also makes the pve-version dynamic, and starts VMs with the lowest possible 'feature-level', i.e. if a feature is only available with 4.1+pve2, but the VM isn't using it, we still start it with 4.1+pve0. We die if we don't support a version that is requested from us. This allows us to use the pve-version as live-migration blocks (i.e. bumping the version and then live-migrating a VM which uses the new feature (so is running with the bumped version) to an outdated node will present the user with a helpful error message and fail instead of silently modifying the config and only failing *after* the migration). $version_guard is introduced in config_to_command to use for features that need to check pve-version, it automatically handles selecting the newest necessary pve-version for the VM. Tests have to be adjusted, since all of them now resolve to pve0 instead of pve1. EXPECT_ERROR matching is changed to use 'eq' instead of regex to allow special characters in error messages. Signed-off-by: Stefan Reiter <s.rei...@proxmox.com> --- For some reason, thinking this through is astonishingly confusing. My head is spinning with QEMU versions, and I'll probably have nightmares of '+pve0' tonight, but I *think* this should now cover all of our bases? Based on several off-list email and in-person discussions with Thomas and Dominik. Speaking of @Dominik: You'd have to rebase your recent patch [0] to use the new format. Anyway, thourough review appreciated, let's avoid having to change this again. [0] https://pve.proxmox.com/pipermail/pve-devel/2020-February/041588.html PVE/QemuServer.pm | 53 +++++++++++++++---- PVE/QemuServer/Machine.pm | 38 ++++++++++++- test/cfg2cmd/i440fx-win10-hostpci.conf.cmd | 2 +- .../minimal-defaults-to-new-machine.conf | 2 +- ...imal-defaults-unsupported-pve-version.conf | 5 ++ test/cfg2cmd/minimal-defaults.conf.cmd | 2 +- test/cfg2cmd/pinned-version.conf.cmd | 2 +- .../q35-linux-hostpci-multifunction.conf.cmd | 2 +- test/cfg2cmd/q35-linux-hostpci.conf.cmd | 2 +- test/cfg2cmd/q35-win10-hostpci.conf.cmd | 2 +- test/cfg2cmd/spice-linux-4.1.conf.cmd | 2 +- test/run_config2command_tests.pl | 2 +- 12 files changed, 92 insertions(+), 22 deletions(-) create mode 100644 test/cfg2cmd/minimal-defaults-unsupported-pve-version.conf diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index 318ee54..37ffc86 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -3241,14 +3241,24 @@ my $default_machines = { }; sub get_vm_machine { - my ($conf, $forcemachine, $arch, $add_pve_version) = @_; + my ($conf, $forcemachine, $arch, $add_pve_version, $kvmversion) = @_; my $machine = $forcemachine || $conf->{machine}; if (!$machine || $machine =~ m/^(?:pc|q35|virt)$/) { $arch //= 'x86_64'; $machine ||= $default_machines->{$arch}; - $machine .= "+pve$PVE::QemuServer::Machine::PVE_MACHINE_VERSION" if $add_pve_version; + if ($add_pve_version) { + $kvmversion //= kvm_user_version(); + my $pvever = PVE::QemuServer::Machine::get_pve_version($kvmversion); + $machine .= "+pve$pvever"; + } + } + + if ($add_pve_version && $machine !~ m/\+pve\d+$/) { + # for version-pinned machines that do not include a pve-version (e.g. + # pc-q35-4.1), we assume 0 to keep them stable in case we bump + $machine .= '+pve0'; } return $machine; @@ -3422,8 +3432,26 @@ sub config_to_command { $kvm //= 1 if is_native($arch); $machine_version =~ m/(\d+)\.(\d+)/; + my ($machine_major, $machine_minor) = ($1, $2); die "Installed QEMU version '$kvmver' is too old to run machine type '$machine_type', please upgrade node '$nodename'\n" - if !PVE::QemuServer::min_version($kvmver, $1, $2); + if !PVE::QemuServer::min_version($kvmver, $machine_major, $machine_minor); + + if (!PVE::QemuServer::Machine::can_run_pve_machine_version($machine_version, $kvmver)) { + my $max_pve_version = PVE::QemuServer::Machine::get_pve_version($machine_version); + die "Installed qemu-server (max feature level for $machine_major.$machine_minor is pve$max_pve_version)" + . " is too old to run machine type '$machine_type', please upgrade node '$nodename'\n"; + } + + # if a specific +pve version is required for a feature, use $version_guard + # instead of min_version to allow machines to be run with the minimum + # required version + my $required_pve_version = 0; + my $version_guard = sub { + my ($major, $minor, $pve) = @_; + return 0 if !min_version($machine_version, $major, $minor, $pve); + $required_pve_version = $pve if $pve && $pve > $required_pve_version; + return 1; + }; if ($kvm) { die "KVM virtualisation configured, but not available. Either disable in VM configuration or enable in BIOS.\n" @@ -3774,14 +3802,6 @@ sub config_to_command { push @$rtcFlags, 'driftfix=slew' if $tdf; - if (!$kvm) { - push @$machineFlags, 'accel=tcg'; - } - - if ($machine_type) { - push @$machineFlags, "type=${machine_type}"; - } - if (($conf->{startdate}) && ($conf->{startdate} ne 'now')) { push @$rtcFlags, "base=$conf->{startdate}"; } elsif ($useLocaltime) { @@ -4005,6 +4025,17 @@ sub config_to_command { } } + if (!$kvm) { + push @$machineFlags, 'accel=tcg'; + } + + my $machine_type_min = $machine_type; + if ($add_pve_version) { + $machine_type_min =~ s/\+pve\d+$//; + $machine_type_min .= "+pve$required_pve_version"; + } + push @$machineFlags, "type=${machine_type_min}"; + push @$cmd, @$devices; push @$cmd, '-rtc', join(',', @$rtcFlags) if scalar(@$rtcFlags); diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm index 8c13402..9fbe9a8 100644 --- a/PVE/QemuServer/Machine.pm +++ b/PVE/QemuServer/Machine.pm @@ -8,7 +8,9 @@ use PVE::QemuServer::Monitor; # Bump this for VM HW layout changes during a release (where the QEMU machine # version stays the same) -our $PVE_MACHINE_VERSION = 1; +our $PVE_MACHINE_VERSION = { + '4.1' => 1, +}; sub machine_type_is_q35 { my ($conf) = @_; @@ -47,7 +49,8 @@ sub extract_version { return $versionstr; } elsif (defined($kvmversion)) { if ($kvmversion =~ m/^(\d+)\.(\d+)/) { - return "$1.$2+pve$PVE_MACHINE_VERSION"; + my $pvever = get_pve_version($kvmversion); + return "$1.$2+pve$pvever"; } } @@ -61,6 +64,37 @@ sub machine_version { extract_version($machine_type), $major, $minor, $pve); } +sub get_pve_version { + my ($verstr) = @_; + + if ($verstr =~ m/^(\d+\.\d+)/) { + return $PVE_MACHINE_VERSION->{$1} // 0; + } + + die "internal error: cannot get pve version for invalid string '$verstr'"; +} + +sub can_run_pve_machine_version { + my ($machine_version, $kvmversion) = @_; + + $machine_version =~ m/^(\d+)\.(\d+)(?:\+pve(\d+))$/; + my $major = $1; + my $minor = $2; + my $pvever = $3; + + $kvmversion =~ m/(\d+)\.(\d+)/; + return 0 if PVE::QemuServer::Helpers::version_cmp($1, $major, $2, $minor) < 0; + + # if $pvever is missing or 0, we definitely support it as long as we didn't + # fail the QEMU version check above + return 1 if !$pvever; + + my $max_supported = get_pve_version("$major.$minor"); + return 1 if $max_supported >= $pvever; + + return 0; +} + # dies if a) VM not running or not exisiting b) Version query failed # So, any defined return value is valid, any invalid state can be caught by eval sub runs_at_least_qemu_version { diff --git a/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd b/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd index bda7f63..43741ae 100644 --- a/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd +++ b/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd @@ -33,5 +33,5 @@ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300' \ -rtc 'driftfix=slew,base=localtime' \ - -machine 'type=pc+pve1' \ + -machine 'type=pc+pve0' \ -global 'kvm-pit.lost_tick_policy=discard' diff --git a/test/cfg2cmd/minimal-defaults-to-new-machine.conf b/test/cfg2cmd/minimal-defaults-to-new-machine.conf index 5c0486b..35967cf 100644 --- a/test/cfg2cmd/minimal-defaults-to-new-machine.conf +++ b/test/cfg2cmd/minimal-defaults-to-new-machine.conf @@ -1,5 +1,5 @@ # TEST: newer machine verison than QEMU version installed on node # QEMU_VERSION: 4.1.1 -# EXPECT_ERROR: Installed QEMU version '4.1.1' is too old to run machine type 'pc-q35-42.9', please upgrade node 'localhost' +# EXPECT_ERROR: Installed QEMU version '4.1.1' is too old to run machine type 'pc-q35-42.9+pve0', please upgrade node 'localhost' smbios1: uuid=6cf17dc3-8341-4ecc-aebd-7503f2583fb3 machine: pc-q35-42.9 diff --git a/test/cfg2cmd/minimal-defaults-unsupported-pve-version.conf b/test/cfg2cmd/minimal-defaults-unsupported-pve-version.conf new file mode 100644 index 0000000..87e7a07 --- /dev/null +++ b/test/cfg2cmd/minimal-defaults-unsupported-pve-version.conf @@ -0,0 +1,5 @@ +# TEST: newer machine verison than QEMU version installed on node +# QEMU_VERSION: 4.1.1 +# EXPECT_ERROR: Installed qemu-server (max feature level for 4.0 is pve0) is too old to run machine type 'pc-q35-4.0+pve77', please upgrade node 'localhost' +smbios1: uuid=6cf17dc3-8341-4ecc-aebd-7503f2583fb3 +machine: pc-q35-4.0+pve77 diff --git a/test/cfg2cmd/minimal-defaults.conf.cmd b/test/cfg2cmd/minimal-defaults.conf.cmd index 83ae328..c499a85 100644 --- a/test/cfg2cmd/minimal-defaults.conf.cmd +++ b/test/cfg2cmd/minimal-defaults.conf.cmd @@ -21,4 +21,4 @@ -device 'VGA,id=vga,bus=pci.0,addr=0x2' \ -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3' \ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \ - -machine 'type=pc+pve1' + -machine 'type=pc+pve0' diff --git a/test/cfg2cmd/pinned-version.conf.cmd b/test/cfg2cmd/pinned-version.conf.cmd index abfbea0..2fc31fc 100644 --- a/test/cfg2cmd/pinned-version.conf.cmd +++ b/test/cfg2cmd/pinned-version.conf.cmd @@ -27,4 +27,4 @@ -device 'scsi-hd,bus=scsihw0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,bootindex=100' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=A2:C0:43:77:08:A1,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300' \ - -machine 'type=pc-q35-3.1' + -machine 'type=pc-q35-3.1+pve0' diff --git a/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd b/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd index e20be7d..c766d33 100644 --- a/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd +++ b/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd @@ -31,4 +31,4 @@ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300' \ - -machine 'type=q35+pve1' + -machine 'type=q35+pve0' diff --git a/test/cfg2cmd/q35-linux-hostpci.conf.cmd b/test/cfg2cmd/q35-linux-hostpci.conf.cmd index 152624c..7059b29 100644 --- a/test/cfg2cmd/q35-linux-hostpci.conf.cmd +++ b/test/cfg2cmd/q35-linux-hostpci.conf.cmd @@ -36,4 +36,4 @@ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300' \ - -machine 'type=q35+pve1' + -machine 'type=q35+pve0' diff --git a/test/cfg2cmd/q35-win10-hostpci.conf.cmd b/test/cfg2cmd/q35-win10-hostpci.conf.cmd index ff799ea..3397324 100644 --- a/test/cfg2cmd/q35-win10-hostpci.conf.cmd +++ b/test/cfg2cmd/q35-win10-hostpci.conf.cmd @@ -34,5 +34,5 @@ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=2E:01:68:F9:9C:87,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300' \ -rtc 'driftfix=slew,base=localtime' \ - -machine 'type=q35+pve1' \ + -machine 'type=q35+pve0' \ -global 'kvm-pit.lost_tick_policy=discard' diff --git a/test/cfg2cmd/spice-linux-4.1.conf.cmd b/test/cfg2cmd/spice-linux-4.1.conf.cmd index 4ed6fd2..6aa781b 100644 --- a/test/cfg2cmd/spice-linux-4.1.conf.cmd +++ b/test/cfg2cmd/spice-linux-4.1.conf.cmd @@ -27,4 +27,4 @@ -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \ -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \ -device 'virtio-net-pci,mac=A2:C0:43:67:08:A1,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300' \ - -machine 'type=pc+pve1' + -machine 'type=pc+pve0' diff --git a/test/run_config2command_tests.pl b/test/run_config2command_tests.pl index bad6501..c3e5092 100755 --- a/test/run_config2command_tests.pl +++ b/test/run_config2command_tests.pl @@ -292,7 +292,7 @@ sub do_test($) { return; } chomp $err; - if ($err !~ /^\s*$err_expect\s*$/) { + if ($err ne $err_expect) { fail("$testname"); note("error does not match expected error: '$err' !~ '$err_expect'"); } else { -- 2.20.1 _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
