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

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

[Fabian Ebner]

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

[pve-devel] [PATCH qemu-server 2/3] fix #3301: status: add currently running machine and QEMU version to full status

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer.pm | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index a498444..e866faa 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2555,6 +2555,16 @@ our $vmstatus_return_properties = {
 	type => 'string',
 	optional => 1,
     },
+    'running-machine' => {
+	description => "The currently running machine type (if running).",
+	type => 'string',
+	optional => 1,
+    },
+    'running-qemu' => {
+	description => "The currently running QEMU version (if running).",
+	type => 'string',
+	optional => 1,
+    },
 };
 
 my $last_proc_pid_stat;
@@ -2733,10 +2743,32 @@ sub vmstatus {
 	$res->{$vmid}->{diskwrite} = $totalwrbytes;
     };
 
+    my $machinecb = sub {
+	my ($vmid, $resp) = @_;
+	my $data = $resp->{'return'} || [];
+
+	$res->{$vmid}->{'running-machine'} =
+	    PVE::QemuServer::Machine::current_from_query_machines($data);
+    };
+
+    my $versioncb = sub {
+	my ($vmid, $resp) = @_;
+	my $data = $resp->{'return'} // {};
+	my $version = 'unknown';
+
+	if (my $v = $data->{qemu}) {
+	    $version = $v->{major} . "." . $v->{minor} . "." . $v->{micro};
+	}
+
+	$res->{$vmid}->{'running-qemu'} = $version;
+    };
+
     my $statuscb = sub {
 	my ($vmid, $resp) = @_;
 
 	$qmpclient->queue_cmd($vmid, $blockstatscb, 'query-blockstats');
+	$qmpclient->queue_cmd($vmid, $machinecb, 'query-machines');
+	$qmpclient->queue_cmd($vmid, $versioncb, 'query-version');
 	# this fails if ballon driver is not loaded, so this must be
 	# the last commnand (following command are aborted if this fails).
 	$qmpclient->queue_cmd($vmid, $ballooncb, 'query-balloon');
-- 
2.20.1

[pve-devel] [PATCH manager 3/3] fix #3301: ui: show currently running QEMU version

[Fabian Ebner]

in the VM summary page.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

I felt like the running machine is less interesting to users, so I only
added the QEMU version. And I added it directly to the status as it doesn't
seem to fit badly.

Of course I can change it to be its own line and also add a line for the machine
if that's preferred.

 www/manager6/panel/GuestStatusView.js | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/www/manager6/panel/GuestStatusView.js b/www/manager6/panel/GuestStatusView.js
index 8dab4c19..84a854c3 100644
--- a/www/manager6/panel/GuestStatusView.js
+++ b/www/manager6/panel/GuestStatusView.js
@@ -40,6 +40,10 @@ Ext.define('PVE.panel.GuestStatusView', {
 		if (qmpstatus && qmpstatus !== record.data.status) {
 		    text += ' (' + qmpstatus + ')';
 		}
+		let qemuversion = record.data["running-qemu"];
+		if (qemuversion) {
+		    text += ' (Qemu: ' + qemuversion + ')';
+		}
 		return text;
 	    },
 	},
-- 
2.20.1

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

[Stefan Reiter]

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?

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

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

[Thomas Lamprecht]

On 01.03.21 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(-)
> 
>

applied, with Stefan's R-b, thanks!

[pve-devel] applied: [PATCH qemu-server 2/3] fix #3301: status: add currently running machine and QEMU version to full status

[Thomas Lamprecht]

On 01.03.21 16:53, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  PVE/QemuServer.pm | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
>

applied, with Stefan's R-b, thanks!

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

[Fabian Ebner]

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