[libvirt] [PATCH v3 02/24] qemu: monitor: Return struct from qemuMonitor(Text|Json)QueryCPUs

Peter Krempa pkrempa at redhat.com
Wed Aug 24 13:35:18 UTC 2016


Prepare to extract more data by returning a array of structs rather than
just an array of thread ids. Additionally report fatal errors separately
from qemu not being able to produce data.
---

Notes:
    v3:
    - fixed uninitialized variable in tests
    v2:
    - initialized ncpus in qemuMonitorTextQueryCPUs
    - fixed return value on alloc fail

 src/qemu/qemu_monitor.c      | 31 ++++++++++++------
 src/qemu/qemu_monitor.h      |  6 ++++
 src/qemu/qemu_monitor_json.c | 77 +++++++++++++++++++++++---------------------
 src/qemu/qemu_monitor_json.h |  3 +-
 src/qemu/qemu_monitor_text.c | 41 +++++++++++------------
 src/qemu/qemu_monitor_text.h |  3 +-
 tests/qemumonitorjsontest.c  | 39 +++++++++++++++-------
 7 files changed, 121 insertions(+), 79 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index aa3b176..65c32bd 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1666,6 +1666,16 @@ qemuMonitorCPUInfoFree(qemuMonitorCPUInfoPtr cpus,
     VIR_FREE(cpus);
 }

+void
+qemuMonitorQueryCpusFree(struct qemuMonitorQueryCpusEntry *entries,
+                         size_t nentries ATTRIBUTE_UNUSED)
+{
+    if (!entries)
+        return;
+
+    VIR_FREE(entries);
+}
+

 /**
  * qemuMonitorGetCPUInfo:
@@ -1686,7 +1696,8 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
                       size_t maxvcpus)
 {
     qemuMonitorCPUInfoPtr info = NULL;
-    int *pids = NULL;
+    struct qemuMonitorQueryCpusEntry *cpuentries = NULL;
+    size_t ncpuentries = 0;
     size_t i;
     int ret = -1;
     int rc;
@@ -1697,26 +1708,28 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon,
         return -1;

     if (mon->json)
-        rc = qemuMonitorJSONQueryCPUs(mon, &pids);
+        rc = qemuMonitorJSONQueryCPUs(mon, &cpuentries, &ncpuentries);
     else
-        rc = qemuMonitorTextQueryCPUs(mon, &pids);
+        rc = qemuMonitorTextQueryCPUs(mon, &cpuentries, &ncpuentries);

     if (rc < 0) {
-        virResetLastError();
-        VIR_STEAL_PTR(*vcpus, info);
-        ret = 0;
+        if (rc == -2) {
+            VIR_STEAL_PTR(*vcpus, info);
+            ret = 0;
+        }
+
         goto cleanup;
     }

-    for (i = 0; i < rc; i++)
-        info[i].tid = pids[i];
+    for (i = 0; i < ncpuentries; i++)
+        info[i].tid = cpuentries[i].tid;

     VIR_STEAL_PTR(*vcpus, info);
     ret = 0;

  cleanup:
     qemuMonitorCPUInfoFree(info, maxvcpus);
-    VIR_FREE(pids);
+    qemuMonitorQueryCpusFree(cpuentries, ncpuentries);
     return ret;
 }

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 3fa993f..826991f 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -390,6 +390,12 @@ int qemuMonitorGetStatus(qemuMonitorPtr mon,
 int qemuMonitorSystemReset(qemuMonitorPtr mon);
 int qemuMonitorSystemPowerdown(qemuMonitorPtr mon);

+struct qemuMonitorQueryCpusEntry {
+    pid_t tid;
+};
+void qemuMonitorQueryCpusFree(struct qemuMonitorQueryCpusEntry *entries,
+                              size_t nentries);
+

 struct _qemuMonitorCPUInfo {
     pid_t tid;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 1df1e4a..4f4fb4f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1323,69 +1323,69 @@ int qemuMonitorJSONSystemReset(qemuMonitorPtr mon)
  *   { "CPU": 1, "current": false, "halted": true, "pc": 7108165 } ]
  */
 static int
-qemuMonitorJSONExtractCPUInfo(virJSONValuePtr reply,
-                              int **pids)
+qemuMonitorJSONExtractCPUInfo(virJSONValuePtr data,
+                              struct qemuMonitorQueryCpusEntry **entries,
+                              size_t *nentries)
 {
-    virJSONValuePtr data;
+    struct qemuMonitorQueryCpusEntry *cpus = NULL;
     int ret = -1;
     size_t i;
-    int *threads = NULL;
     ssize_t ncpus;

-    if (!(data = virJSONValueObjectGetArray(reply, "return"))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("cpu reply was missing return data"));
-        goto cleanup;
-    }
-
-    if ((ncpus = virJSONValueArraySize(data)) <= 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("cpu information was empty"));
-        goto cleanup;
-    }
+    if ((ncpus = virJSONValueArraySize(data)) <= 0)
+        return -2;

-    if (VIR_ALLOC_N(threads, ncpus) < 0)
+    if (VIR_ALLOC_N(cpus, ncpus) < 0)
         goto cleanup;

     for (i = 0; i < ncpus; i++) {
         virJSONValuePtr entry = virJSONValueArrayGet(data, i);
-        int thread;
+        int thread = 0;
         if (!entry) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("cpu information was missing an array element"));
+            ret = -2;
             goto cleanup;
         }

-        if (virJSONValueObjectGetNumberInt(entry, "thread_id", &thread) < 0) {
-            /* Some older qemu versions don't report the thread_id,
-             * so treat this as non-fatal, simply returning no data */
-            ret = 0;
-            goto cleanup;
-        }
+        /* Some older qemu versions don't report the thread_id so treat this as
+         * non-fatal, simply returning no data */
+        ignore_value(virJSONValueObjectGetNumberInt(entry, "thread_id", &thread));

-        threads[i] = thread;
+        cpus[i].tid = thread;
     }

-    *pids = threads;
-    threads = NULL;
-    ret = ncpus;
+    VIR_STEAL_PTR(*entries, cpus);
+    *nentries = ncpus;
+    ret = 0;

  cleanup:
-    VIR_FREE(threads);
+    qemuMonitorQueryCpusFree(cpus, ncpus);
     return ret;
 }


+/**
+ * qemuMonitorJSONQueryCPUs:
+ *
+ * @mon: monitor object
+ * @entries: filled with detected entries on success
+ * @nentries: number of entries returned
+ *
+ * Queries qemu for cpu-related information. Failure to execute the command or
+ * extract results does not produce an error as libvirt can continue without
+ * this information.
+ *
+ * Returns 0 on success success, -1 on a fatal error (oom ...) and -2 if the
+ * query failed gracefully.
+ */
 int
 qemuMonitorJSONQueryCPUs(qemuMonitorPtr mon,
-                         int **pids)
+                         struct qemuMonitorQueryCpusEntry **entries,
+                         size_t *nentries)
 {
     int ret = -1;
-    virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-cpus",
-                                                     NULL);
+    virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-cpus", NULL);
     virJSONValuePtr reply = NULL;
-
-    *pids = NULL;
+    virJSONValuePtr data;

     if (!cmd)
         return -1;
@@ -1393,10 +1393,13 @@ qemuMonitorJSONQueryCPUs(qemuMonitorPtr mon,
     if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
         goto cleanup;

-    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+    if (!(data = virJSONValueObjectGetArray(reply, "return"))) {
+        ret = -2;
         goto cleanup;
+    }
+
+    ret = qemuMonitorJSONExtractCPUInfo(data, entries, nentries);

-    ret = qemuMonitorJSONExtractCPUInfo(reply, pids);
  cleanup:
     virJSONValueFree(cmd);
     virJSONValueFree(reply);
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 174f0ef..90fe697 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -59,7 +59,8 @@ int qemuMonitorJSONSystemPowerdown(qemuMonitorPtr mon);
 int qemuMonitorJSONSystemReset(qemuMonitorPtr mon);

 int qemuMonitorJSONQueryCPUs(qemuMonitorPtr mon,
-                             int **pids);
+                             struct qemuMonitorQueryCpusEntry **entries,
+                             size_t *nentries);
 int qemuMonitorJSONGetVirtType(qemuMonitorPtr mon,
                                virDomainVirtType *virtType);
 int qemuMonitorJSONUpdateVideoMemorySize(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index ca04965..ff7cc79 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -502,12 +502,15 @@ int qemuMonitorTextSystemReset(qemuMonitorPtr mon)

 int
 qemuMonitorTextQueryCPUs(qemuMonitorPtr mon,
-                         int **pids)
+                         struct qemuMonitorQueryCpusEntry **entries,
+                         size_t *nentries)
 {
     char *qemucpus = NULL;
     char *line;
-    pid_t *cpupids = NULL;
-    size_t ncpupids = 0;
+    struct qemuMonitorQueryCpusEntry *cpus = NULL;
+    size_t ncpus = 0;
+    struct qemuMonitorQueryCpusEntry cpu = {0};
+    int ret = -2; /* -2 denotes a non-fatal error to get the data */

     if (qemuMonitorHMPCommand(mon, "info cpus", &qemucpus) < 0)
         return -1;
@@ -529,15 +532,19 @@ qemuMonitorTextQueryCPUs(qemuMonitorPtr mon,

         /* Extract host Thread ID */
         if ((offset = strstr(line, "thread_id=")) == NULL)
-            goto error;
+            goto cleanup;

         if (virStrToLong_i(offset + strlen("thread_id="), &end, 10, &tid) < 0)
-            goto error;
+            goto cleanup;
         if (end == NULL || !c_isspace(*end))
-            goto error;
+            goto cleanup;

-        if (VIR_APPEND_ELEMENT_COPY(cpupids, ncpupids, tid) < 0)
-            goto error;
+        cpu.tid = tid;
+
+        if (VIR_APPEND_ELEMENT_COPY(cpus, ncpus, cpu) < 0) {
+            ret = -1;
+            goto cleanup;
+        }

         VIR_DEBUG("tid=%d", tid);

@@ -547,20 +554,14 @@ qemuMonitorTextQueryCPUs(qemuMonitorPtr mon,
             line = strchr(offset, '\n');
     } while (line != NULL);

-    /* Validate we got data for all VCPUs we expected */
-    VIR_FREE(qemucpus);
-    *pids = cpupids;
-    return ncpupids;
+    VIR_STEAL_PTR(*entries, cpus);
+    *nentries = ncpus;
+    ret = 0;

- error:
+ cleanup:
+    qemuMonitorQueryCpusFree(cpus, ncpus);
     VIR_FREE(qemucpus);
-    VIR_FREE(cpupids);
-
-    /* Returning 0 to indicate non-fatal failure, since
-     * older QEMU does not have VCPU<->PID mapping and
-     * we don't want to fail on that
-     */
-    return 0;
+    return ret;
 }


diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h
index b7f0cab..86f43e7 100644
--- a/src/qemu/qemu_monitor_text.h
+++ b/src/qemu/qemu_monitor_text.h
@@ -50,7 +50,8 @@ int qemuMonitorTextSystemPowerdown(qemuMonitorPtr mon);
 int qemuMonitorTextSystemReset(qemuMonitorPtr mon);

 int qemuMonitorTextQueryCPUs(qemuMonitorPtr mon,
-                             int **pids);
+                             struct qemuMonitorQueryCpusEntry **entries,
+                             size_t *nentries);
 int qemuMonitorTextGetVirtType(qemuMonitorPtr mon,
                                virDomainVirtType *virtType);
 int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon,
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 9988754..242544d 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1201,6 +1201,16 @@ GEN_TEST_FUNC(qemuMonitorJSONNBDServerStart, "localhost", 12345)
 GEN_TEST_FUNC(qemuMonitorJSONNBDServerAdd, "vda", true)
 GEN_TEST_FUNC(qemuMonitorJSONDetachCharDev, "serial1")

+static bool
+testQemuMonitorJSONqemuMonitorJSONQueryCPUsEqual(struct qemuMonitorQueryCpusEntry *a,
+                                                 struct qemuMonitorQueryCpusEntry *b)
+{
+    if (a->tid != b->tid)
+        return false;
+
+    return true;
+}
+

 static int
 testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void *data)
@@ -1208,9 +1218,14 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void *data)
     virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
     qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
     int ret = -1;
-    pid_t *cpupids = NULL;
-    pid_t expected_cpupids[] = {17622, 17624, 17626, 17628};
-    int ncpupids;
+    struct qemuMonitorQueryCpusEntry *cpudata = NULL;
+    struct qemuMonitorQueryCpusEntry expect[] = {
+        {17622},
+        {17624},
+        {17626},
+        {17628},
+    };
+    size_t ncpudata = 0;
     size_t i;

     if (!test)
@@ -1252,19 +1267,21 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void *data)
                                "}") < 0)
         goto cleanup;

-    ncpupids = qemuMonitorJSONQueryCPUs(qemuMonitorTestGetMonitor(test), &cpupids);
+    if (qemuMonitorJSONQueryCPUs(qemuMonitorTestGetMonitor(test),
+                                 &cpudata, &ncpudata) < 0)
+        goto cleanup;

-    if (ncpupids != 4) {
+    if (ncpudata != 4) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       "Expecting ncpupids = 4 but got %d", ncpupids);
+                       "Expecting ncpupids = 4 but got %zu", ncpudata);
         goto cleanup;
     }

-    for (i = 0; i < ncpupids; i++) {
-        if (cpupids[i] != expected_cpupids[i]) {
+    for (i = 0; i < ncpudata; i++) {
+        if (!testQemuMonitorJSONqemuMonitorJSONQueryCPUsEqual(cpudata + i,
+                                                              expect + i)) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
-                           "Expecting cpupids[%zu] = %d but got %d",
-                           i, expected_cpupids[i], cpupids[i]);
+                           "vcpu entry %zu does not match expected data", i);
             goto cleanup;
         }
     }
@@ -1272,7 +1289,7 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void *data)
     ret = 0;

  cleanup:
-    VIR_FREE(cpupids);
+    qemuMonitorQueryCpusFree(cpudata, ncpudata);
     qemuMonitorTestFree(test);
     return ret;
 }
-- 
2.8.2




More information about the libvir-list mailing list