Approach is correct IMO, probably the easiest way, and I agree that it
seems unlikely that QEMU's patch version will be important in the future.
Some stuff inline.
On 11/25/19 11:27 AM, Thomas Lamprecht wrote:
With our QEMU 4.1.1 package we can pass a additional internal version
to QEMU's machine, it will be split out there and ignored, but
returned on a QMP 'query-machines' call.
This allows us to use it for reducing the granularity with which we
can roll-out HW layout changes/additions for VMs. Until now we
required a machine version bump, happening normally every major
release of QEMU, with seldom, for us irrelevant, exceptions.
This often delays rolling out a feature, which would break
live-migration, by several months. That can now be avoided, the new
"pve-version" component of the machine can be bumped at will, and
thus we are much more flexible.
That versions orders after the ($major, $minor) version components
from an stable release - it can thus also be reset on the next
release.
The implementation extends the qemu-machine REGEX, remembers
"pve-version" when doing a "query-machines" and integrates support
into the min_version and extract_version helpers.
We start out with a version of 1.
Signed-off-by: Thomas Lamprecht <t.lampre...@proxmox.com>
---
PVE/QemuServer.pm | 17 +++++++++-------
PVE/QemuServer/Helpers.pm | 6 +++---
PVE/QemuServer/Machine.pm | 27 ++++++++++++++++++++------
test/cfg2cmd/minimal-defaults.conf.cmd | 2 +-
test/cfg2cmd/spice-linux-4.1.conf.cmd | 2 +-
5 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index fcedcf1..78a0a02 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -93,7 +93,7 @@
PVE::JSONSchema::register_standard_option('pve-qm-image-format', {
PVE::JSONSchema::register_standard_option('pve-qemu-machine', {
description => "Specifies the Qemu machine type.",
type => 'string',
- pattern =>
'(pc|pc(-i440fx)?-\d+(\.\d+)+(\.pxe)?|q35|pc-q35-\d+(\.\d+)+(\.pxe)?|virt(?:-\d+(\.\d+)+)?)',
+ pattern =>
'(pc|pc(-i440fx)?-\d+(\.\d+)+(\+pve\d+)?(\.pxe)?|q35|pc-q35-\d+(\.\d+)+(\+pve\d+)(\.pxe)?|virt(?:-\d+(\.\d+)+)?(\+pve\d+)?)',
Does this really work with '.pxe'? When is that even used, i.e. how
would one test this?
If QEMU needs to know about this, it has to be placed before the '+pve'
version, otherwise we just cut it off, no?
maxLength => 40,
optional => 1,
});
@@ -1875,7 +1875,7 @@ sub print_drivedevice_full {
}
# for compatibility only, we prefer scsi-hd (#2408, #2355, #2380)
- my $version =
PVE::QemuServer::Machine::extract_version($machine_type) // kvm_user_version();
+ my $version =
PVE::QemuServer::Machine::extract_version($machine_type, kvm_user_version());
if ($path =~ m/^iscsi\:\/\// &&
!min_version($version, 4, 1)) {
$devicetype = 'generic';
@@ -2740,7 +2740,7 @@ sub write_vm_config {
&$cleanup_config($conf->{pending}, 1);
foreach my $snapname (keys %{$conf->{snapshots}}) {
- die "internal error: snapshot name '$snapname' is forbidden" if
lc($snapname) eq 'pending';
+ die "internal error" if $snapname eq 'pending';
Doesn't belong to this patch.
&$cleanup_config($conf->{snapshots}->{$snapname}, undef, $snapname);
}
@@ -3361,13 +3361,14 @@ my $default_machines = {
};
sub get_vm_machine {
- my ($conf, $forcemachine, $arch) = @_;
+ my ($conf, $forcemachine, $arch, $add_pve_version) = @_;
my $machine = $forcemachine || $conf->{machine};
if (!$machine) {
$arch //= 'x86_64';
$machine ||= $default_machines->{$arch};
+ $machine .= "+pve$PVE::QemuServer::Machine::PVE_MACHINE_VERSION" if
$add_pve_version;
}
return $machine;
@@ -3469,8 +3470,10 @@ sub config_to_command {
my $kvm_binary = get_command_for_arch($arch);
my $kvmver = kvm_user_version($kvm_binary);
- my $machine_type = get_vm_machine($conf, $forcemachine, $arch);
- my $machine_version =
PVE::QemuServer::Machine::extract_version($machine_type) // $kvmver;
+ my $add_pve_version = min_version($kvmver, 4, 1);
+
+ my $machine_type = get_vm_machine($conf, $forcemachine, $arch,
$add_pve_version);
+ my $machine_version =
PVE::QemuServer::Machine::extract_version($machine_type, $kvmver);
$kvm //= 1 if is_native($arch);
if ($kvm) {
@@ -7048,7 +7051,7 @@ sub qemu_use_old_bios_files {
$machine_type = $1;
$use_old_bios_files = 1;
} else {
- my $version = PVE::QemuServer::Machine::extract_version($machine_type)
// kvm_user_version();
+ my $version = PVE::QemuServer::Machine::extract_version($machine_type,
kvm_user_version());
# Note: kvm version < 2.4 use non-efi pxe files, and have problems
when we
# load new efi bios files on migration. So this hack is required to
allow
# live migration from qemu-2.2 to qemu-2.4, which is sometimes used
when
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index 0e78f36..fcc9392 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -103,10 +103,10 @@ sub vm_running_locally {
}
sub min_version {
- my ($verstr, $version_major, $version_minor) = @_;
+ my ($verstr, $major, $minor, $pve) = @_;
- if ($verstr =~ m/^(\d+)\.(\d+)/) {
- return 1 if version_cmp($1, $version_major, $2, $version_minor) >= 0;
+ if ($verstr =~ m/^(\d+)\.(\d+)(?:\.(\d+))?(?:\+pve(\d+))?/) {
+ return 1 if version_cmp($1, $major, $2, $minor, $4, $pve) >= 0;
return 0;
}
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index 40c8eeb..98b2975 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -6,6 +6,10 @@ use warnings;
use PVE::QemuServer::Helpers;
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;
+
sub machine_type_is_q35 {
my ($conf) = @_;
@@ -18,31 +22,42 @@ sub get_current_qemu_machine {
my $res = PVE::QemuServer::Monitor::mon_cmd($vmid, 'query-machines');
- my ($current, $default);
+ my ($current, $pve_version, $default);
foreach my $e (@$res) {
$default = $e->{name} if $e->{'is-default'};
$current = $e->{name} if $e->{'is-current'};
+ $pve_version = $e->{'pve-version'} if $e->{'pve-version'};
}
+ $current .= "+$pve_version" if $current && $pve_version;
+
# fallback to the default machine if current is not supported by qemu
return $current || $default || 'pc';
}
+# returns a string with major.minor.pveversion, patch is ignored as it's seldom
+# ressembling a real QEMU machine type, so it would be 0 99% of the time..
It returns major.minor+pve{version}, not major.minor.pve - works fine,
just the comment is misleading.
sub extract_version {
- my ($machine_type) = @_;
+ my ($machine_type, $kvmversion) = @_;
- if ($machine_type && $machine_type =~ m/^((?:pc(-i440fx|-q35)?|virt)-(\d+)\.(\d+))/) {
- return "$3.$4";
+ if (defined($machine_type) && $machine_type =~
m/^(?:pc(?:-i440fx|-q35)?|virt)-(\d+)\.(\d+)(?:\.(\d+))?(\+pve\d+)?/) {
+ my $versionstr = "$1.$2";
+ $versionstr .= $4 if $4;
+ return $versionstr;
+ } elsif (defined($kvmversion)) {
+ if ($kvmversion =~ m/^(\d+)\.(\d+)/) {
+ return "$1.$2+pve$PVE_MACHINE_VERSION";
+ }
}
return undef;
}
sub machine_version {
- my ($machine_type, $version_major, $version_minor) = @_;
+ my ($machine_type, $major, $minor, $pve) = @_;
return PVE::QemuServer::Helpers::min_version(
- extract_version($machine_type), $version_major, $version_minor);
+ extract_version($machine_type), $major, $minor, $pve);
}
# dies if a) VM not running or not exisiting b) Version query failed
diff --git a/test/cfg2cmd/minimal-defaults.conf.cmd
b/test/cfg2cmd/minimal-defaults.conf.cmd
index 5abebe9..83ae328 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'
+ -machine 'type=pc+pve1'
Good idea to test, but this will break everytime the PVE_MACHINE_VERSION
is bumped. Seems like something to fix in cfg2cmd.pl and not here though
(maybe leave the +pve1 from the .cmd file and dynamically insert it with
the current version during the test?).
Also, maybe a test with pinned pve-version (not sure this has a real
use-case, but it works 'for free' and I don't see any downside).
diff --git a/test/cfg2cmd/spice-linux-4.1.conf.cmd
b/test/cfg2cmd/spice-linux-4.1.conf.cmd
index 158e73b..4ed6fd2 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'
+ -machine 'type=pc+pve1'
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
