[PATCH v3 2/2] Use virProcessGetStat

Martin Kletzander mkletzan at redhat.com
Tue Nov 23 09:48:06 UTC 2021


This eliminates one incorrect parsing implementation which relied on the
command field not having a closing bracket.  This possibility is already
tested against in the virProcessGetStat() tests.

Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
---
 src/qemu/qemu_driver.c | 34 ++++++------------------------
 src/util/virprocess.c  | 48 ++++++------------------------------------
 2 files changed, 13 insertions(+), 69 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d954635dde2a..16d449913ca7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1399,36 +1399,18 @@ qemuGetSchedInfo(unsigned long long *cpuWait,
 
 static int
 qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss,
-                   pid_t pid, int tid)
+                   pid_t pid, pid_t tid)
 {
-    g_autofree char *proc = NULL;
-    FILE *pidinfo;
+    g_auto(GStrv) proc_stat = virProcessGetStat(pid, tid);
     unsigned long long usertime = 0, systime = 0;
     long rss = 0;
     int cpu = 0;
 
-    /* In general, we cannot assume pid_t fits in int; but /proc parsing
-     * is specific to Linux where int works fine.  */
-    if (tid)
-        proc = g_strdup_printf("/proc/%d/task/%d/stat", (int)pid, tid);
-    else
-        proc = g_strdup_printf("/proc/%d/stat", (int)pid);
-    if (!proc)
-        return -1;
-
-    pidinfo = fopen(proc, "r");
-
-    /* See 'man proc' for information about what all these fields are. We're
-     * only interested in a very few of them */
-    if (!pidinfo ||
-        fscanf(pidinfo,
-               /* pid -> stime */
-               "%*d (%*[^)]) %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu %llu"
-               /* cutime -> endcode */
-               "%*d %*d %*d %*d %*d %*d %*u %*u %ld %*u %*u %*u"
-               /* startstack -> processor */
-               "%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d",
-               &usertime, &systime, &rss, &cpu) != 4) {
+    if (!proc_stat ||
+        virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_UTIME], NULL, 10, &usertime) < 0 ||
+        virStrToLong_ullp(proc_stat[VIR_PROCESS_STAT_STIME], NULL, 10, &systime) < 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");
     }
 
@@ -1450,8 +1432,6 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss,
     VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d rss=%ld",
               (int)pid, tid, usertime, systime, cpu, rss);
 
-    VIR_FORCE_FCLOSE(pidinfo);
-
     return 0;
 }
 
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index aab51962acfc..20d04bad6f26 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -1153,56 +1153,20 @@ virProcessSetMaxCoreSize(pid_t pid G_GNUC_UNUSED,
 int virProcessGetStartTime(pid_t pid,
                            unsigned long long *timestamp)
 {
-    char *tmp;
-    int len;
-    g_autofree char *filename = NULL;
-    g_autofree char *buf = NULL;
-    g_auto(GStrv) tokens = NULL;
-
-    filename = g_strdup_printf("/proc/%llu/stat", (long long)pid);
-
-    if ((len = virFileReadAll(filename, 1024, &buf)) < 0)
-        return -1;
+    g_auto(GStrv) proc_stat = virProcessGetStat(pid, 0);
 
-    /* start time is the token at index 19 after the '(process name)' entry - since only this
-     * field can contain the ')' character, search backwards for this to avoid malicious
-     * processes trying to fool us
-     */
-
-    if (!(tmp = strrchr(buf, ')'))) {
+    if (!proc_stat || g_strv_length(proc_stat) < 22) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Cannot find start time in %s"),
-                       filename);
+                       _("Cannot find start time for pid %d"), (int)pid);
         return -1;
     }
-    tmp += 2; /* skip ') ' */
-    if ((tmp - buf) >= len) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Cannot find start time in %s"),
-                       filename);
-        return -1;
-    }
-
-    tokens = g_strsplit(tmp, " ", 0);
 
-    if (!tokens ||
-        g_strv_length(tokens) < 20) {
+    if (virStrToLong_ull(proc_stat[21], NULL, 10, timestamp) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Cannot find start time in %s"),
-                       filename);
+                       _("Cannot parse start time %s for pid %d"),
+                       proc_stat[21], (int)pid);
         return -1;
     }
-
-    if (virStrToLong_ull(tokens[19],
-                         NULL,
-                         10,
-                         timestamp) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Cannot parse start time %s in %s"),
-                       tokens[19], filename);
-        return -1;
-    }
-
     return 0;
 }
 #elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
-- 
2.34.0




More information about the libvir-list mailing list