On 08/03/2016 04:11 AM, Peter Krempa wrote:
The function will gradually add more returned data. Return a struct for every vCPU containing the data. --- src/qemu/qemu_domain.c | 26 ++++++++++------------- src/qemu/qemu_monitor.c | 56 ++++++++++++++++++++++++++++++++++++++++++------- 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 4cdd012..9d389f7 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,31 +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; }
FWIW: Once we reach this point the VIR_DOMAIN_VIRT_QEMU check is the only way qemuDomainHasVcpuPids can return 0 in qemuDomainValidateVcpuInfo... So I wonder - would it be useful to alter that function too so that we're sure things match up properly vis-a-vis the 'tid' and online checks? And my alter, I was thinking along the lines of a similar check for VIR_DOMAIN_VIRT_QEMU...
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 4403bdd..0011ceb 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1656,25 +1656,67 @@ 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) {
If qemuMonitorJSONExtractCPUInfo() finds ncpus == 0, then it returns an error which won't be "cleaned up" unless rc <= 0
+ virResetLastError(); + ret = 0; + goto cleanup;
So, ret == 0, we go to cleanup, it cleans up 'info', then our caller finds rc == 0, so it can fall into that for maxvcpus loop looking 'info' and then of course freeing 'info' again.
+ } + + for (i = 0; i < rc; i++) + info[i].tid = pids[i];
Still no "gaps" in our "internal" pids list which is fine, but I assume could change in the future... Just keeping it fresh in my mind ;-) ACK with the necessary adjustments for the above rc check condition which I think would be along the lines of: if (rc <= 0) virResetLastError(); if (rc < 0) goto cleanup; But I could be wrong... John
+ + VIR_STEAL(*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 805656b..1ef002a 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,