From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <d.csapak@proxmox.com>
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 1DFB37C17F
 for <pve-devel@lists.proxmox.com>; Thu,  4 Nov 2021 09:06:25 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 0E98D1C3DE
 for <pve-devel@lists.proxmox.com>; Thu,  4 Nov 2021 09:05:55 +0100 (CET)
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) server-digest SHA256)
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id CD1841C3CC
 for <pve-devel@lists.proxmox.com>; Thu,  4 Nov 2021 09:05:53 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 9DE4345EE4;
 Thu,  4 Nov 2021 09:05:47 +0100 (CET)
Message-ID: <e7312da0-2ca2-45fc-a048-2791fab0ba22@proxmox.com>
Date: Thu, 4 Nov 2021 09:05:46 +0100
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 <pve-devel@lists.proxmox.com>,
 Thomas Lamprecht <t.lamprecht@proxmox.com>
References: <20211103155812.1461208-1-t.lamprecht@proxmox.com>
From: Dominik Csapak <d.csapak@proxmox.com>
In-Reply-To: <20211103155812.1461208-1-t.lamprecht@proxmox.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 2.015 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           -3.528 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
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [qemu.pm, qemuserver.pm]
Subject: Re: [pve-devel] [PATCH v2 0/4] <add meta info and bandaid for QEMU
 6.1 and unpinned q35 machine backward compat
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Thu, 04 Nov 2021 08:06:25 -0000

LGTM, tested fine and worked as advertised

if we want to go forward this way:

Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>

On 11/3/21 16:58, Thomas Lamprecht wrote:
> changes v1 -> v2:
> * move checks into own method
> * ensure forced machine is parsed so that min_version can work
> * only apply on `l26` ostype
> * fix test file name and comment
> 
> v1 cover letter:
> 
> 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                             | 73 +++++++++++++++++++
>   .../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-7.0.conf              | 14 ++++
>   test/cfg2cmd/q35-simple-7.0.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 ++++++++
>   12 files changed, 243 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-7.0.conf
>   create mode 100644 test/cfg2cmd/q35-simple-7.0.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
>