8:21 a.m.
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,