Re: [pve-devel] [PATCH qemu-server 4/8] machine: correctly select pve machine version for non pinned windows guests

[Fiona Ebner <f.ebner@proxmox.com>]

Am 06.03.25 um 15:15 schrieb Dominik Csapak:
> On 3/6/25 15:10, Fiona Ebner wrote:
>> Am 06.03.25 um 14:36 schrieb Dominik Csapak:
>>> On 3/6/25 14:10, Fiona Ebner wrote:
>>>> Am 06.03.25 um 11:44 schrieb Dominik Csapak:
>>>>> 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?
>>>>
>>>
>>> I though the intention of the code was to pin the guest to that version
>>> where it
>>> was created. If we omit the machine version, this is not true anymore,
>>> since I'd then create a windows vm with e.g. pc-q35-9.2+pve1 then set it
>>> to q35, and would get pc-q35-9.2+pve0
>>
>> Fair enough. There might be people manually changing windows guests to
>> use the latest machine even if we don't recommend it or allow it in the
>> UI. Still, it would just mean a pve machine version downgrade for
>> subsequent boots, which also can happen if you offline migrate to a node
>> that does not yet support the newest pve machine version.
>>
>>>
>>> and while i don't have anymore use cases for the current case, wouldn't
>>> this approach here correct also when we decide to bump again in the
>>> future?
>>
>> Correct what?
>>
> 
> i wanted to write 'wouldn't this approach here be correct [..]'

I never said it's not correct. Just questioning if it's worth it.

>>> also recording with which pve version the vm was created could help in
>>> debugging
>>> issues at some point.. (my patch only adds the version in the meta info
>>> if it's > 0 anyway)
>>>
>>
>> In principle, yes. But honestly, it's much less informative than the
>> creation QEMU version, and even that is rather rarely useful for
>> debugging in my experience.
>>
>> So not a huge fan, since I'd find it nicer to keep the meta info slick,
>> but we can go for it if you really think it's worth it.
> 
> 
> mhmm i get what you mean, would it be an ok compromise to record the pve
> version with
> the creation machine maybe?
> 
> i'd have to touch the places where we read that ofc but that shouldn't
> be too many
> 

The creation-qemu is the binary version, but the latest pve version is
related to the qemu-server version/machine version, so while I get where
you're coming from, I'd rather keep them separate.


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