----- Original Message -----
From: "Peter Krempa" <pkrempa@redhat.com> To: "Francesco Romani" <fromani@redhat.com>, libvir-list@redhat.com Sent: Tuesday, September 9, 2014 2:14:23 PM Subject: Re: [libvirt] [PATCH 1/8] qemu: bulk stats: extend internal collection API
On 09/08/14 15:05, 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 connection 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.
Signed-off-by: Francesco Romani <fromani@redhat.com> --- src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d724eeb..2950a4b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c
@@ -17338,7 +17339,8 @@ qemuDomainGetStatsState(virDomainObjPtr dom,
typedef int -(*qemuDomainGetStatsFunc)(virDomainObjPtr dom, +(*qemuDomainGetStatsFunc)(virConnectPtr conn,
Looking through the rest of the series. Rather than the complete connection object you need just the virQEMUDriverPtr for entering the monitor, but I can live with this.
Since I need to resubmit to address your comments, I'll fix this to pass just virQEMUDriverPtr.
- if (qemuDomainGetStats(conn, dom, stats, &tmp, flags) < 0) + if (needmon && qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) < 0) goto cleanup;
- if (tmp) - tmpstats[nstats++] = tmp; + if ((needmon && virDomainObjIsActive(dom)) || !needmon) {
Hmm this skips offline domains entirely if one of the stats groups needs the monitor.
I think we should rather skip individual stats groups, or better stats fields that we can't provide.
Any ideas?
What about this (pseudo-C): unsigned int privflags = 0; if (needmon && qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) needmon = false; /* auto disable monitoring, the remainder of the function should be unchanged */ else privflags |= MONITOR_AVAILABLE; if ((needmon && virDomainObjIsActive(dom)) || !needmon) { if (qemuDomainGetStats(conn, dom, stats, &tmp, privflags) < 0) /* pass monitor availability down the chain. Individual workers will bail out immediately and silently if they need monitor but it is not available */ goto endjob; if (tmp) tmpstats[nstats++] = tmp; } No other change should be needed to this patch, and with trivial changes all the others can be fixed. -- Francesco Romani RedHat Engineering Virtualization R & D Phone: 8261328 IRC: fromani