From b3180425ce0ac01948cd997d56b4af21a5380438 Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Mon, 1 Aug 2016 13:44:25 +0200 Subject: [PATCH] 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. --- 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 aa3b176d01..65c32bdb19 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 3fa993f37b..826991f36c 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 1df1e4a4f0..4f4fb4fac3 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) + 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; @@ -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 174f0ef2c0..90fe697f35 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 ca04965f91..ff7cc79fda 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 b7f0cab4ee..86f43e7c55 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 9988754869..242544d672 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; }