On 09/03/14 01:19, Eric Blake wrote:
> 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@xxxxxxxxxxxxxx>
>> ---
>
>> +++ 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.
>
Also we should have a look at Francesco's series who implements the
block stats too.
Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list