[pve-devel] [PATCH v3 qemu-server 6/8] refactor: split qemu_machine_feature_enabled

Stefan Reiter s.reiter at proxmox.com
Mon Nov 4 14:57:31 CET 2019


...into:

* PVE::QemuServer::Helpers::min_version: check a major.minor version
  string with a given major/minor version (this is equivalent to calling
  the old qemu_machine_feature_enabled with only $kvmver)
* PVE::QemuServer::Machine::extract_version: get major.minor version
  string from arbitrary machine type (e.g. pc-q35-4.0, ...)
* PVE::QemuServer::Machine::machine_version: helper to call
  extract_version automatically before min_version

Includes a cfg2cmd test case with pinned machine version.

Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
---

qemu_machine_feature_enabled has an astonishing variety of use-cases;
this patch just tries to make the differences clear by renaming and splitting
parts of the bundled-up semantic into seperate functions.


 PVE/QemuServer.pm                    | 46 +++++++++++++-------------
 PVE/QemuServer/Helpers.pm            | 33 +++++++++++++++++++
 PVE/QemuServer/Machine.pm            | 49 ++++++----------------------
 test/cfg2cmd/pinned-version.conf     | 15 +++++++++
 test/cfg2cmd/pinned-version.conf.cmd | 30 +++++++++++++++++
 5 files changed, 112 insertions(+), 61 deletions(-)
 create mode 100644 test/cfg2cmd/pinned-version.conf
 create mode 100644 test/cfg2cmd/pinned-version.conf.cmd

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index da6c7bb..d802ae3 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -41,7 +41,7 @@ use PVE::QMPClient;
 use PVE::QemuConfig;
 use PVE::QemuServer::Helpers;
 use PVE::QemuServer::Cloudinit;
-use PVE::QemuServer::Machine qw(qemu_machine_feature_enabled);
+use PVE::QemuServer::Machine;
 use PVE::QemuServer::Memory;
 use PVE::QemuServer::Monitor qw(mon_cmd);
 use PVE::QemuServer::PCI qw(print_pci_addr print_pcie_addr print_pcie_root_port);
@@ -1885,8 +1885,9 @@ 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();
 	    if ($path =~ m/^iscsi\:\/\// &&
-	       !qemu_machine_feature_enabled($machine_type, undef, 4, 1)) {
+	       !PVE::QemuServer::Helpers::min_version($version, 4, 1)) {
 		$devicetype = 'generic';
 	    }
 	}
@@ -3393,7 +3394,7 @@ sub get_command_for_arch($) {
 }
 
 sub get_cpu_options {
-    my ($conf, $arch, $kvm, $machine_type, $kvm_off, $kvmver, $winversion, $gpu_passthrough) = @_;
+    my ($conf, $arch, $kvm, $kvm_off, $machine_version, $winversion, $gpu_passthrough) = @_;
 
     my $cpuFlags = [];
     my $ostype = $conf->{ostype};
@@ -3424,13 +3425,13 @@ sub get_cpu_options {
 
     push @$cpuFlags, '-rdtscp' if $cpu =~ m/^Opteron/;
 
-    if (qemu_machine_feature_enabled ($machine_type, $kvmver, 2, 3) && $arch eq 'x86_64') {
+    if (PVE::QemuServer::Helpers::min_version($machine_version, 2, 3) && $arch eq 'x86_64') {
 
 	push @$cpuFlags , '+kvm_pv_unhalt' if $kvm;
 	push @$cpuFlags , '+kvm_pv_eoi' if $kvm;
     }
 
-    add_hyperv_enlightenments($cpuFlags, $winversion, $machine_type, $kvmver, $conf->{bios}, $gpu_passthrough, $hv_vendor_id) if $kvm;
+    add_hyperv_enlightenments($cpuFlags, $winversion, $machine_version, $conf->{bios}, $gpu_passthrough, $hv_vendor_id) if $kvm;
 
     push @$cpuFlags, 'enforce' if $cpu ne 'host' && $kvm && $arch eq 'x86_64';
 
@@ -3466,6 +3467,7 @@ sub config_to_command {
     my ($arch, $machine_type) = get_basic_machine_info($conf, $forcemachine);
     my $kvm_binary = get_command_for_arch($arch);
     my $kvmver = kvm_user_version($kvm_binary);
+    my $machine_version = PVE::QemuServer::Machine::extract_version($machine_type) // $kvmver;
     $kvm //= 1 if is_native($arch);
 
     if ($kvm) {
@@ -3503,7 +3505,7 @@ sub config_to_command {
     push @$cmd, '-chardev', "socket,id=qmp,path=$qmpsocket,server,nowait";
     push @$cmd, '-mon', "chardev=qmp,mode=control";
 
-    if (qemu_machine_feature_enabled($machine_type, $kvmver, 2, 12)) {
+    if (PVE::QemuServer::Helpers::min_version($machine_version, 2, 12)) {
 	push @$cmd, '-chardev', "socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5";
 	push @$cmd, '-mon', "chardev=qmp-event,mode=control";
     }
@@ -3574,7 +3576,7 @@ sub config_to_command {
     # load q35 config
     if ($q35) {
 	# we use different pcie-port hardware for qemu >= 4.0 for passthrough
-	if (qemu_machine_feature_enabled($machine_type, $kvmver, 4, 0)) {
+	if (PVE::QemuServer::Helpers::min_version($machine_version, 4, 0)) {
 	    push @$devices, '-readconfig', '/usr/share/qemu-server/pve-q35-4.0.cfg';
 	} else {
 	    push @$devices, '-readconfig', '/usr/share/qemu-server/pve-q35.cfg';
@@ -3592,7 +3594,7 @@ sub config_to_command {
     if (!$vga->{type}) {
 	if ($arch eq 'aarch64') {
 	    $vga->{type} = 'virtio';
-	} elsif (qemu_machine_feature_enabled($machine_type, $kvmver, 2, 9)) {
+	} elsif (PVE::QemuServer::Helpers::min_version($machine_version, 2, 9)) {
 	    $vga->{type} = (!$winversion || $winversion >= 6) ? 'std' : 'cirrus';
 	} else {
 	    $vga->{type} = ($winversion >= 6) ? 'std' : 'cirrus';
@@ -3689,7 +3691,7 @@ sub config_to_command {
 
     # usb devices
     my $usb_dev_features = {};
-    $usb_dev_features->{spice_usb3} = 1 if qemu_machine_feature_enabled($machine_type, $kvmver, 4, 0);
+    $usb_dev_features->{spice_usb3} = 1 if PVE::QemuServer::Helpers::min_version($machine_version, 4, 0);
 
     my @usbdevices = PVE::QemuServer::USB::get_usb_devices($conf, $usbdesc->{format}, $MAX_USB_DEVICES, $usb_dev_features);
     push @$devices, @usbdevices if @usbdevices;
@@ -3758,7 +3760,7 @@ sub config_to_command {
     die "MAX $allowed_vcpus vcpus allowed per VM on this node\n"
 	if ($allowed_vcpus < $maxcpus);
 
-    if($hotplug_features->{cpu} && qemu_machine_feature_enabled ($machine_type, $kvmver, 2, 7)) {
+    if($hotplug_features->{cpu} && PVE::QemuServer::Helpers::min_version($machine_version, 2, 7)) {
 
 	push @$cmd, '-smp', "1,sockets=$sockets,cores=$cores,maxcpus=$maxcpus";
         for (my $i = 2; $i <= $vcpus; $i++)  {
@@ -3831,7 +3833,7 @@ sub config_to_command {
 	push @$rtcFlags, 'base=localtime';
     }
 
-    push @$cmd, get_cpu_options($conf, $arch, $kvm, $machine_type, $kvm_off, $kvmver, $winversion, $gpu_passthrough);
+    push @$cmd, get_cpu_options($conf, $arch, $kvm, $kvm_off, $machine_version, $winversion, $gpu_passthrough);
 
     PVE::QemuServer::Memory::config($conf, $vmid, $sockets, $cores, $defaults, $hotplug_features, $cmd);
 
@@ -4018,7 +4020,7 @@ sub config_to_command {
 
     if (!$q35) {
 	# add pci bridges
-        if (qemu_machine_feature_enabled ($machine_type, $kvmver, 2, 3)) {
+        if (PVE::QemuServer::Helpers::min_version($machine_version, 2, 3)) {
 	   $bridges->{1} = 1;
 	   $bridges->{2} = 1;
 	}
@@ -4522,7 +4524,7 @@ sub qemu_cpu_hotplug {
 
     if ($vcpus < $currentvcpus) {
 
-	if (qemu_machine_feature_enabled ($machine_type, undef, 2, 7)) {
+	if (PVE::QemuServer::Machine::machine_version($machine_type, 2, 7)) {
 
 	    for (my $i = $currentvcpus; $i > $vcpus; $i--) {
 		qemu_devicedel($vmid, "cpu$i");
@@ -4550,7 +4552,7 @@ sub qemu_cpu_hotplug {
     die "vcpus in running vm does not match its configuration\n"
 	if scalar(@{$currentrunningvcpus}) != $currentvcpus;
 
-    if (qemu_machine_feature_enabled ($machine_type, undef, 2, 7)) {
+    if (PVE::QemuServer::Machine::machine_version($machine_type, 2, 7)) {
 
 	for (my $i = $currentvcpus+1; $i <= $vcpus; $i++) {
 	    my $cpustr = print_cpu_device($conf, $i);
@@ -6951,7 +6953,7 @@ sub clone_disk {
 	} else {
 
 	    my $kvmver = get_running_qemu_version ($vmid);
-	    if (!qemu_machine_feature_enabled (undef, $kvmver, 2, 7)) {
+	    if (!PVE::QemuServer::Helpers::min_version($kvmver, 2, 7)) {
 		die "drive-mirror with iothread requires qemu version 2.7 or higher\n"
 		    if $drive->{iothread};
 	    }
@@ -6987,12 +6989,12 @@ sub qemu_use_old_bios_files {
         $machine_type = $1;
         $use_old_bios_files = 1;
     } else {
-	my $kvmver = 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
         # updrading from proxmox-ve-3.X to proxmox-ve 4.0
-	$use_old_bios_files = !qemu_machine_feature_enabled ($machine_type, $kvmver, 2, 4);
+	$use_old_bios_files = !PVE::QemuServer::Helpers::min_version($version, 2, 4);
     }
 
     return ($use_old_bios_files, $machine_type);
@@ -7047,7 +7049,7 @@ sub scsihw_infos {
 }
 
 sub add_hyperv_enlightenments {
-    my ($cpuFlags, $winversion, $machine_type, $kvmver, $bios, $gpu_passthrough, $hv_vendor_id) = @_;
+    my ($cpuFlags, $winversion, $machine_version, $bios, $gpu_passthrough, $hv_vendor_id) = @_;
 
     return if $winversion < 6;
     return if $bios && $bios eq 'ovmf' && $winversion < 8;
@@ -7057,7 +7059,7 @@ sub add_hyperv_enlightenments {
 	push @$cpuFlags , "hv_vendor_id=$hv_vendor_id";
     }
 
-    if (qemu_machine_feature_enabled ($machine_type, $kvmver, 2, 3)) {
+    if (PVE::QemuServer::Helpers::min_version($machine_version, 2, 3)) {
 	push @$cpuFlags , 'hv_spinlocks=0x1fff';
 	push @$cpuFlags , 'hv_vapic';
 	push @$cpuFlags , 'hv_time';
@@ -7065,7 +7067,7 @@ sub add_hyperv_enlightenments {
 	push @$cpuFlags , 'hv_spinlocks=0xffff';
     }
 
-    if (qemu_machine_feature_enabled ($machine_type, $kvmver, 2, 6)) {
+    if (PVE::QemuServer::Helpers::min_version($machine_version, 2, 6)) {
 	push @$cpuFlags , 'hv_reset';
 	push @$cpuFlags , 'hv_vpindex';
 	push @$cpuFlags , 'hv_runtime';
@@ -7074,12 +7076,12 @@ sub add_hyperv_enlightenments {
     if ($winversion >= 7) {
 	push @$cpuFlags , 'hv_relaxed';
 
-	if (qemu_machine_feature_enabled ($machine_type, $kvmver, 2, 12)) {
+	if (PVE::QemuServer::Helpers::min_version($machine_version, 2, 12)) {
 	    push @$cpuFlags , 'hv_synic';
 	    push @$cpuFlags , 'hv_stimer';
 	}
 
-	if (qemu_machine_feature_enabled ($machine_type, $kvmver, 3, 1)) {
+	if (PVE::QemuServer::Helpers::min_version($machine_version, 3, 1)) {
 	    push @$cpuFlags , 'hv_ipi';
 	}
     }
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index 1f472a9..170753a 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -102,4 +102,37 @@ sub vm_running_locally {
     return undef;
 }
 
+sub min_version {
+    my ($verstr, $version_major, $version_minor) = @_;
+
+    if ($verstr =~ m/^(\d+)\.(\d+)/) {
+	return 1 if version_cmp($1, $version_major, $2, $version_minor) >= 0;
+	return 0;
+    }
+
+    die "internal error: cannot check version of invalid string '$verstr'";
+}
+
+# gets in pairs the versions you want to compares, i.e.:
+# ($a-major, $b-major, $a-minor, $b-minor, $a-extra, $b-extra, ...)
+# returns 0 if same, -1 if $a is older than $b, +1 if $a is newer than $b
+sub version_cmp {
+    my @versions = @_;
+
+    my $size = scalar(@versions);
+
+    return 0 if $size == 0;
+    die "cannot compare odd count of versions" if $size & 1;
+
+    for (my $i = 0; $i < $size; $i += 2) {
+	my ($a, $b) = splice(@versions, 0, 2);
+	$a //= 0;
+	$b //= 0;
+
+	return 1 if $a > $b;
+	return -1 if $a < $b;
+    }
+    return 0;
+}
+
 1;
diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
index d47f4e4..6515a3a 100644
--- a/PVE/QemuServer/Machine.pm
+++ b/PVE/QemuServer/Machine.pm
@@ -4,13 +4,9 @@ use strict;
 use warnings;
 
 use PVE::QemuConfig;
+use PVE::QemuServer::Helpers;
 use PVE::QemuServer::Monitor;
 
-use base 'Exporter';
-our @EXPORT_OK = qw(
-qemu_machine_feature_enabled
-);
-
 sub machine_type_is_q35 {
     my ($conf) = @_;
 
@@ -33,46 +29,21 @@ sub get_current_qemu_machine {
     return $current || $default || 'pc';
 }
 
-sub qemu_machine_feature_enabled {
-    my ($machine, $kvmver, $version_major, $version_minor) = @_;
-
-    my $current_major;
-    my $current_minor;
-
-    if ($machine && $machine =~ m/^((?:pc(-i440fx|-q35)?|virt)-(\d+)\.(\d+))/) {
-
-	$current_major = $3;
-	$current_minor = $4;
-
-    } elsif ($kvmver =~ m/^(\d+)\.(\d+)/) {
+sub extract_version {
+    my ($machine_type) = @_;
 
-	$current_major = $1;
-	$current_minor = $2;
+    if ($machine_type && $machine_type =~ m/^((?:pc(-i440fx|-q35)?|virt)-(\d+)\.(\d+))/) {
+	return "$3.$4";
     }
 
-    return 1 if version_cmp($current_major, $version_major, $current_minor, $version_minor) >= 0;
+    return undef;
 }
 
-# gets in pairs the versions you want to compares, i.e.:
-# ($a-major, $b-major, $a-minor, $b-minor, $a-extra, $b-extra, ...)
-# returns 0 if same, -1 if $a is older than $b, +1 if $a is newer than $b
-sub version_cmp {
-    my @versions = @_;
+sub machine_version {
+    my ($machine_type, $version_major, $version_minor) = @_;
 
-    my $size = scalar(@versions);
-
-    return 0 if $size == 0;
-    die "cannot compare odd count of versions" if $size & 1;
-
-    for (my $i = 0; $i < $size; $i += 2) {
-	my ($a, $b) = splice(@versions, 0, 2);
-	$a //= 0;
-	$b //= 0;
-
-	return 1 if $a > $b;
-	return -1 if $a < $b;
-    }
-    return 0;
+    return PVE::QemuServer::Helpers::min_version(
+	extract_version($machine_type), $version_major, $version_minor);
 }
 
 # dies if a) VM not running or not exisiting b) Version query failed
diff --git a/test/cfg2cmd/pinned-version.conf b/test/cfg2cmd/pinned-version.conf
new file mode 100644
index 0000000..6119183
--- /dev/null
+++ b/test/cfg2cmd/pinned-version.conf
@@ -0,0 +1,15 @@
+# TEST: Simple test for a basic configuration with pinned QEMU machine version
+bootdisk: scsi0
+cores: 3
+ide2: none,media=cdrom
+machine: pc-q35-3.1
+memory: 1024
+name: pinned
+net0: virtio=A2:C0:43:77:08:A1,bridge=vmbr0
+numa: 0
+ostype: l26
+scsi0: local:8006/vm-8006-disk-0.raw,discard=on,size=104858K
+scsihw: virtio-scsi-pci
+smbios1: uuid=c7fdd046-fefc-11e9-832e-770e1d5636a0
+sockets: 1
+vmgenid: bdd46b98-fefc-11e9-97b4-d72c378e0f96
diff --git a/test/cfg2cmd/pinned-version.conf.cmd b/test/cfg2cmd/pinned-version.conf.cmd
new file mode 100644
index 0000000..cc43d22
--- /dev/null
+++ b/test/cfg2cmd/pinned-version.conf.cmd
@@ -0,0 +1,30 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name pinned \
+  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
+  -mon 'chardev=qmp,mode=control' \
+  -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+  -mon 'chardev=qmp-event,mode=control' \
+  -pidfile /var/run/qemu-server/8006.pid \
+  -daemonize \
+  -smbios 'type=1,uuid=c7fdd046-fefc-11e9-832e-770e1d5636a0' \
+  -smp '3,sockets=1,cores=3,maxcpus=3' \
+  -nodefaults \
+  -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
+  -vnc unix:/var/run/qemu-server/8006.vnc,password \
+  -cpu kvm64,+lahf_lm,+sep,+kvm_pv_unhalt,+kvm_pv_eoi,enforce \
+  -m 1024 \
+  -device 'vmgenid,guid=bdd46b98-fefc-11e9-97b4-d72c378e0f96' \
+  -readconfig /usr/share/qemu-server/pve-q35.cfg \
+  -device 'usb-tablet,id=tablet,bus=ehci.0,port=1' \
+  -device 'VGA,id=vga,bus=pcie.0,addr=0x1' \
+  -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3' \
+  -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+  -drive 'if=none,id=drive-ide2,media=cdrom,aio=threads' \
+  -device 'ide-cd,bus=ide.1,unit=0,drive=drive-ide2,id=ide2,bootindex=200' \
+  -device 'virtio-scsi-pci,id=scsihw0,bus=pci.0,addr=0x5' \
+  -drive 'file=/var/lib/vz/images/8006/vm-8006-disk-0.raw,if=none,id=drive-scsi0,discard=on,format=raw,cache=none,aio=native,detect-zeroes=unmap' \
+  -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'
-- 
2.20.1





More information about the pve-devel mailing list