On 09/12/14 13:48, Francesco Romani wrote:
Future patches which will implement more bulk stats groups for QEMU will need to access the connection object.
To accomodate that, a few changes are needed:
* enrich internal prototype to pass qemu driver object. * add per-group flag to mark if one collector needs monitor access or not. * if at least one collector of the requested stats needs monitor access, thus we must start a query job for each domain. The specific collectors will run nested monitor jobs inside that. * although requested, monitor could be not available. pass a flag to workers to signal the availability of monitor, in order to gather as much data as is possible anyway.
Signed-off-by: Francesco Romani <fromani@redhat.com> --- src/qemu/qemu_driver.c | 60 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 917b286..279c8b3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17244,7 +17244,8 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
static int -qemuDomainGetStatsState(virDomainObjPtr dom, +qemuDomainGetStatsState(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr dom, virDomainStatsRecordPtr record, int *maxparams, unsigned int privflags ATTRIBUTE_UNUSED) @@ -17267,8 +17268,17 @@ qemuDomainGetStatsState(virDomainObjPtr dom, }
+typedef enum { + QEMU_DOMAIN_STATS_HAVE_MONITOR = (1 << 0), /* QEMU monitor available */ +} qemuDomainStatsFlags; + + +#define HAVE_MONITOR(flags) ((flags) & QEMU_DOMAIN_STATS_HAVE_MONITOR) + + typedef int -(*qemuDomainGetStatsFunc)(virDomainObjPtr dom, +(*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver, + virDomainObjPtr dom, virDomainStatsRecordPtr record, int *maxparams, unsigned int flags); @@ -17276,11 +17286,12 @@ typedef int struct qemuDomainGetStatsWorker { qemuDomainGetStatsFunc func; unsigned int stats; + bool monitor; };
static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = { - { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE}, - { NULL, 0 } + { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE, false }, + { NULL, 0, false } };
@@ -17312,6 +17323,20 @@ qemuDomainGetStatsCheckSupport(unsigned int *stats, }
+static bool +qemuDomainGetStatsNeedMonitor(unsigned int stats) +{ + size_t i; + + for (i = 0; qemuDomainGetStatsWorkers[i].func; i++) + if (stats & qemuDomainGetStatsWorkers[i].stats) + if (qemuDomainGetStatsWorkers[i].monitor) + return true; + + return false; +} + + static int qemuDomainGetStats(virConnectPtr conn, virDomainObjPtr dom, @@ -17329,8 +17354,8 @@ qemuDomainGetStats(virConnectPtr conn,
for (i = 0; qemuDomainGetStatsWorkers[i].func; i++) { if (stats & qemuDomainGetStatsWorkers[i].stats) { - if (qemuDomainGetStatsWorkers[i].func(dom, tmp, &maxparams, - flags) < 0) + if (qemuDomainGetStatsWorkers[i].func(conn->privateData, dom, tmp, + &maxparams, flags) < 0) goto cleanup; } } @@ -17369,6 +17394,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, int nstats = 0; size_t i; int ret = -1; + unsigned int privflags = 0;
if (ndoms) virCheckFlags(VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1); @@ -17403,6 +17429,9 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, if (VIR_ALLOC_N(tmpstats, ndoms + 1) < 0) goto cleanup;
+ if (qemuDomainGetStatsNeedMonitor(stats)) + privflags |= QEMU_DOMAIN_STATS_HAVE_MONITOR; + for (i = 0; i < ndoms; i++) { virDomainStatsRecordPtr tmp = NULL;
@@ -17413,12 +17442,22 @@ qemuConnectGetAllDomainStats(virConnectPtr conn, !virConnectGetAllDomainStatsCheckACL(conn, dom->def)) continue;
- if (qemuDomainGetStats(conn, dom, stats, &tmp, flags) < 0) - goto cleanup; + if (HAVE_MONITOR(privflags) && + qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) < 0) + /* As it was never requested. Gather as much as possible anyway. */ + privflags &= ~QEMU_DOMAIN_STATS_HAVE_MONITOR;
This masks out the monitor for every subsequent domain too. If a single domain is stuck in the job, other can work, so this should be done on a per-domain basis.
+ + if (qemuDomainGetStats(conn, dom, stats, &tmp, privflags) < 0) + goto endjob;
if (tmp) tmpstats[nstats++] = tmp;
+ if (HAVE_MONITOR(privflags) && !qemuDomainObjEndJob(driver, dom)) { + dom = NULL; + goto cleanup; + } + virObjectUnlock(dom); dom = NULL; } @@ -17428,6 +17467,11 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
ret = nstats;
+ endjob: + if (HAVE_MONITOR(privflags) && dom) + if (!qemuDomainObjEndJob(driver, dom)) + dom = NULL; + cleanup: if (dom) virObjectUnlock(dom);
Otherwise looks good. I'll see how the rest of the series goes and if this will be one of few problems I'll fix it myself. Peter