[pve-devel] [PATCH-SERIES v2 qemu-server 0/2] fix #6207: vm status fixes

[pve-devel] [PATCH-SERIES v2 qemu-server 0/2] fix #6207: vm status fixes

[Fiona Ebner]

Changes in v2 (many thanks to Thomas for the discussion!):
* New approach, return undef instead of caching previous value.
* Add new patch to improve how 'query-proxmox-support' QMP commands
  are issued.

If disk read/write cannot be queried because of QMP timeout, they
should not be reported as 0, because a consumer of the RRD stats
cannot distinguish between 0 being an actual 0 value and 0 being an
indicator for the absence of the real value. The RRD graphs in the UI
will already show this correctly.

Additionally, the 'query-proxmox-support' command was not yet part of
the QMP queue, but done sequentially with a timeout of 5 seconds each,
which is not acceptable for pvestatd. The second patch fixes that.

Fiona Ebner (2):
  fix #6207: vm status: return undef values when disk{read,write} cannot
    be queried
  vm status: also queue query-proxmox-support QMP commands

 src/PVE/QemuServer.pm | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

-- 
2.47.3



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

[pve-devel] [PATCH v2 qemu-server 1/2] fix #6207: vm status: return undef values when disk{read, write} cannot be queried

[Fiona Ebner]

If disk read/write cannot be queried because of QMP timeout, they
should not be reported as 0, because a consumer of the RRD stats
cannot distinguish between 0 being an actual 0 value and 0 being an
indicator for the absence of the real value. The RRD graphs in the UI
will already show this correctly.

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

Changes in v2:
* New approach, return undef instead of caching previous value.

 src/PVE/QemuServer.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index f7f85436..d6d0cb13 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -2731,8 +2731,8 @@ sub vmstatus {
         $d->{netout} = 0;
         $d->{netin} = 0;
 
-        $d->{diskread} = 0;
-        $d->{diskwrite} = 0;
+        $d->{diskread} = undef;
+        $d->{diskwrite} = undef;
 
         $d->{template} = 1 if PVE::QemuConfig->is_template($conf);
 
-- 
2.47.3



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

[pve-devel] [PATCH qemu-server 2/2] vm status: also queue query-proxmox-support QMP commands

[Fiona Ebner]

The vmstatus() function is used by pvestatd and needs to be fast.
However, the 'query-proxmox-support' querying is done sequentially for
each VM and each query has its own timeout (it's the default 5
seconds). If QMP is blocked for some reason for a single VM, that
already adds 5 seconds to the whole operation. Compared with the whole
stats querying queue, which is allowed to use 3 seconds in total, this
is rather extreme and needs to be fixed.

Back when commit 6891fd70 ("print query-proxmox-support result in
'full' status") was implemented, not all supported QEMU versions in
Proxmox VE implemented the 'query-proxmox-support' QMP command.
Because of this, the queue might be interrupted if ordering this
command too early. It still could've been ordered before the
'query-balloon' one, which also can fail. Nowadays, all supported QEMU
versions do implement the command and this just returns static
information which cannot fail (as long as QMP communication itself
works), so it can also be ordered at the beginning of the queue (after
the main 'query-status').

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

New in v2.

 src/PVE/QemuServer.pm | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index d6d0cb13..7d5ab718 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -2886,9 +2886,15 @@ sub vmstatus {
         $res->{$vmid}->{'running-qemu'} = $version;
     };
 
+    my $proxmox_support_cb = sub {
+        my ($vmid, $resp) = @_;
+        $res->{$vmid}->{'proxmox-support'} = $resp->{'return'} // {};
+    };
+
     my $statuscb = sub {
         my ($vmid, $resp) = @_;
 
+        $qmpclient->queue_cmd($vmid, $proxmox_support_cb, 'query-proxmox-support');
         $qmpclient->queue_cmd($vmid, $blockstatscb, 'query-blockstats');
         $qmpclient->queue_cmd($vmid, $machinecb, 'query-machines');
         $qmpclient->queue_cmd($vmid, $versioncb, 'query-version');
@@ -2913,16 +2919,6 @@ sub vmstatus {
 
     $qmpclient->queue_execute(undef, 2);
 
-    foreach my $vmid (keys %$list) {
-        next if $opt_vmid && ($vmid ne $opt_vmid);
-        next if !$res->{$vmid}->{pid}; #not running
-
-        # we can't use the $qmpclient since it might have already aborted on
-        # 'query-balloon', but this might also fail for older versions...
-        my $qemu_support = eval { mon_cmd($vmid, "query-proxmox-support") };
-        $res->{$vmid}->{'proxmox-support'} = $qemu_support // {};
-    }
-
     foreach my $vmid (keys %$list) {
         next if $opt_vmid && ($vmid ne $opt_vmid);
         $res->{$vmid}->{qmpstatus} = $res->{$vmid}->{status} if !$res->{$vmid}->{qmpstatus};
-- 
2.47.3



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

[pve-devel] applied: [PATCH-SERIES v2 qemu-server 0/2] fix #6207: vm status fixes

[Thomas Lamprecht]

On Thu, 25 Sep 2025 14:25:07 +0200, Fiona Ebner wrote:
> Changes in v2 (many thanks to Thomas for the discussion!):
> * New approach, return undef instead of caching previous value.
> * Add new patch to improve how 'query-proxmox-support' QMP commands
>   are issued.
> 
> If disk read/write cannot be queried because of QMP timeout, they
> should not be reported as 0, because a consumer of the RRD stats
> cannot distinguish between 0 being an actual 0 value and 0 being an
> indicator for the absence of the real value. The RRD graphs in the UI
> will already show this correctly.
> 
> [...]

Applied, thanks!

btw., while technically correct to have only patch 1/2 marked as v2 as patch
2/2, I'd rather see the versioning per series not per patch, b4 e.g. also gets
confused this way as it cannot find the v2 of patch 2/2.

[1/2] fix #6207: vm status: return undef values when disk{read, write} cannot be queried
      commit: 528df523161aa424cc6dca69230eae1f9a503cef
[2/2] vm status: also queue query-proxmox-support QMP commands
      commit: 31d6f5f63bd6edb6c43de3e0213d38199c350cd4


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