Re: [pve-devel] [PATCH qemu-server 1/3] machine: split out helper for handling query-machines qmp command result

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

On 04.03.21 13:51, Stefan Reiter wrote:
> LGTM, for both qemu-server patches:
> 
> Reviewed-by: Stefan Reiter <s.reiter@proxmox.com>
> 
> Not sure about the formatting in the GUI, but I'm also the wrong person 
> to ask there. Maybe don't capitalize "Qemu", as we also don't do that 
> for "running"/"stopped"/... either?
> 

Thanks for the review! I agree that the "Qemu" looks a bit out of place, 
and even with lower-case "qemu" it doesn't feel completely right to me 
now (using "QEMU" feels slightly better). I'll send a v2 with the 
version as a separate field, hopefully that's better.

> On 01/03/2021 16:53, Fabian Ebner wrote:
>> to be re-used in the vmstatus() call.
>>
>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>> ---
>>   PVE/QemuServer/Machine.pm | 16 +++++++++++-----
>>   1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
>> index c168ade..2474951 100644
>> --- a/PVE/QemuServer/Machine.pm
>> +++ b/PVE/QemuServer/Machine.pm
>> @@ -18,11 +18,8 @@ sub machine_type_is_q35 {
>>       return $conf->{machine} && ($conf->{machine} =~ m/q35/) ? 1 : 0;
>>   }
>> -# this only works if VM is running
>> -sub get_current_qemu_machine {
>> -    my ($vmid) = @_;
>> -
>> -    my $res = PVE::QemuServer::Monitor::mon_cmd($vmid, 
>> 'query-machines');
>> +sub current_from_query_machines {
>> +    my ($res) = @_;
>>       my ($current, $pve_version, $default);
>>       foreach my $e (@$res) {
>> @@ -37,6 +34,15 @@ sub get_current_qemu_machine {
>>       return $current || $default || 'pc';
>>   }
>> +# this only works if VM is running
>> +sub get_current_qemu_machine {
>> +    my ($vmid) = @_;
>> +
>> +    my $res = PVE::QemuServer::Monitor::mon_cmd($vmid, 
>> 'query-machines');
>> +
>> +    return current_from_query_machines($res);
>> +}
>> +
>>   # returns a string with major.minor+pve<VERSION>, patch version-part 
>> is ignored
>>   # as it's seldom ressembling a real QEMU machine type, so it would 
>> be '0' 99% of
>>   # the time anyway.. This explicitly separates pveversion from the 
>> machine.
>>