Re: [pve-devel] [PATCH 4/4] cfg2cmd: switch off ACPI hotplug on bridges for q35 VMs

[Stefan Reiter <s.reiter@proxmox.com>]

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' \
>