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 <l...@cn.fujitsu.com>
> ---

> +++ 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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to