[pve-devel] [PATCH qemu-server 1/2] Use 'QEMU version' -> '+pve-version' mapping for machine types

Stefan Reiter s.reiter at proxmox.com
Mon Feb 10 16:05:35 CET 2020


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.reiter at 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




More information about the pve-devel mailing list