Re: [pve-devel] [PATCH 0/4] add meta info and bandaid for QEMU 6.1 and unpinned q35 machine backward compat

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

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! ;)