qemu: monitor: Return struct from qemuMonitor(Text|Json)QueryCPUs

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.

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index aa3b176d01ca6db9e66d16525b40f8b18eb21442..65c32bdb193da71278f32073e081a883e279e702 100644 (file)
--- 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 3fa993f37b588c1cff2796f48fd9ad983e33122f..826991f36cabddce91668037a971ce43769b4c8f 100644 (file)
--- 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 1df1e4a4f02334d7050a9692c980662e39ea776f..4f4fb4fac3b8912dd32411412a0073ac07611ff7 100644 (file)
--- 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 174f0ef2c05c39ba085329f8b37b62735da1897f..90fe697f35cc33a6256610717a3c2bf097e4a855 100644 (file)
--- 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 ca04965f915743de98eed9fb061077d810575f2a..ff7cc79fda4bcdbc654410da21b82fc8a016f644 100644 (file)
--- 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 b7f0cab4eedb657bde35a46d286bcf1fab1f45fd..86f43e7c55e27cca7297ff0f9cbe3429bc5834d2 100644 (file)
--- 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 99887548693f2208fe77c6de1822c202bd55ccdd..242544d672706c626b9fed400ea963cea2585789 100644 (file)
--- 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;
 }