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

[Dominik Csapak]

On 3/6/25 15:20, Fiona Ebner wrote:
> 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.

Ok, I'll send a new version without recording the PVE version