[PATCH v2 2/3] virprocess: Make virProcessGetStatInfo() fail if unable to parse data

Michal Privoznik mprivozn at redhat.com
Wed Jan 18 09:58:18 UTC 2023


Yeah, we've already seen this commit (v8.0.0-rc2~4) and also its
revert (v8.1.0-rc1~345). While the original idea was sound, the
implementation was less so and it changed behaviour of some
public APIs (e.g. whilst getting stats for a running guest was
best effort it started to return errors).

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/qemu/qemu_driver.c | 11 ++++-------
 src/util/virprocess.c  | 28 +++++++---------------------
 2 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c576c601ad..25c681bfd2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2508,13 +2508,10 @@ qemuDomainGetInfo(virDomainPtr dom,
     }
 
     if (virDomainObjIsActive(vm)) {
-        if (virProcessGetStatInfo(&(info->cpuTime), NULL, NULL,
-                                  NULL, NULL,
-                                  vm->pid, 0) < 0) {
-            virReportError(VIR_ERR_OPERATION_FAILED, "%s",
-                           _("cannot read cputime for domain"));
-            goto cleanup;
-        }
+        /* Specifically ignore the error here. QEMU's PID might be already gone
+         * even though the check above says it's still active. */
+        virProcessGetStatInfo(&(info->cpuTime), NULL,
+                              NULL, NULL, NULL, vm->pid, 0);
     }
 
     if (VIR_ASSIGN_IS_OVERFLOW(info->nrVirtCpu, virDomainDefGetVcpus(vm->def))) {
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 39ca5de811..800af29537 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -1761,7 +1761,7 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
         virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, &stime) < 0 ||
         virStrToLong_l(proc_stat[VIR_PROCESS_STAT_RSS], NULL, 10, &rss) < 0 ||
         virStrToLong_i(proc_stat[VIR_PROCESS_STAT_PROCESSOR], NULL, 10, &cpu) < 0) {
-        VIR_WARN("cannot parse process status data");
+        return -1;
     }
 
     utime *= jiff2nsec;
@@ -1778,7 +1778,6 @@ virProcessGetStatInfo(unsigned long long *cpuTime,
     if (vm_rss)
         *vm_rss = rss * virGetSystemPageSizeKB();
 
-
     VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d rss=%ld",
               (int) pid, tid, utime, stime, cpu, rss);
 
@@ -1852,28 +1851,15 @@ virProcessGetSchedInfo(unsigned long long *cpuWait,
 
 #else
 int
-virProcessGetStatInfo(unsigned long long *cpuTime,
-                      unsigned long long *userTime,
-                      unsigned long long *sysTime,
-                      int *lastCpu,
-                      long *vm_rss,
+virProcessGetStatInfo(unsigned long long *cpuTime G_GNUC_UNUSED,
+                      unsigned long long *userTime G_GNUC_UNUSED,
+                      unsigned long long *sysTime G_GNUC_UNUSED,
+                      int *lastCpu G_GNUC_UNUSED,
+                      long *vm_rss G_GNUC_UNUSED,
                       pid_t pid G_GNUC_UNUSED,
                       pid_t tid G_GNUC_UNUSED)
 {
-    /* We don't have a way to collect this information on non-Linux
-     * platforms, so just report neutral values */
-    if (cpuTime)
-        *cpuTime = 0;
-    if (userTime)
-        *userTime = 0;
-    if (sysTime)
-        *sysTime = 0;
-    if (lastCpu)
-        *lastCpu = 0;
-    if (vm_rss)
-        *vm_rss = 0;
-
-    return 0;
+    return -1;
 }
 
 int
-- 
2.38.2



More information about the libvir-list mailing list