[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: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 [..]'



>>
>> 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