[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]