[libvirt] [PATCH v3 2/5] util: virhostcpu: factor out frequency parsing

[Andrea Bolognani]

From: Bjoern Walk <bwalk at linux.vnet.ibm.com>

All different architectures use the same copy-pasted code to parse
processor frequency information from /proc/cpuinfo. Let's extract that
code into a function to avoid repetition.

We now also tolerate if the parsing of /proc/cpuinfo is not successful
and just report a warning instead of bailing out and abandoning the rest
of the CPU information.

Reviewed-by: Marc Hartmayer <mhartmay at linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
Reviewed-by: Andrea Bolognani <abologna at redhat.com>
Signed-off-by: Bjoern Walk <bwalk at linux.vnet.ibm.com>
---
 src/util/virhostcpu.c | 141 ++++++++++++++++++++++----------------------------
 1 file changed, 62 insertions(+), 79 deletions(-)

diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index c485a9721..d47062013 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -508,6 +508,65 @@ virHostCPUHasValidSubcoreConfiguration(int threads_per_subcore)
     return ret;
 }
 
+static int
+virHostCPUParseFrequencyString(const char *str,
+                               const char *prefix,
+                               unsigned int *mhz)
+{
+    char *p;
+    unsigned int ui;
+
+    if (!STRPREFIX(str, prefix))
+        return 1;
+
+    str += strlen(prefix);
+
+    while (*str && c_isspace(*str))
+        str++;
+
+    if (*str != ':' || !str[1]) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("parsing cpu MHz from cpuinfo"));
+        return -1;
+    }
+
+    if (virStrToLong_ui(str + 1, &p, 10, &ui) == 0 &&
+        /* Accept trailing fractional part. */
+        (*p == '\0' || *p == '.' || c_isspace(*p)))
+        *mhz = ui;
+
+    return 0;
+}
+
+static int
+virHostCPUParseFrequency(FILE *cpuinfo,
+                         virArch arch,
+                         unsigned int *mhz)
+{
+    const char *prefix = NULL;
+    char line[1024];
+
+    if (ARCH_IS_X86(arch))
+        prefix = "cpu MHz";
+    else if (ARCH_IS_PPC(arch))
+        prefix = "clock";
+    else if (ARCH_IS_ARM(arch))
+        prefix = "BogoMIPS";
+
+    if (!prefix) {
+        VIR_WARN("Your architecture is not supported by the %s parser",
+                 CPUINFO_PATH);
+        return 1;
+    }
+
+    while (fgets(line, sizeof(line), cpuinfo) != NULL) {
+        if (virHostCPUParseFrequencyString(line, prefix, mhz) < 0)
+            return -1;
+    }
+
+    return 0;
+}
+
 int
 virHostCPUGetInfoPopulateLinux(FILE *cpuinfo,
                                virArch arch,
@@ -520,7 +579,6 @@ virHostCPUGetInfoPopulateLinux(FILE *cpuinfo,
 {
     virBitmapPtr present_cpus_map = NULL;
     virBitmapPtr online_cpus_map = NULL;
-    char line[1024];
     DIR *nodedir = NULL;
     struct dirent *nodedirent = NULL;
     int nodecpus, nodecores, nodesockets, nodethreads, offline = 0;
@@ -535,84 +593,9 @@ virHostCPUGetInfoPopulateLinux(FILE *cpuinfo,
     *cpus = *nodes = *sockets = *cores = *threads = 0;
 
     /* Start with parsing CPU clock speed from /proc/cpuinfo */
-    while (fgets(line, sizeof(line), cpuinfo) != NULL) {
-        if (ARCH_IS_X86(arch)) {
-            char *buf = line;
-            if (STRPREFIX(buf, "cpu MHz")) {
-                char *p;
-                unsigned int ui;
-
-                buf += 7;
-                while (*buf && c_isspace(*buf))
-                    buf++;
-
-                if (*buf != ':' || !buf[1]) {
-                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                   _("parsing cpu MHz from cpuinfo"));
-                    goto cleanup;
-                }
-
-                if (virStrToLong_ui(buf+1, &p, 10, &ui) == 0 &&
-                    /* Accept trailing fractional part.  */
-                    (*p == '\0' || *p == '.' || c_isspace(*p)))
-                    *mhz = ui;
-            }
-        } else if (ARCH_IS_PPC(arch)) {
-            char *buf = line;
-            if (STRPREFIX(buf, "clock")) {
-                char *p;
-                unsigned int ui;
-
-                buf += 5;
-                while (*buf && c_isspace(*buf))
-                    buf++;
-
-                if (*buf != ':' || !buf[1]) {
-                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                   _("parsing cpu MHz from cpuinfo"));
-                    goto cleanup;
-                }
-
-                if (virStrToLong_ui(buf+1, &p, 10, &ui) == 0 &&
-                    /* Accept trailing fractional part.  */
-                    (*p == '\0' || *p == '.' || c_isspace(*p)))
-                    *mhz = ui;
-                /* No other interesting infos are available in /proc/cpuinfo.
-                 * However, there is a line identifying processor's version,
-                 * identification and machine, but we don't want it to be caught
-                 * and parsed in next iteration, because it is not in expected
-                 * format and thus lead to error. */
-            }
-        } else if (ARCH_IS_ARM(arch)) {
-            char *buf = line;
-            if (STRPREFIX(buf, "BogoMIPS")) {
-                char *p;
-                unsigned int ui;
-
-                buf += 8;
-                while (*buf && c_isspace(*buf))
-                    buf++;
-
-                if (*buf != ':' || !buf[1]) {
-                    virReportError(VIR_ERR_INTERNAL_ERROR,
-                                   "%s", _("parsing cpu MHz from cpuinfo"));
-                    goto cleanup;
-                }
-
-                if (virStrToLong_ui(buf+1, &p, 10, &ui) == 0
-                    /* Accept trailing fractional part.  */
-                    && (*p == '\0' || *p == '.' || c_isspace(*p)))
-                    *mhz = ui;
-            }
-        } else if (ARCH_IS_S390(arch)) {
-            /* s390x has no realistic value for CPU speed,
-             * assign a value of zero to signify this */
-            *mhz = 0;
-        } else {
-            VIR_WARN("Parser for /proc/cpuinfo needs to be adapted for your architecture");
-            break;
-        }
-    }
+    if (virHostCPUParseFrequency(cpuinfo, arch, mhz) < 0)
+        VIR_WARN("Unable to parse CPU frequency information from %s",
+                 CPUINFO_PATH);
 
     /* Get information about what CPUs are present in the host and what
      * CPUs are online, so that we don't have to so for each node */
-- 
2.14.3