From ea8dd0fe977164f2530c8e87c31c2adb732d4239 Mon Sep 17 00:00:00 2001

Message-Id: <ea8dd0fe977164f2530c8e87c31c2adb732d4239@dist-git>

From: Peter Krempa <pkrempa@redhat.com>

Date: Wed, 24 Aug 2016 16:11:18 -0400

Subject: [PATCH] qemu: monitor: Return struct from

 qemuMonitor(Text|Json)QueryCPUs

https://bugzilla.redhat.com/show_bug.cgi?id=1097930

https://bugzilla.redhat.com/show_bug.cgi?id=1224341

Prepare to extract more data by returning an array of structs rather than

just an array of thread ids. Additionally report fatal errors separately

from qemu not being able to produce data.

(cherry picked from commit b3180425ce0ac01948cd997d56b4af21a5380438)

---

 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 c3b9f41..d5af7d1 100644

--- a/src/qemu/qemu_monitor.c

+++ b/src/qemu/qemu_monitor.c

@@ -1661,6 +1661,16 @@ qemuMonitorCPUInfoFree(qemuMonitorCPUInfoPtr cpus,

     VIR_FREE(cpus);

 }

+void

+qemuMonitorQueryCpusFree(struct qemuMonitorQueryCpusEntry *entries,

+                         size_t nentries ATTRIBUTE_UNUSED)

+{

+    if (!entries)

+        return;

+

+    VIR_FREE(entries);

+}

+

 /**

  * qemuMonitorGetCPUInfo:

@@ -1681,7 +1691,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;

@@ -1692,26 +1703,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 3ab3f08..5ec2101 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 a45772e..5836e1e 100644

--- a/src/qemu/qemu_monitor_json.c

+++ b/src/qemu/qemu_monitor_json.c

@@ -1329,69 +1329,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)

+        return -2;

-    if ((ncpus = virJSONValueArraySize(data)) <= 0) {

-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

-                       _("cpu information was empty"));

-        goto cleanup;

-    }

-

-    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;

@@ -1399,10 +1399,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 ef198b4..2a439da 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 0b05587..e29acfc 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 1d245d0..3fd4eb6 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.10.0