public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat
@ 2021-10-21  8:36 Thomas Lamprecht
  2021-10-21  8:36 ` [pve-devel] [PATCH 1/4] config: add new meta property withe creation time Thomas Lamprecht
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Thomas Lamprecht @ 2021-10-21  8:36 UTC (permalink / raw)
  To: pve-devel

First add a new meta property that is currently exclusively set on new
VM creation and then read-only, initially add the creation time as UNIX
epoch and the QEMU version that was installed during installation
(thought about using the one on first start but that actually does not
gives any more guarantee, so just go for simple).

Use that information to band aid around a change regarding hotplug in
QEMU 6.1 that can affected older VMs on fresh start (migration and
rollback is covered by force-machine mechanisms as always already).

I'm not 100% convinced of the whole thing, albeit I see some merit in
the meta property even if we do not go with the last patch, anyhow, I
proposed this off-list to Dominik (and those thing is partly his idea
too), Wolfgang, Fabian and Stefan and none of them rejected the idea nor
communicated a better/more preferred alternative, so I went for it
(still not steaming from enthusiasm though)

Thomas Lamprecht (4):
  config: add new meta property withe creation time
  config: meta: also save the QEMU version installed during creation
  tests: cfg2cmd: add a few q35 related tests
  cfg2cmd: switch off ACPI hotplug on bridges for q35 VMs

 PVE/API2/Qemu.pm                              |  2 +
 PVE/QemuServer.pm                             | 62 +++++++++++++++++++
 .../q35-linux-hostpci-multifunction.conf.cmd  |  1 +
 test/cfg2cmd/q35-linux-hostpci.conf.cmd       |  1 +
 test/cfg2cmd/q35-simple-6.0.conf              | 13 ++++
 test/cfg2cmd/q35-simple-6.0.conf.cmd          | 28 +++++++++
 test/cfg2cmd/q35-simple-6.1.conf              | 14 +++++
 test/cfg2cmd/q35-simple-6.1.conf.cmd          | 28 +++++++++
 test/cfg2cmd/q35-simple-pinned-6.1.conf       | 13 ++++
 test/cfg2cmd/q35-simple-pinned-6.1.conf.cmd   | 28 +++++++++
 test/cfg2cmd/q35-simple.conf                  | 13 ++++
 test/cfg2cmd/q35-simple.conf.cmd              | 29 +++++++++
 test/cfg2cmd/q35-win10-hostpci.conf.cmd       |  1 +
 13 files changed, 233 insertions(+)
 create mode 100644 test/cfg2cmd/q35-simple-6.0.conf
 create mode 100644 test/cfg2cmd/q35-simple-6.0.conf.cmd
 create mode 100644 test/cfg2cmd/q35-simple-6.1.conf
 create mode 100644 test/cfg2cmd/q35-simple-6.1.conf.cmd
 create mode 100644 test/cfg2cmd/q35-simple-pinned-6.1.conf
 create mode 100644 test/cfg2cmd/q35-simple-pinned-6.1.conf.cmd
 create mode 100644 test/cfg2cmd/q35-simple.conf
 create mode 100644 test/cfg2cmd/q35-simple.conf.cmd

-- 
2.30.2





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] [PATCH 1/4] config: add new meta property withe creation time
  2021-10-21  8:36 [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat Thomas Lamprecht
@ 2021-10-21  8:36 ` Thomas Lamprecht
  2021-10-21  8:36 ` [pve-devel] [PATCH 2/4] config: meta: also save the QEMU version installed during creation Thomas Lamprecht
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Thomas Lamprecht @ 2021-10-21  8:36 UTC (permalink / raw)
  To: pve-devel

currently we only add the creation time (ctime), that was requested
as low priority wish from some users from time to time.

Note that the meta info is not available in the update API endpoints,
and at the moment the code should not change/add/delete it either in
any place.

We may want to update in on actions like clone or backup-restore in
the future, e.g., to also save the time of that event and possibly
the original source VMID, put that can be thought out later.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 PVE/API2/Qemu.pm  |  2 ++
 PVE/QemuServer.pm | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 8c5abc1..408bbbb 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -706,6 +706,8 @@ __PACKAGE__->register_method({
 		my $conf = $param;
 		my $arch = PVE::QemuServer::get_vm_arch($conf);
 
+		$conf->{meta} = PVE::QemuServer::new_meta_info_string();
+
 		my $vollist = [];
 		eval {
 		    $vollist = &$create_disks($rpcenv, $authuser, $conf, $arch, $storecfg, $vmid, $pool, $param, $storage);
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index e7204c7..639ccd0 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -278,6 +278,15 @@ my $rng_fmt = {
     },
 };
 
+my $meta_info_fmt = {
+    'ctime' => {
+	type => 'integer',
+	description => "The guest creation timestamp as UNIX epoch time",
+	minimum => 0,
+	optional => 1,
+    },
+};
+
 my $confdesc = {
     onboot => {
 	optional => 1,
@@ -699,6 +708,12 @@ EODESCR
 	description => "Configure a VirtIO-based Random Number Generator.",
 	optional => 1,
     },
+    meta => {
+	type => 'string',
+	format => $meta_info_fmt,
+	description => "Some (read-only) meta-information about this guest.",
+	optional => 1,
+    },
 };
 
 my $cicustom_fmt = {
@@ -2096,6 +2111,27 @@ sub parse_rng {
     return $res;
 }
 
+sub parse_meta_info {
+    my ($value) = @_;
+
+    return if !$value;
+
+    my $res = eval { parse_property_string($meta_info_fmt, $value) };
+    warn $@ if $@;
+    return $res;
+}
+
+sub new_meta_info_string {
+    my () = @_; # for now do not allow to override any value
+
+    return PVE::JSONSchema::print_property_string(
+	{
+	    ctime => "". int(time()),
+	},
+	$meta_info_fmt
+    );
+}
+
 PVE::JSONSchema::register_format('pve-qm-usb-device', \&verify_usb_device);
 sub verify_usb_device {
     my ($value, $noerr) = @_;
@@ -2117,6 +2153,7 @@ sub json_config_properties {
 	vmstate => 1,
 	runningmachine => 1,
 	runningcpu => 1,
+	meta => 1,
     };
 
     foreach my $opt (keys %$confdesc) {
-- 
2.30.2





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] [PATCH 2/4] config: meta: also save the QEMU version installed during creation
  2021-10-21  8:36 [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat Thomas Lamprecht
  2021-10-21  8:36 ` [pve-devel] [PATCH 1/4] config: add new meta property withe creation time Thomas Lamprecht
@ 2021-10-21  8:36 ` Thomas Lamprecht
  2021-10-21  8:36 ` [pve-devel] [PATCH 3/4] tests: cfg2cmd: add a few q35 related tests Thomas Lamprecht
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Thomas Lamprecht @ 2021-10-21  8:36 UTC (permalink / raw)
  To: pve-devel

This is intended to be used to apply some workarounds for the
non-windows ostyped VMs which we'd still like to not pin on a
specific machine version, as normally Linux et al. can cope with such
changes on fresh boot just fine and until now this was a once every
few year issue (albeit systemd's "predictable" interface naming has
some potential to pick up on churn frequency).

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 PVE/QemuServer.pm | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 639ccd0..b10f1b5 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -285,6 +285,12 @@ my $meta_info_fmt = {
 	minimum => 0,
 	optional => 1,
     },
+    'creation-qemu' => {
+	type => 'string',
+	description => "The QEMU (machine) version from the time this VM was created.",
+	pattern => '\d+(\.\d+)+',
+	optional => 1,
+    },
 };
 
 my $confdesc = {
@@ -2126,6 +2132,7 @@ sub new_meta_info_string {
 
     return PVE::JSONSchema::print_property_string(
 	{
+	    'creation-qemu' => kvm_user_version(),
 	    ctime => "". int(time()),
 	},
 	$meta_info_fmt
-- 
2.30.2





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] [PATCH 3/4] tests: cfg2cmd: add a few q35 related tests
  2021-10-21  8:36 [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat Thomas Lamprecht
  2021-10-21  8:36 ` [pve-devel] [PATCH 1/4] config: add new meta property withe creation time Thomas Lamprecht
  2021-10-21  8:36 ` [pve-devel] [PATCH 2/4] config: meta: also save the QEMU version installed during creation Thomas Lamprecht
@ 2021-10-21  8:36 ` Thomas Lamprecht
  2021-10-21  9:34   ` Stefan Reiter
  2021-10-21  8:36 ` [pve-devel] [PATCH 4/4] cfg2cmd: switch off ACPI hotplug on bridges for q35 VMs Thomas Lamprecht
  2021-10-21  9:34 ` [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat Stefan Reiter
  4 siblings, 1 reply; 13+ messages in thread
From: Thomas Lamprecht @ 2021-10-21  8:36 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 test/cfg2cmd/q35-simple-6.0.conf            | 13 ++++++++++
 test/cfg2cmd/q35-simple-6.0.conf.cmd        | 28 +++++++++++++++++++++
 test/cfg2cmd/q35-simple-6.1.conf            | 14 +++++++++++
 test/cfg2cmd/q35-simple-6.1.conf.cmd        | 28 +++++++++++++++++++++
 test/cfg2cmd/q35-simple-pinned-6.1.conf     | 13 ++++++++++
 test/cfg2cmd/q35-simple-pinned-6.1.conf.cmd | 28 +++++++++++++++++++++
 test/cfg2cmd/q35-simple.conf                | 13 ++++++++++
 test/cfg2cmd/q35-simple.conf.cmd            | 28 +++++++++++++++++++++
 8 files changed, 165 insertions(+)
 create mode 100644 test/cfg2cmd/q35-simple-6.0.conf
 create mode 100644 test/cfg2cmd/q35-simple-6.0.conf.cmd
 create mode 100644 test/cfg2cmd/q35-simple-6.1.conf
 create mode 100644 test/cfg2cmd/q35-simple-6.1.conf.cmd
 create mode 100644 test/cfg2cmd/q35-simple-pinned-6.1.conf
 create mode 100644 test/cfg2cmd/q35-simple-pinned-6.1.conf.cmd
 create mode 100644 test/cfg2cmd/q35-simple.conf
 create mode 100644 test/cfg2cmd/q35-simple.conf.cmd

diff --git a/test/cfg2cmd/q35-simple-6.0.conf b/test/cfg2cmd/q35-simple-6.0.conf
new file mode 100644
index 0000000..70426b3
--- /dev/null
+++ b/test/cfg2cmd/q35-simple-6.0.conf
@@ -0,0 +1,13 @@
+# TEST: Config with q35, Linux & nothing much else but on 6.0
+# QEMU_VERSION: 6.0.0
+bios: ovmf
+bootdisk: scsi0
+cores: 2
+efidisk0: local:100/vm-100-disk-1.qcow2,size=128K
+machine: q35
+memory: 512
+net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
+ostype: l26
+scsihw: virtio-scsi-pci
+smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
+vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
diff --git a/test/cfg2cmd/q35-simple-6.0.conf.cmd b/test/cfg2cmd/q35-simple-6.0.conf.cmd
new file mode 100644
index 0000000..5045caf
--- /dev/null
+++ b/test/cfg2cmd/q35-simple-6.0.conf.cmd
@@ -0,0 +1,28 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name vm8006 \
+  -no-shutdown \
+  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+  -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=3dd750ce-d910-44d0-9493-525c0be4e687' \
+  -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
+  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
+  -smp '2,sockets=1,cores=2,maxcpus=2' \
+  -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=on' \
+  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+  -m 512 \
+  -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \
+  -device 'vmgenid,guid=54d1c06c-8f5b-440f-b5b2-6eab1380e13d' \
+  -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' \
+  -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+pve0'
diff --git a/test/cfg2cmd/q35-simple-6.1.conf b/test/cfg2cmd/q35-simple-6.1.conf
new file mode 100644
index 0000000..df5ee73
--- /dev/null
+++ b/test/cfg2cmd/q35-simple-6.1.conf
@@ -0,0 +1,14 @@
+# TEST: Config with q35, Linux & nothing much else but on 6.1
+# QEMU_VERSION: 7.0.0
+bios: ovmf
+bootdisk: scsi0
+cores: 2
+efidisk0: local:100/vm-100-disk-1.qcow2,size=128K
+machine: q35
+meta: creation-qemu=6.1
+memory: 512
+net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
+ostype: l26
+scsihw: virtio-scsi-pci
+smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
+vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
diff --git a/test/cfg2cmd/q35-simple-6.1.conf.cmd b/test/cfg2cmd/q35-simple-6.1.conf.cmd
new file mode 100644
index 0000000..5045caf
--- /dev/null
+++ b/test/cfg2cmd/q35-simple-6.1.conf.cmd
@@ -0,0 +1,28 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name vm8006 \
+  -no-shutdown \
+  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+  -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=3dd750ce-d910-44d0-9493-525c0be4e687' \
+  -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
+  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
+  -smp '2,sockets=1,cores=2,maxcpus=2' \
+  -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=on' \
+  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+  -m 512 \
+  -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \
+  -device 'vmgenid,guid=54d1c06c-8f5b-440f-b5b2-6eab1380e13d' \
+  -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' \
+  -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+pve0'
diff --git a/test/cfg2cmd/q35-simple-pinned-6.1.conf b/test/cfg2cmd/q35-simple-pinned-6.1.conf
new file mode 100644
index 0000000..9ecfe00
--- /dev/null
+++ b/test/cfg2cmd/q35-simple-pinned-6.1.conf
@@ -0,0 +1,13 @@
+# TEST: Config with q35, Linux & nothing much else
+#
+bios: ovmf
+bootdisk: scsi0
+cores: 2
+efidisk0: local:100/vm-100-disk-1.qcow2,size=128K
+machine: pc-q35-6.1
+memory: 512
+net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
+ostype: l26
+scsihw: virtio-scsi-pci
+smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
+vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
diff --git a/test/cfg2cmd/q35-simple-pinned-6.1.conf.cmd b/test/cfg2cmd/q35-simple-pinned-6.1.conf.cmd
new file mode 100644
index 0000000..26dfaaa
--- /dev/null
+++ b/test/cfg2cmd/q35-simple-pinned-6.1.conf.cmd
@@ -0,0 +1,28 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name vm8006 \
+  -no-shutdown \
+  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+  -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=3dd750ce-d910-44d0-9493-525c0be4e687' \
+  -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
+  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
+  -smp '2,sockets=1,cores=2,maxcpus=2' \
+  -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=on' \
+  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+  -m 512 \
+  -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \
+  -device 'vmgenid,guid=54d1c06c-8f5b-440f-b5b2-6eab1380e13d' \
+  -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' \
+  -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=pc-q35-6.1+pve0'
diff --git a/test/cfg2cmd/q35-simple.conf b/test/cfg2cmd/q35-simple.conf
new file mode 100644
index 0000000..21f7812
--- /dev/null
+++ b/test/cfg2cmd/q35-simple.conf
@@ -0,0 +1,13 @@
+# TEST: Config with q35, Linux & nothing much else
+#
+bios: ovmf
+bootdisk: scsi0
+cores: 2
+efidisk0: local:100/vm-100-disk-1.qcow2,size=128K
+machine: q35
+memory: 512
+net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
+ostype: l26
+scsihw: virtio-scsi-pci
+smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
+vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
diff --git a/test/cfg2cmd/q35-simple.conf.cmd b/test/cfg2cmd/q35-simple.conf.cmd
new file mode 100644
index 0000000..5045caf
--- /dev/null
+++ b/test/cfg2cmd/q35-simple.conf.cmd
@@ -0,0 +1,28 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name vm8006 \
+  -no-shutdown \
+  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+  -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=3dd750ce-d910-44d0-9493-525c0be4e687' \
+  -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
+  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
+  -smp '2,sockets=1,cores=2,maxcpus=2' \
+  -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=on' \
+  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+  -m 512 \
+  -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \
+  -device 'vmgenid,guid=54d1c06c-8f5b-440f-b5b2-6eab1380e13d' \
+  -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' \
+  -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+pve0'
-- 
2.30.2





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] [PATCH 4/4] cfg2cmd: switch off ACPI hotplug on bridges for q35 VMs
  2021-10-21  8:36 [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat Thomas Lamprecht
                   ` (2 preceding siblings ...)
  2021-10-21  8:36 ` [pve-devel] [PATCH 3/4] tests: cfg2cmd: add a few q35 related tests Thomas Lamprecht
@ 2021-10-21  8:36 ` Thomas Lamprecht
  2021-10-21  9:34   ` Stefan Reiter
  2021-10-21  9:34 ` [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat Stefan Reiter
  4 siblings, 1 reply; 13+ messages in thread
From: Thomas Lamprecht @ 2021-10-21  8:36 UTC (permalink / raw)
  To: pve-devel

See commit 17858a1695 (hw/acpi/ich9: Set ACPI PCI hot-plug as default
on Q35)[0] in upstream QEMU repository for details about why the
change was made.

As that change affects systemds predictable interface naming[1],
e.g., by going from a previously `ens18` name to `enp6s18`, it may
have rather bad effects for users that did not setup some .link files
to enforce a specific naming by an more stable information like the
NIC's MAC-Address

The alternative would be making the preferred mode of hotplug an
option like `hotplug-mode=<acpi|pcie>`, but it does not seems like
one would like to change that much in the first place...

Note the changes to the tests and especially the tests with q35
machines that did not change.

[0]: https://gitlab.com/qemu-project/qemu/-/commit/17858a1695
[1]: https://www.freedesktop.org/software/systemd/man/systemd.net-naming-scheme.html#Naming

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 PVE/QemuServer.pm                              | 18 ++++++++++++++++++
 .../q35-linux-hostpci-multifunction.conf.cmd   |  1 +
 test/cfg2cmd/q35-linux-hostpci.conf.cmd        |  1 +
 test/cfg2cmd/q35-simple.conf.cmd               |  1 +
 test/cfg2cmd/q35-win10-hostpci.conf.cmd        |  1 +
 5 files changed, 22 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index b10f1b5..84caee7 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3534,6 +3534,24 @@ sub config_to_command {
 	}
     }
 
+    my $meta = parse_meta_info($conf->{meta}) // {};
+    # check if we need to apply some handling for VMs that always use the latest machine version but
+    # had a machine version transition happen that affected HW such that, e.g., an OS config change
+    # would be required (we do not want to pin machine version for non-windows OS type)
+    my $create_qemu_vers = $meta->{'creation-qemu'};
+    if (
+	(!defined($conf->{machine}) || $conf->{machine} =~ m/^(?:pc|q35|virt)$/) # non-versioned machine
+	&& (!defined($create_qemu_vers) || !min_version($create_qemu_vers, 6, 1)) # created before 6.1
+	&& (!$forcemachine || min_version($forcemachine, 6, 1)) # handle snapshot-rollback/migrations
+	&& min_version($kvmver, 6, 1) # only need to apply the first change with 6.1
+    ) {
+	if ($q35) {
+	    # this changed to default-on in Q 6.1 for q35 machines, it will mess with PCI slot view
+	    # and thus with the predictable interface naming of systemd
+	    push @$cmd, '-global', 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off';
+	}
+    }
+
     if ($conf->{vmgenid}) {
 	push @$devices, '-device', 'vmgenid,guid='.$conf->{vmgenid};
     }
diff --git a/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd b/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd
index 1f9beda..d393906 100644
--- a/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd
+++ b/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd
@@ -11,6 +11,7 @@
   -smbios 'type=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
   -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
   -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
+  -global 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' \
   -smp '2,sockets=2,cores=1,maxcpus=2' \
   -nodefaults \
   -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
diff --git a/test/cfg2cmd/q35-linux-hostpci.conf.cmd b/test/cfg2cmd/q35-linux-hostpci.conf.cmd
index dd1bece..5da5c35 100644
--- a/test/cfg2cmd/q35-linux-hostpci.conf.cmd
+++ b/test/cfg2cmd/q35-linux-hostpci.conf.cmd
@@ -11,6 +11,7 @@
   -smbios 'type=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
   -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
   -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
+  -global 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' \
   -smp '2,sockets=2,cores=1,maxcpus=2' \
   -nodefaults \
   -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
diff --git a/test/cfg2cmd/q35-simple.conf.cmd b/test/cfg2cmd/q35-simple.conf.cmd
index 5045caf..1c97a89 100644
--- a/test/cfg2cmd/q35-simple.conf.cmd
+++ b/test/cfg2cmd/q35-simple.conf.cmd
@@ -11,6 +11,7 @@
   -smbios 'type=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
   -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
   -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
+  -global 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' \
   -smp '2,sockets=1,cores=2,maxcpus=2' \
   -nodefaults \
   -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
diff --git a/test/cfg2cmd/q35-win10-hostpci.conf.cmd b/test/cfg2cmd/q35-win10-hostpci.conf.cmd
index 37ef8f7..843de96 100644
--- a/test/cfg2cmd/q35-win10-hostpci.conf.cmd
+++ b/test/cfg2cmd/q35-win10-hostpci.conf.cmd
@@ -11,6 +11,7 @@
   -smbios 'type=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
   -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
   -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
+  -global 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' \
   -smp '2,sockets=2,cores=1,maxcpus=2' \
   -nodefaults \
   -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
-- 
2.30.2





^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat
  2021-10-21  8:36 [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat Thomas Lamprecht
                   ` (3 preceding siblings ...)
  2021-10-21  8:36 ` [pve-devel] [PATCH 4/4] cfg2cmd: switch off ACPI hotplug on bridges for q35 VMs Thomas Lamprecht
@ 2021-10-21  9:34 ` Stefan Reiter
  2021-10-21  9:47   ` Thomas Lamprecht
  4 siblings, 1 reply; 13+ messages in thread
From: Stefan Reiter @ 2021-10-21  9:34 UTC (permalink / raw)
  To: Proxmox VE development discussion, Thomas Lamprecht

On 10/21/21 10:36 AM, Thomas Lamprecht wrote:
> First add a new meta property that is currently exclusively set on new
> VM creation and then read-only, initially add the creation time as UNIX
> epoch and the QEMU version that was installed during installation
> (thought about using the one on first start but that actually does not
> gives any more guarantee, so just go for simple).
> 
> Use that information to band aid around a change regarding hotplug in
> QEMU 6.1 that can affected older VMs on fresh start (migration and
> rollback is covered by force-machine mechanisms as always already).
> 
> I'm not 100% convinced of the whole thing, albeit I see some merit in
> the meta property even if we do not go with the last patch, anyhow, I
> proposed this off-list to Dominik (and those thing is partly his idea
> too), Wolfgang, Fabian and Stefan and none of them rejected the idea nor
> communicated a better/more preferred alternative, so I went for it
> (still not steaming from enthusiasm though)

So we're doing all of this to avoid issues with older VMs that expect
"acpi-pci-hotplug-with-bridge-support=off" on Q35 (previously default),
but we still want to set it for new VMs that are created with QEMU 6.1
and never booted with anything older.

But taking a step back, do we actually want the new ACPI hotplug in
general? If we choose to simply leave it be, we could just always add
"acpi-pci-hotplug-with-bridge-support=off" to Q35 on QEMU > 6.1.
Since it's a global property, I think we wouldn't even need to check
machine-type/forcemachine at all, since we'd only make the default
explicit with older ones.

> 
> Thomas Lamprecht (4):
>    config: add new meta property withe creation time
>    config: meta: also save the QEMU version installed during creation
>    tests: cfg2cmd: add a few q35 related tests
>    cfg2cmd: switch off ACPI hotplug on bridges for q35 VMs
> 
>   PVE/API2/Qemu.pm                              |  2 +
>   PVE/QemuServer.pm                             | 62 +++++++++++++++++++
>   .../q35-linux-hostpci-multifunction.conf.cmd  |  1 +
>   test/cfg2cmd/q35-linux-hostpci.conf.cmd       |  1 +
>   test/cfg2cmd/q35-simple-6.0.conf              | 13 ++++
>   test/cfg2cmd/q35-simple-6.0.conf.cmd          | 28 +++++++++
>   test/cfg2cmd/q35-simple-6.1.conf              | 14 +++++
>   test/cfg2cmd/q35-simple-6.1.conf.cmd          | 28 +++++++++
>   test/cfg2cmd/q35-simple-pinned-6.1.conf       | 13 ++++
>   test/cfg2cmd/q35-simple-pinned-6.1.conf.cmd   | 28 +++++++++
>   test/cfg2cmd/q35-simple.conf                  | 13 ++++
>   test/cfg2cmd/q35-simple.conf.cmd              | 29 +++++++++
>   test/cfg2cmd/q35-win10-hostpci.conf.cmd       |  1 +
>   13 files changed, 233 insertions(+)
>   create mode 100644 test/cfg2cmd/q35-simple-6.0.conf
>   create mode 100644 test/cfg2cmd/q35-simple-6.0.conf.cmd
>   create mode 100644 test/cfg2cmd/q35-simple-6.1.conf
>   create mode 100644 test/cfg2cmd/q35-simple-6.1.conf.cmd
>   create mode 100644 test/cfg2cmd/q35-simple-pinned-6.1.conf
>   create mode 100644 test/cfg2cmd/q35-simple-pinned-6.1.conf.cmd
>   create mode 100644 test/cfg2cmd/q35-simple.conf
>   create mode 100644 test/cfg2cmd/q35-simple.conf.cmd
> 




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH 3/4] tests: cfg2cmd: add a few q35 related tests
  2021-10-21  8:36 ` [pve-devel] [PATCH 3/4] tests: cfg2cmd: add a few q35 related tests Thomas Lamprecht
@ 2021-10-21  9:34   ` Stefan Reiter
  2021-10-21  9:45     ` Thomas Lamprecht
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Reiter @ 2021-10-21  9:34 UTC (permalink / raw)
  To: Proxmox VE development discussion, Thomas Lamprecht

On 10/21/21 10:36 AM, Thomas Lamprecht wrote:
> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> ---
>   test/cfg2cmd/q35-simple-6.0.conf            | 13 ++++++++++
>   test/cfg2cmd/q35-simple-6.0.conf.cmd        | 28 +++++++++++++++++++++
>   test/cfg2cmd/q35-simple-6.1.conf            | 14 +++++++++++
>   test/cfg2cmd/q35-simple-6.1.conf.cmd        | 28 +++++++++++++++++++++
>   test/cfg2cmd/q35-simple-pinned-6.1.conf     | 13 ++++++++++
>   test/cfg2cmd/q35-simple-pinned-6.1.conf.cmd | 28 +++++++++++++++++++++
>   test/cfg2cmd/q35-simple.conf                | 13 ++++++++++
>   test/cfg2cmd/q35-simple.conf.cmd            | 28 +++++++++++++++++++++
>   8 files changed, 165 insertions(+)
>   create mode 100644 test/cfg2cmd/q35-simple-6.0.conf
>   create mode 100644 test/cfg2cmd/q35-simple-6.0.conf.cmd
>   create mode 100644 test/cfg2cmd/q35-simple-6.1.conf
>   create mode 100644 test/cfg2cmd/q35-simple-6.1.conf.cmd
>   create mode 100644 test/cfg2cmd/q35-simple-pinned-6.1.conf
>   create mode 100644 test/cfg2cmd/q35-simple-pinned-6.1.conf.cmd
>   create mode 100644 test/cfg2cmd/q35-simple.conf
>   create mode 100644 test/cfg2cmd/q35-simple.conf.cmd
> 
> diff --git a/test/cfg2cmd/q35-simple-6.0.conf b/test/cfg2cmd/q35-simple-6.0.conf
> new file mode 100644
> index 0000000..70426b3
> --- /dev/null
> +++ b/test/cfg2cmd/q35-simple-6.0.conf
> @@ -0,0 +1,13 @@
> +# TEST: Config with q35, Linux & nothing much else but on 6.0
> +# QEMU_VERSION: 6.0.0
> +bios: ovmf
> +bootdisk: scsi0
> +cores: 2
> +efidisk0: local:100/vm-100-disk-1.qcow2,size=128K
> +machine: q35
> +memory: 512
> +net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
> +ostype: l26
> +scsihw: virtio-scsi-pci
> +smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
> +vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
> diff --git a/test/cfg2cmd/q35-simple-6.0.conf.cmd b/test/cfg2cmd/q35-simple-6.0.conf.cmd
> new file mode 100644
> index 0000000..5045caf
> --- /dev/null
> +++ b/test/cfg2cmd/q35-simple-6.0.conf.cmd
> @@ -0,0 +1,28 @@
> +/usr/bin/kvm \
> +  -id 8006 \
> +  -name vm8006 \
> +  -no-shutdown \
> +  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
> +  -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=3dd750ce-d910-44d0-9493-525c0be4e687' \
> +  -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
> +  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
> +  -smp '2,sockets=1,cores=2,maxcpus=2' \
> +  -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=on' \
> +  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
> +  -m 512 \
> +  -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \
> +  -device 'vmgenid,guid=54d1c06c-8f5b-440f-b5b2-6eab1380e13d' \
> +  -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' \
> +  -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+pve0'
> diff --git a/test/cfg2cmd/q35-simple-6.1.conf b/test/cfg2cmd/q35-simple-6.1.conf
> new file mode 100644
> index 0000000..df5ee73
> --- /dev/null
> +++ b/test/cfg2cmd/q35-simple-6.1.conf
> @@ -0,0 +1,14 @@
> +# TEST: Config with q35, Linux & nothing much else but on 6.1
> +# QEMU_VERSION: 7.0.0

6.1.0? I mean the test still makes sense, but then the name/comment are wrong ;)

> +bios: ovmf
> +bootdisk: scsi0
> +cores: 2
> +efidisk0: local:100/vm-100-disk-1.qcow2,size=128K
> +machine: q35
> +meta: creation-qemu=6.1
> +memory: 512
> +net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
> +ostype: l26
> +scsihw: virtio-scsi-pci
> +smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
> +vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
> diff --git a/test/cfg2cmd/q35-simple-6.1.conf.cmd b/test/cfg2cmd/q35-simple-6.1.conf.cmd
> new file mode 100644
> index 0000000..5045caf
> --- /dev/null
> +++ b/test/cfg2cmd/q35-simple-6.1.conf.cmd
> @@ -0,0 +1,28 @@
> +/usr/bin/kvm \
> +  -id 8006 \
> +  -name vm8006 \
> +  -no-shutdown \
> +  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
> +  -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=3dd750ce-d910-44d0-9493-525c0be4e687' \
> +  -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
> +  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
> +  -smp '2,sockets=1,cores=2,maxcpus=2' \
> +  -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=on' \
> +  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
> +  -m 512 \
> +  -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \
> +  -device 'vmgenid,guid=54d1c06c-8f5b-440f-b5b2-6eab1380e13d' \
> +  -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' \
> +  -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+pve0'
> diff --git a/test/cfg2cmd/q35-simple-pinned-6.1.conf b/test/cfg2cmd/q35-simple-pinned-6.1.conf
> new file mode 100644
> index 0000000..9ecfe00
> --- /dev/null
> +++ b/test/cfg2cmd/q35-simple-pinned-6.1.conf
> @@ -0,0 +1,13 @@
> +# TEST: Config with q35, Linux & nothing much else
> +#
> +bios: ovmf
> +bootdisk: scsi0
> +cores: 2
> +efidisk0: local:100/vm-100-disk-1.qcow2,size=128K
> +machine: pc-q35-6.1
> +memory: 512
> +net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
> +ostype: l26
> +scsihw: virtio-scsi-pci
> +smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
> +vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
> diff --git a/test/cfg2cmd/q35-simple-pinned-6.1.conf.cmd b/test/cfg2cmd/q35-simple-pinned-6.1.conf.cmd
> new file mode 100644
> index 0000000..26dfaaa
> --- /dev/null
> +++ b/test/cfg2cmd/q35-simple-pinned-6.1.conf.cmd
> @@ -0,0 +1,28 @@
> +/usr/bin/kvm \
> +  -id 8006 \
> +  -name vm8006 \
> +  -no-shutdown \
> +  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
> +  -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=3dd750ce-d910-44d0-9493-525c0be4e687' \
> +  -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
> +  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
> +  -smp '2,sockets=1,cores=2,maxcpus=2' \
> +  -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=on' \
> +  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
> +  -m 512 \
> +  -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \
> +  -device 'vmgenid,guid=54d1c06c-8f5b-440f-b5b2-6eab1380e13d' \
> +  -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' \
> +  -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=pc-q35-6.1+pve0'
> diff --git a/test/cfg2cmd/q35-simple.conf b/test/cfg2cmd/q35-simple.conf
> new file mode 100644
> index 0000000..21f7812
> --- /dev/null
> +++ b/test/cfg2cmd/q35-simple.conf
> @@ -0,0 +1,13 @@
> +# TEST: Config with q35, Linux & nothing much else
> +#

Not that it matters much but I think this test is fairly useless, we already have very similar ones with the 'hostpci' q35 configs. May be good to have it explicit though...

> +bios: ovmf
> +bootdisk: scsi0
> +cores: 2
> +efidisk0: local:100/vm-100-disk-1.qcow2,size=128K
> +machine: q35
> +memory: 512
> +net0: virtio=2E:01:68:F9:9C:87,bridge=vmbr0
> +ostype: l26
> +scsihw: virtio-scsi-pci
> +smbios1: uuid=3dd750ce-d910-44d0-9493-525c0be4e687
> +vmgenid: 54d1c06c-8f5b-440f-b5b2-6eab1380e13d
> diff --git a/test/cfg2cmd/q35-simple.conf.cmd b/test/cfg2cmd/q35-simple.conf.cmd
> new file mode 100644
> index 0000000..5045caf
> --- /dev/null
> +++ b/test/cfg2cmd/q35-simple.conf.cmd
> @@ -0,0 +1,28 @@
> +/usr/bin/kvm \
> +  -id 8006 \
> +  -name vm8006 \
> +  -no-shutdown \
> +  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
> +  -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=3dd750ce-d910-44d0-9493-525c0be4e687' \
> +  -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
> +  -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
> +  -smp '2,sockets=1,cores=2,maxcpus=2' \
> +  -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=on' \
> +  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
> +  -m 512 \
> +  -readconfig /usr/share/qemu-server/pve-q35-4.0.cfg \
> +  -device 'vmgenid,guid=54d1c06c-8f5b-440f-b5b2-6eab1380e13d' \
> +  -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' \
> +  -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+pve0'
> 




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH 4/4] cfg2cmd: switch off ACPI hotplug on bridges for q35 VMs
  2021-10-21  8:36 ` [pve-devel] [PATCH 4/4] cfg2cmd: switch off ACPI hotplug on bridges for q35 VMs Thomas Lamprecht
@ 2021-10-21  9:34   ` Stefan Reiter
  2021-10-21  9:47     ` Thomas Lamprecht
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Reiter @ 2021-10-21  9:34 UTC (permalink / raw)
  To: Proxmox VE development discussion, Thomas Lamprecht

On 10/21/21 10:36 AM, Thomas Lamprecht wrote:
> See commit 17858a1695 (hw/acpi/ich9: Set ACPI PCI hot-plug as default
> on Q35)[0] in upstream QEMU repository for details about why the
> change was made.
> 
> As that change affects systemds predictable interface naming[1],
> e.g., by going from a previously `ens18` name to `enp6s18`, it may
> have rather bad effects for users that did not setup some .link files
> to enforce a specific naming by an more stable information like the
> NIC's MAC-Address
> 
> The alternative would be making the preferred mode of hotplug an
> option like `hotplug-mode=<acpi|pcie>`, but it does not seems like
> one would like to change that much in the first place...
> 
> Note the changes to the tests and especially the tests with q35
> machines that did not change.
> 
> [0]: https://gitlab.com/qemu-project/qemu/-/commit/17858a1695
> [1]: https://www.freedesktop.org/software/systemd/man/systemd.net-naming-scheme.html#Naming
> 
> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> ---
>   PVE/QemuServer.pm                              | 18 ++++++++++++++++++
>   .../q35-linux-hostpci-multifunction.conf.cmd   |  1 +
>   test/cfg2cmd/q35-linux-hostpci.conf.cmd        |  1 +
>   test/cfg2cmd/q35-simple.conf.cmd               |  1 +
>   test/cfg2cmd/q35-win10-hostpci.conf.cmd        |  1 +
>   5 files changed, 22 insertions(+)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index b10f1b5..84caee7 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -3534,6 +3534,24 @@ sub config_to_command {
>   	}
>       }
>   
> +    my $meta = parse_meta_info($conf->{meta}) // {};
> +    # check if we need to apply some handling for VMs that always use the latest machine version but
> +    # had a machine version transition happen that affected HW such that, e.g., an OS config change
> +    # would be required (we do not want to pin machine version for non-windows OS type)
> +    my $create_qemu_vers = $meta->{'creation-qemu'};
> +    if (
> +	(!defined($conf->{machine}) || $conf->{machine} =~ m/^(?:pc|q35|virt)$/) # non-versioned machine
> +	&& (!defined($create_qemu_vers) || !min_version($create_qemu_vers, 6, 1)) # created before 6.1
> +	&& (!$forcemachine || min_version($forcemachine, 6, 1)) # handle snapshot-rollback/migrations

$forcemachine is not in QEMU version format, it contains a machine type (with a version at the end), e.g.: "pc-i440fx-6.1+pve0". 'min_version' cannot handle that.

> +	&& min_version($kvmver, 6, 1) # only need to apply the first change with 6.1
> +    ) {
> +	if ($q35) {

I think this could go into the outer if as well, so it shortcircuits for i440fx.
Or even just change:

   (!defined($conf->{machine}) || $conf->{machine} =~ m/^(?:pc|q35|virt)$/)
to
   (defined($conf->{machine}) && $conf->{machine} eq 'q35')

as the default can never be q35.

> +	    # this changed to default-on in Q 6.1 for q35 machines, it will mess with PCI slot view
> +	    # and thus with the predictable interface naming of systemd
> +	    push @$cmd, '-global', 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off';
> +	}
> +    }
> +

Considering the 'meta' property is supposed to be generic and might see more use in the future, I'd put this in a seperate function, 'add_qemu_version_fixups' or so.

>       if ($conf->{vmgenid}) {
>   	push @$devices, '-device', 'vmgenid,guid='.$conf->{vmgenid};
>       }
> diff --git a/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd b/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd
> index 1f9beda..d393906 100644
> --- a/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd
> +++ b/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd
> @@ -11,6 +11,7 @@
>     -smbios 'type=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
>     -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
>     -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
> +  -global 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' \
>     -smp '2,sockets=2,cores=1,maxcpus=2' \
>     -nodefaults \
>     -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
> diff --git a/test/cfg2cmd/q35-linux-hostpci.conf.cmd b/test/cfg2cmd/q35-linux-hostpci.conf.cmd
> index dd1bece..5da5c35 100644
> --- a/test/cfg2cmd/q35-linux-hostpci.conf.cmd
> +++ b/test/cfg2cmd/q35-linux-hostpci.conf.cmd
> @@ -11,6 +11,7 @@
>     -smbios 'type=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
>     -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
>     -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
> +  -global 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' \
>     -smp '2,sockets=2,cores=1,maxcpus=2' \
>     -nodefaults \
>     -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
> diff --git a/test/cfg2cmd/q35-simple.conf.cmd b/test/cfg2cmd/q35-simple.conf.cmd
> index 5045caf..1c97a89 100644
> --- a/test/cfg2cmd/q35-simple.conf.cmd
> +++ b/test/cfg2cmd/q35-simple.conf.cmd
> @@ -11,6 +11,7 @@
>     -smbios 'type=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
>     -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
>     -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
> +  -global 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' \
>     -smp '2,sockets=1,cores=2,maxcpus=2' \
>     -nodefaults \
>     -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
> diff --git a/test/cfg2cmd/q35-win10-hostpci.conf.cmd b/test/cfg2cmd/q35-win10-hostpci.conf.cmd
> index 37ef8f7..843de96 100644
> --- a/test/cfg2cmd/q35-win10-hostpci.conf.cmd
> +++ b/test/cfg2cmd/q35-win10-hostpci.conf.cmd
> @@ -11,6 +11,7 @@
>     -smbios 'type=1,uuid=3dd750ce-d910-44d0-9493-525c0be4e687' \
>     -drive 'if=pflash,unit=0,format=raw,readonly=on,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd' \
>     -drive 'if=pflash,unit=1,format=qcow2,id=drive-efidisk0,file=/var/lib/vz/images/100/vm-100-disk-1.qcow2' \
> +  -global 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off' \
>     -smp '2,sockets=2,cores=1,maxcpus=2' \
>     -nodefaults \
>     -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
> 




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH 3/4] tests: cfg2cmd: add a few q35 related tests
  2021-10-21  9:34   ` Stefan Reiter
@ 2021-10-21  9:45     ` Thomas Lamprecht
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Lamprecht @ 2021-10-21  9:45 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Reiter

On 21.10.21 11:34, Stefan Reiter wrote:
> 
> Not that it matters much but I think this test is fairly useless, we already have very similar ones with the 'hostpci' q35 configs. May be good to have it explicit though...

that's the idea :)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH 4/4] cfg2cmd: switch off ACPI hotplug on bridges for q35 VMs
  2021-10-21  9:34   ` Stefan Reiter
@ 2021-10-21  9:47     ` Thomas Lamprecht
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Lamprecht @ 2021-10-21  9:47 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Reiter

On 21.10.21 11:34, Stefan Reiter wrote:
> On 10/21/21 10:36 AM, Thomas Lamprecht wrote:
>> See commit 17858a1695 (hw/acpi/ich9: Set ACPI PCI hot-plug as default
>> on Q35)[0] in upstream QEMU repository for details about why the
>> change was made.
>>
>> As that change affects systemds predictable interface naming[1],
>> e.g., by going from a previously `ens18` name to `enp6s18`, it may
>> have rather bad effects for users that did not setup some .link files
>> to enforce a specific naming by an more stable information like the
>> NIC's MAC-Address
>>
>> The alternative would be making the preferred mode of hotplug an
>> option like `hotplug-mode=<acpi|pcie>`, but it does not seems like
>> one would like to change that much in the first place...
>>
>> Note the changes to the tests and especially the tests with q35
>> machines that did not change.
>>
>> [0]: https://gitlab.com/qemu-project/qemu/-/commit/17858a1695
>> [1]: https://www.freedesktop.org/software/systemd/man/systemd.net-naming-scheme.html#Naming
>>
>> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
>> ---
>>   PVE/QemuServer.pm                              | 18 ++++++++++++++++++
>>   .../q35-linux-hostpci-multifunction.conf.cmd   |  1 +
>>   test/cfg2cmd/q35-linux-hostpci.conf.cmd        |  1 +
>>   test/cfg2cmd/q35-simple.conf.cmd               |  1 +
>>   test/cfg2cmd/q35-win10-hostpci.conf.cmd        |  1 +
>>   5 files changed, 22 insertions(+)
>>
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index b10f1b5..84caee7 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -3534,6 +3534,24 @@ sub config_to_command {
>>       }
>>       }
>>   +    my $meta = parse_meta_info($conf->{meta}) // {};
>> +    # check if we need to apply some handling for VMs that always use the latest machine version but
>> +    # had a machine version transition happen that affected HW such that, e.g., an OS config change
>> +    # would be required (we do not want to pin machine version for non-windows OS type)
>> +    my $create_qemu_vers = $meta->{'creation-qemu'};
>> +    if (
>> +    (!defined($conf->{machine}) || $conf->{machine} =~ m/^(?:pc|q35|virt)$/) # non-versioned machine
>> +    && (!defined($create_qemu_vers) || !min_version($create_qemu_vers, 6, 1)) # created before 6.1
>> +    && (!$forcemachine || min_version($forcemachine, 6, 1)) # handle snapshot-rollback/migrations
> 
> $forcemachine is not in QEMU version format, it contains a machine type (with a version at the end), e.g.: "pc-i440fx-6.1+pve0". 'min_version' cannot handle that.
> 
>> +    && min_version($kvmver, 6, 1) # only need to apply the first change with 6.1
>> +    ) {
>> +    if ($q35) {
> 
> I think this could go into the outer if as well, so it shortcircuits for i440fx.

It can and I had it that way but I wanted to have above simpler to adapt to more of
such changes.

> Or even just change:
> 
>   (!defined($conf->{machine}) || $conf->{machine} =~ m/^(?:pc|q35|virt)$/)
> to
>   (defined($conf->{machine}) && $conf->{machine} eq 'q35')
> 
> as the default can never be q35.
> 
>> +        # this changed to default-on in Q 6.1 for q35 machines, it will mess with PCI slot view
>> +        # and thus with the predictable interface naming of systemd
>> +        push @$cmd, '-global', 'ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off';
>> +    }
>> +    }
>> +
> 
> Considering the 'meta' property is supposed to be generic and might see more use in the future, I'd put this in a seperate function, 'add_qemu_version_fixups' or so.
> 

yes, that would be def. nicer




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat
  2021-10-21  9:34 ` [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat Stefan Reiter
@ 2021-10-21  9:47   ` Thomas Lamprecht
  2021-10-21  9:56     ` Stefan Reiter
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Lamprecht @ 2021-10-21  9:47 UTC (permalink / raw)
  To: Proxmox VE development discussion, Stefan Reiter

On 21.10.21 11:34, Stefan Reiter wrote:
> On 10/21/21 10:36 AM, Thomas Lamprecht wrote:
>> First add a new meta property that is currently exclusively set on new
>> VM creation and then read-only, initially add the creation time as UNIX
>> epoch and the QEMU version that was installed during installation
>> (thought about using the one on first start but that actually does not
>> gives any more guarantee, so just go for simple).
>>
>> Use that information to band aid around a change regarding hotplug in
>> QEMU 6.1 that can affected older VMs on fresh start (migration and
>> rollback is covered by force-machine mechanisms as always already).
>>
>> I'm not 100% convinced of the whole thing, albeit I see some merit in
>> the meta property even if we do not go with the last patch, anyhow, I
>> proposed this off-list to Dominik (and those thing is partly his idea
>> too), Wolfgang, Fabian and Stefan and none of them rejected the idea nor
>> communicated a better/more preferred alternative, so I went for it
>> (still not steaming from enthusiasm though)
> 
> So we're doing all of this to avoid issues with older VMs that expect
> "acpi-pci-hotplug-with-bridge-support=off" on Q35 (previously default),
> but we still want to set it for new VMs that are created with QEMU 6.1
> and never booted with anything older.
> 
> But taking a step back, do we actually want the new ACPI hotplug in
> general? If we choose to simply leave it be, we could just always add
> "acpi-pci-hotplug-with-bridge-support=off" to Q35 on QEMU > 6.1.
> Since it's a global property, I think we wouldn't even need to check
> machine-type/forcemachine at all, since we'd only make the default
> explicit with older ones.

Check the commit I linked in patch 4/4, the change has some value.

https://gitlab.com/qemu-project/qemu/-/commit/17858a1695

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat
  2021-10-21  9:47   ` Thomas Lamprecht
@ 2021-10-21  9:56     ` Stefan Reiter
  2021-10-21 10:01       ` Thomas Lamprecht
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Reiter @ 2021-10-21  9:56 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox VE development discussion

On 10/21/21 11:47 AM, Thomas Lamprecht wrote:
> On 21.10.21 11:34, Stefan Reiter wrote:
>> On 10/21/21 10:36 AM, Thomas Lamprecht wrote:
>>> First add a new meta property that is currently exclusively set on new
>>> VM creation and then read-only, initially add the creation time as UNIX
>>> epoch and the QEMU version that was installed during installation
>>> (thought about using the one on first start but that actually does not
>>> gives any more guarantee, so just go for simple).
>>>
>>> Use that information to band aid around a change regarding hotplug in
>>> QEMU 6.1 that can affected older VMs on fresh start (migration and
>>> rollback is covered by force-machine mechanisms as always already).
>>>
>>> I'm not 100% convinced of the whole thing, albeit I see some merit in
>>> the meta property even if we do not go with the last patch, anyhow, I
>>> proposed this off-list to Dominik (and those thing is partly his idea
>>> too), Wolfgang, Fabian and Stefan and none of them rejected the idea nor
>>> communicated a better/more preferred alternative, so I went for it
>>> (still not steaming from enthusiasm though)
>>
>> So we're doing all of this to avoid issues with older VMs that expect
>> "acpi-pci-hotplug-with-bridge-support=off" on Q35 (previously default),
>> but we still want to set it for new VMs that are created with QEMU 6.1
>> and never booted with anything older.
>>
>> But taking a step back, do we actually want the new ACPI hotplug in
>> general? If we choose to simply leave it be, we could just always add
>> "acpi-pci-hotplug-with-bridge-support=off" to Q35 on QEMU > 6.1.
>> Since it's a global property, I think we wouldn't even need to check
>> machine-type/forcemachine at all, since we'd only make the default
>> explicit with older ones.
> 
> Check the commit I linked in patch 4/4, the change has some value.
> 
> https://gitlab.com/qemu-project/qemu/-/commit/17858a1695
> 

I did read it, and I agree it has some improvements, was just
wondering if it was worth our effort here (never encountered any of
the described bugs or saw a user that encountered them anywhere).

But I don't think this series is as bad as you make it out to be
either ;)




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat
  2021-10-21  9:56     ` Stefan Reiter
@ 2021-10-21 10:01       ` Thomas Lamprecht
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Lamprecht @ 2021-10-21 10:01 UTC (permalink / raw)
  To: Stefan Reiter, Proxmox VE development discussion

On 21.10.21 11:56, Stefan Reiter wrote:
> On 10/21/21 11:47 AM, Thomas Lamprecht wrote:
>> On 21.10.21 11:34, Stefan Reiter wrote:
>>> On 10/21/21 10:36 AM, Thomas Lamprecht wrote:
>>>> First add a new meta property that is currently exclusively set on new
>>>> VM creation and then read-only, initially add the creation time as UNIX
>>>> epoch and the QEMU version that was installed during installation
>>>> (thought about using the one on first start but that actually does not
>>>> gives any more guarantee, so just go for simple).
>>>>
>>>> Use that information to band aid around a change regarding hotplug in
>>>> QEMU 6.1 that can affected older VMs on fresh start (migration and
>>>> rollback is covered by force-machine mechanisms as always already).
>>>>
>>>> I'm not 100% convinced of the whole thing, albeit I see some merit in
>>>> the meta property even if we do not go with the last patch, anyhow, I
>>>> proposed this off-list to Dominik (and those thing is partly his idea
>>>> too), Wolfgang, Fabian and Stefan and none of them rejected the idea nor
>>>> communicated a better/more preferred alternative, so I went for it
>>>> (still not steaming from enthusiasm though)
>>>
>>> So we're doing all of this to avoid issues with older VMs that expect
>>> "acpi-pci-hotplug-with-bridge-support=off" on Q35 (previously default),
>>> but we still want to set it for new VMs that are created with QEMU 6.1
>>> and never booted with anything older.
>>>
>>> But taking a step back, do we actually want the new ACPI hotplug in
>>> general? If we choose to simply leave it be, we could just always add
>>> "acpi-pci-hotplug-with-bridge-support=off" to Q35 on QEMU > 6.1.
>>> Since it's a global property, I think we wouldn't even need to check
>>> machine-type/forcemachine at all, since we'd only make the default
>>> explicit with older ones.
>>
>> Check the commit I linked in patch 4/4, the change has some value.
>>
>> https://gitlab.com/qemu-project/qemu/-/commit/17858a1695
>>
> 
> I did read it, and I agree it has some improvements, was just
> wondering if it was worth our effort here (never encountered any of
> the described bugs or saw a user that encountered them anywhere).
> 

hmm, yeah neither did, but not sure how much worth it is to go against
the upstream's new default here...

FWIW, I also stumbled upon a followup for that change that happened after
6.1 release:

https://gitlab.com/qemu-project/qemu/-/commit/0e780da76a6fe283a20283856718bca3986c104f

> But I don't think this series is as bad as you make it out to be
> either ;)

hehe, thanks! ;)

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-10-21 10:01 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-21  8:36 [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat Thomas Lamprecht
2021-10-21  8:36 ` [pve-devel] [PATCH 1/4] config: add new meta property withe creation time Thomas Lamprecht
2021-10-21  8:36 ` [pve-devel] [PATCH 2/4] config: meta: also save the QEMU version installed during creation Thomas Lamprecht
2021-10-21  8:36 ` [pve-devel] [PATCH 3/4] tests: cfg2cmd: add a few q35 related tests Thomas Lamprecht
2021-10-21  9:34   ` Stefan Reiter
2021-10-21  9:45     ` Thomas Lamprecht
2021-10-21  8:36 ` [pve-devel] [PATCH 4/4] cfg2cmd: switch off ACPI hotplug on bridges for q35 VMs Thomas Lamprecht
2021-10-21  9:34   ` Stefan Reiter
2021-10-21  9:47     ` Thomas Lamprecht
2021-10-21  9:34 ` [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat Stefan Reiter
2021-10-21  9:47   ` Thomas Lamprecht
2021-10-21  9:56     ` Stefan Reiter
2021-10-21 10:01       ` Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal