On 11/25/19 1:22 PM, Thomas Lamprecht wrote:
On 11/25/19 12:33 PM, Stefan Reiter wrote:
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
BTW, shouldn't that be *increasing* the granularity?
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?
The q35 one misses an ? after the (\+pve\d+), but else yes.
If QEMU needs to know about this, it has to be placed before the '+pve'
version, otherwise we just cut it off, no?
No, QEMU cannot parse that, it's "PVE" information for for an old
live-migration compat code path, we remove the .pxe part in
qemu_use_old_bios_files:
if ($machine_type =~ m/^(\S+)\.pxe$/) {
$machine_type = $1;
...
Ah I see. I read that sub as a getter, didn't realize it was used to
also modify the $machine_type.
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.
you're right, fallout from rebasing and saving over the applied
changes from Oguz. :)
@@ -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.
yes, tried some different approaches, and forgot to update the comment.
Thanks for noticing.
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?).
That's the purpose for the minimal-defaults config, to show the
resulting current minimal default command. So this is by design.
Makes sense.
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'
it may make sense to filter it for others though, else this could get
a bit noisy soon.
Looking through the tests also made me realize we need to add the
pve-version if a 'machine' is specified in the config but without a version.
E.g. having 'machine: pc' in the config is not versioned, so it should
also use the newest pve-version (it already uses the newest machine
version since that is aliased by QEMU itself).
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
