From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 282F377103 for ; Thu, 21 Oct 2021 11:47:38 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1FEFC1C1EB for ; Thu, 21 Oct 2021 11:47:08 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 03EA11C1E0 for ; Thu, 21 Oct 2021 11:47:07 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id D1192458F9 for ; Thu, 21 Oct 2021 11:47:06 +0200 (CEST) Message-ID: <8d65a4f5-ab0c-43c7-2c1e-9b3209ea6427@proxmox.com> Date: Thu, 21 Oct 2021 11:47:05 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:94.0) Gecko/20100101 Thunderbird/94.0 Content-Language: en-US To: Proxmox VE development discussion , Stefan Reiter References: <20211021083609.2057094-1-t.lamprecht@proxmox.com> <20211021083609.2057094-5-t.lamprecht@proxmox.com> From: Thomas Lamprecht In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 1.325 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -2.267 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH 4/4] cfg2cmd: switch off ACPI hotplug on bridges for q35 VMs X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Oct 2021 09:47:38 -0000 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=3D`, 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-nami= ng-scheme.html#Naming >> >> Signed-off-by: Thomas Lamprecht >> --- >> =C2=A0 PVE/QemuServer.pm=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | 18 ++++++++++++++++= ++ >> =C2=A0 .../q35-linux-hostpci-multifunction.conf.cmd=C2=A0=C2=A0 |=C2=A0= 1 + >> =C2=A0 test/cfg2cmd/q35-linux-hostpci.conf.cmd=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 |=C2=A0 1 + >> =C2=A0 test/cfg2cmd/q35-simple.conf.cmd=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 1 + >> =C2=A0 test/cfg2cmd/q35-win10-hostpci.conf.cmd=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 |=C2=A0 1 + >> =C2=A0 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 { >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >> =C2=A0 +=C2=A0=C2=A0=C2=A0 my $meta =3D parse_meta_info($conf->{meta})= // {}; >> +=C2=A0=C2=A0=C2=A0 # check if we need to apply some handling for VMs = that always use the latest machine version but >> +=C2=A0=C2=A0=C2=A0 # had a machine version transition happen that aff= ected HW such that, e.g., an OS config change >> +=C2=A0=C2=A0=C2=A0 # would be required (we do not want to pin machine= version for non-windows OS type) >> +=C2=A0=C2=A0=C2=A0 my $create_qemu_vers =3D $meta->{'creation-qemu'};= >> +=C2=A0=C2=A0=C2=A0 if ( >> +=C2=A0=C2=A0=C2=A0 (!defined($conf->{machine}) || $conf->{machine} =3D= ~ m/^(?:pc|q35|virt)$/) # non-versioned machine >> +=C2=A0=C2=A0=C2=A0 && (!defined($create_qemu_vers) || !min_version($c= reate_qemu_vers, 6, 1)) # created before 6.1 >> +=C2=A0=C2=A0=C2=A0 && (!$forcemachine || min_version($forcemachine, 6= , 1)) # handle snapshot-rollback/migrations >=20 > $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' c= annot handle that. >=20 >> +=C2=A0=C2=A0=C2=A0 && min_version($kvmver, 6, 1) # only need to apply= the first change with 6.1 >> +=C2=A0=C2=A0=C2=A0 ) { >> +=C2=A0=C2=A0=C2=A0 if ($q35) { >=20 > I think this could go into the outer if as well, so it shortcircuits fo= r 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: >=20 > =C2=A0 (!defined($conf->{machine}) || $conf->{machine} =3D~ m/^(?:pc|q3= 5|virt)$/) > to > =C2=A0 (defined($conf->{machine}) && $conf->{machine} eq 'q35') >=20 > as the default can never be q35. >=20 >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 # this changed to default-= on in Q 6.1 for q35 machines, it will mess with PCI slot view >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 # and thus with the predic= table interface naming of systemd >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 push @$cmd, '-global', 'IC= H9-LPC.acpi-pci-hotplug-with-bridge-support=3Doff'; >> +=C2=A0=C2=A0=C2=A0 } >> +=C2=A0=C2=A0=C2=A0 } >> + >=20 > 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_v= ersion_fixups' or so. >=20 yes, that would be def. nicer