From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id 741FE1FF16B
	for <inbox@lore.proxmox.com>; Thu,  6 Mar 2025 14:10:11 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 3925669D0;
	Thu,  6 Mar 2025 14:10:05 +0100 (CET)
Message-ID: <ecd1b69e-f381-4703-a1a2-2185f6e72500@proxmox.com>
Date: Thu, 6 Mar 2025 14:10:00 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
 Dominik Csapak <d.csapak@proxmox.com>
References: <20250306104459.1272297-1-d.csapak@proxmox.com>
 <20250306104459.1272297-5-d.csapak@proxmox.com>
Content-Language: en-US
From: Fiona Ebner <f.ebner@proxmox.com>
In-Reply-To: <20250306104459.1272297-5-d.csapak@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.042 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 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 qemu-server 4/8] machine: correctly select
 pve machine version for non pinned windows guests
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>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>

Am 06.03.25 um 11:44 schrieb Dominik Csapak:
> when we don't have a specific machine version on a windows guest, we use
> the creation meta info to pin the machine version. Currently we always
> append the pve machine version from the current installed kvm version,
> which is not necessarily the version we pinned the guest to.
> 
> Instead, use either the info from the creation meta info if it exists,
> or use 'pve0'.
> 
> For non-windows machines, we used the current QEMU machine version so we
> should use the pve machine version from that too.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  PVE/QemuServer/Machine.pm | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
> index f1acde8f..e3da8e21 100644
> --- a/PVE/QemuServer/Machine.pm
> +++ b/PVE/QemuServer/Machine.pm
> @@ -237,14 +237,19 @@ sub get_vm_machine {
>  		if (PVE::QemuServer::Helpers::min_version($meta->{'creation-qemu'}, 9, 1)) {
>  		    # need only major.minor
>  		    ($base_version) = ($meta->{'creation-qemu'} =~ m/^(\d+.\d+)/);
> +		    # append pve machine version if we have one
> +		    if (my $pvever = $meta->{'creation-pve-machine'}) {
> +			$base_version .= "+pve$pvever"

Since this is only the fallback handling for the rare edge case where no
explicit machine version is set for a Windows guest, not sure if it's
even worth doing this. I.e. can we just avoid the additional meta
property and always use pve0 here? Did you intend any other use for the
creation-pve-machine?

Further below we already have:

> 	# for version-pinned machines that do not include a pve-version (e.g.
> 	# pc-q35-4.1), we assume 0 to keep them stable in case we bump
> 	$machine .= '+pve0';

so let's just follow this here too?


> +		    }
>  		}
>  	    }
>  	    $machine = windows_get_pinned_machine_version($machine, $base_version, $kvmversion);
> +	} else{
> +	    $arch //= 'x86_64';
> +	    $machine ||= default_machine_for_arch($arch);
> +	    my $pvever = get_pve_version($kvmversion);
> +	    $machine .= "+pve$pvever";
>  	}
> -	$arch //= 'x86_64';
> -	$machine ||= default_machine_for_arch($arch);
> -	my $pvever = get_pve_version($kvmversion);
> -	$machine .= "+pve$pvever";
>      }
>  
>      if ($machine !~ m/\+pve\d+?(?:\.pxe)?$/) {



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel