[libvirt] [PATCH v2 0/3] New vCPU hotplug prequel
Few of the patches needed more significant changes. Peter Krempa (3): internal: Introduce macro for stealing pointers qemu: monitor: Return structures from qemuMonitorGetCPUInfo qemu: monitor: Return struct from qemuMonitor(Text|Json)QueryCPUs src/internal.h | 12 +++++++ src/qemu/qemu_domain.c | 25 ++++++-------- src/qemu/qemu_monitor.c | 70 ++++++++++++++++++++++++++++++++++++---- src/qemu/qemu_monitor.h | 19 ++++++++++- src/qemu/qemu_monitor_json.c | 77 +++++++++++++++++++++++--------------------- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_monitor_text.c | 39 +++++++++++----------- src/qemu/qemu_monitor_text.h | 3 +- tests/qemumonitorjsontest.c | 39 +++++++++++++++------- 9 files changed, 194 insertions(+), 93 deletions(-) -- 2.9.2
VIR_STEAL_PTR copies the pointer from the second argument into the first argument and then sets the second to NULL. --- src/internal.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/internal.h b/src/internal.h index 0dc34c7..d8cc5ad 100644 --- a/src/internal.h +++ b/src/internal.h @@ -307,6 +307,18 @@ } while (0) /** + * VIR_STEAL_PTR: + * + * Steals pointer passed as second argument into the first argument. Second + * argument must not have side effects. + */ +# define VIR_STEAL_PTR(a, b) \ + do { \ + (a) = (b); \ + (b) = NULL; \ + } while (0) + +/** * virCheckFlags: * @supported: an OR'ed set of supported flags * @retval: return value in case unsupported flags were passed -- 2.9.2
The function will gradually add more returned data. Return a struct for every vCPU containing the data. --- src/qemu/qemu_domain.c | 25 +++++++++------------- src/qemu/qemu_monitor.c | 57 +++++++++++++++++++++++++++++++++++++++++++------ src/qemu/qemu_monitor.h | 13 ++++++++++- 3 files changed, 72 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index bb6f21e..eeac767 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5682,10 +5682,11 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, int asyncJob) { virDomainVcpuDefPtr vcpu; + qemuDomainVcpuPrivatePtr vcpupriv; + qemuMonitorCPUInfoPtr info = NULL; size_t maxvcpus = virDomainDefGetVcpusMax(vm->def); - pid_t *cpupids = NULL; - int ncpupids; size_t i; + int rc; int ret = -1; /* @@ -5721,32 +5722,26 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1; - ncpupids = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), &cpupids); + + rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), &info, maxvcpus); + if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; - /* failure to get the VCPU <-> PID mapping or to execute the query - * command will not be treated fatal as some versions of qemu don't - * support this command */ - if (ncpupids <= 0) { - virResetLastError(); - ret = 0; + if (rc < 0) goto cleanup; - } for (i = 0; i < maxvcpus; i++) { vcpu = virDomainDefGetVcpu(vm->def, i); + vcpupriv = QEMU_DOMAIN_VCPU_PRIVATE(vcpu); - if (i < ncpupids) - QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->tid = cpupids[i]; - else - QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->tid = 0; + vcpupriv->tid = info[i].tid; } ret = 0; cleanup: - VIR_FREE(cpupids); + qemuMonitorCPUInfoFree(info, maxvcpus); return ret; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 83e1272..285beae 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1656,25 +1656,68 @@ qemuMonitorSystemReset(qemuMonitorPtr mon) } +void +qemuMonitorCPUInfoFree(qemuMonitorCPUInfoPtr cpus, + size_t ncpus ATTRIBUTE_UNUSED) +{ + if (!cpus) + return; + + VIR_FREE(cpus); +} + + /** * qemuMonitorGetCPUInfo: * @mon: monitor - * @pids: returned array of thread ids corresponding to the vCPUs + * @cpus: pointer filled by array of qemuMonitorCPUInfo structures + * @maxvcpus: total possible number of vcpus + * + * Detects VCPU information. If qemu doesn't support or fails reporting + * information this function will return success as other parts of libvirt + * are able to cope with that. * - * Detects the vCPU thread ids. Returns count of detected vCPUs on success, - * 0 if qemu didn't report thread ids (does not report libvirt error), - * -1 on error (reports libvirt error). + * Returns 0 on success (including if qemu didn't report any data) and + * -1 on error (reports libvirt error). */ int qemuMonitorGetCPUInfo(qemuMonitorPtr mon, - int **pids) + qemuMonitorCPUInfoPtr *vcpus, + size_t maxvcpus) { + qemuMonitorCPUInfoPtr info = NULL; + int *pids = NULL; + size_t i; + int ret = -1; + int rc; + QEMU_CHECK_MONITOR(mon); + if (VIR_ALLOC_N(info, maxvcpus) < 0) + return -1; + if (mon->json) - return qemuMonitorJSONQueryCPUs(mon, pids); + rc = qemuMonitorJSONQueryCPUs(mon, &pids); else - return qemuMonitorTextQueryCPUs(mon, pids); + rc = qemuMonitorTextQueryCPUs(mon, &pids); + + if (rc < 0) { + virResetLastError(); + VIR_STEAL_PTR(*vcpus, info); + ret = 0; + goto cleanup; + } + + for (i = 0; i < rc; i++) + info[i].tid = pids[i]; + + VIR_STEAL_PTR(*vcpus, info); + ret = 0; + + cleanup: + qemuMonitorCPUInfoFree(info, maxvcpus); + VIR_FREE(pids); + return ret; } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 591d3ed..3fa993f 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -390,8 +390,19 @@ int qemuMonitorGetStatus(qemuMonitorPtr mon, int qemuMonitorSystemReset(qemuMonitorPtr mon); int qemuMonitorSystemPowerdown(qemuMonitorPtr mon); + +struct _qemuMonitorCPUInfo { + pid_t tid; +}; +typedef struct _qemuMonitorCPUInfo qemuMonitorCPUInfo; +typedef qemuMonitorCPUInfo *qemuMonitorCPUInfoPtr; + +void qemuMonitorCPUInfoFree(qemuMonitorCPUInfoPtr list, + size_t nitems); int qemuMonitorGetCPUInfo(qemuMonitorPtr mon, - int **pids); + qemuMonitorCPUInfoPtr *vcpus, + size_t maxvcpus); + int qemuMonitorGetVirtType(qemuMonitorPtr mon, virDomainVirtType *virtType); int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon, -- 2.9.2
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. --- 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 | 39 +++++++++++----------- src/qemu/qemu_monitor_text.h | 3 +- tests/qemumonitorjsontest.c | 39 +++++++++++++++------- 7 files changed, 119 insertions(+), 79 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 285beae..f25c68d 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 d455adf..f39cda4 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..ef325ac 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; + 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,17 @@ 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) + goto cleanup; VIR_DEBUG("tid=%d", tid); @@ -547,20 +552,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 544b569..d6975ec 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; 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.9.2
On 08/04/2016 06:40 AM, Peter Krempa wrote:
Few of the patches needed more significant changes.
Peter Krempa (3): internal: Introduce macro for stealing pointers qemu: monitor: Return structures from qemuMonitorGetCPUInfo qemu: monitor: Return struct from qemuMonitor(Text|Json)QueryCPUs
src/internal.h | 12 +++++++ src/qemu/qemu_domain.c | 25 ++++++-------- src/qemu/qemu_monitor.c | 70 ++++++++++++++++++++++++++++++++++++---- src/qemu/qemu_monitor.h | 19 ++++++++++- src/qemu/qemu_monitor_json.c | 77 +++++++++++++++++++++++--------------------- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_monitor_text.c | 39 +++++++++++----------- src/qemu/qemu_monitor_text.h | 3 +- tests/qemumonitorjsontest.c | 39 +++++++++++++++------- 9 files changed, 194 insertions(+), 93 deletions(-)
ACK series with the following caveats NIT: There's only a 1 line gap between qemuMonitorCPUInfoFree and qemuMonitorQueryCpusFree instead of the normal 2... In patch 2, once info allocation is successful, both error paths will steal info and neither can return -1. By patch 3 things are adjusted such that -1 can be returned from the JSON/Text query functions since the -2 status is added to denote 'non-fatal'. IOW: For 1 patch we have a situation where something is different. So if you're fine with the 1 patch oddity - fine... Or you could merge the two together in order to avoid the extra work to add -2 to patch 2. John
participants (2)
-
John Ferlan -
Peter Krempa