Re: [libvirt] [PATCH 1/2] qemu: implement block group for bulk stats.

On 08/29/2014 01:56 AM, Li Wei wrote: > This patch add the block group for bulk stats. > The following typed parameter used for each block stats: > block.count - number of block devices in this domain > block.0.name - name of the block device > block.0.rd_bytes - number of read bytes > block.0.rd_operations - number of read requests > block.0.rd_total_time - total time spend on cache reads in nano-seconds > block.0.wr_bytes - number of write bytes > block.0.wr_operations - number of write requests > block.0.wr_total_time - total time spend on cache write in nano-seconds > block.0.flush_operations - total flush requests > block.0.flush_total_time - total time spend on cache flushing in nano-seconds > > Signed-off-by: Li Wei <lw(a)cn.fujitsu.com&gt; > --- > +++ b/src/libvirt.c > @@ -21632,6 +21632,19 @@ virConnectGetAllDomainStats(virConnectPtr conn, > * "state.reason" - reason for entering given state, returned as int from > * virDomain*Reason enum corresponding to given state. > * > + * VIR_DOMAIN_STATS_BLOCK: Return block device stats. The typed parameter keys > + * are in this format: > + * "block.count" - number of block devices in this domain > + * "block.0.name" - name of the block device > + * "block.0.rd_bytes" - number of read bytes > + * "block.0.rd_operations" - number of read requests > + * "block.0.rd_total_time" - total time spend on cache reads in nano-seconds > + * "block.0.wr_bytes" - number of write bytes > + * "block.0.wr_operations" - number of write requests > + * "block.0.wr_total_time" - total time spend on cache write in nano-seconds > + * "block.0.flush_operations" - total flush requests > + * "block.0.flush_total_time" - total time spend on cache flushing in nano-seconds s/nano-seconds/nanoseconds/ Missing expected units (such as unsigned long long) Looks like Francesco has taken this patch and modified it into a newer version.

> > +static int > +qemuDomainGetStatsBlock(virDomainObjPtr dom, > + virDomainStatsRecordPtr record, > + int *maxparams, > + unsigned int flags) > +{ > + int ret; > + qemuDomainObjPrivatePtr priv = dom->privateData; > + > + /* only valid for active domain, ignore inactive ones silently */ > + if (!virDomainObjIsActive(dom)) > + return 0; > + > + if (qemuDomainObjBeginJob(qemu_driver, dom, QEMU_JOB_QUERY) < 0) > + return -1; > + > + qemuDomainObjEnterMonitor(qemu_driver, dom); Data race. You cannot safely call qemuDomainObjEnterMonitor unless you have checked that the domain is active _after_ qemuDomainObjBeginJob. Checking for an active domain prior to beginning the job doesn't hurt, but then you have to repeat the check, so it is easier to just swap the two instead of checking twice.