all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Stefan Reiter <s.reiter@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Thomas Lamprecht <t.lamprecht@proxmox.com>
Subject: Re: [pve-devel] [PATCH 4/4] cfg2cmd: switch off ACPI hotplug on bridges for q35 VMs
Date: Thu, 21 Oct 2021 11:34:55 +0200	[thread overview]
Message-ID: <c40e59dd-1716-1bf1-c40b-ff14ba8b1787@proxmox.com> (raw)
In-Reply-To: <20211021083609.2057094-5-t.lamprecht@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' \
> 




  reply	other threads:[~2021-10-21  9:35 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c40e59dd-1716-1bf1-c40b-ff14ba8b1787@proxmox.com \
    --to=s.reiter@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=t.lamprecht@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal