[libvirt] [PATCHv2 10/11] qemu: bulk stats: implement block group

[Eric Blake]

On 09/02/2014 06:31 AM, Francesco Romani wrote:
> This patch implements the VIR_DOMAIN_STATS_BLOCK
> group of statistics.
> 
> Signed-off-by: Francesco Romani <fromani at redhat.com>
> ---
>  include/libvirt/libvirt.h.in |  1 +
>  src/libvirt.c                | 13 +++++++++
>  src/qemu/qemu_driver.c       | 65 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 79 insertions(+)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 33588d6..1d90f5e 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -2515,6 +2515,7 @@ typedef enum {
>      VIR_DOMAIN_STATS_BALLOON = (1 << 2), /* return domain balloon info */
>      VIR_DOMAIN_STATS_VCPU = (1 << 3), /* return domain virtual CPU info */
>      VIR_DOMAIN_STATS_INTERFACE = (1 << 4), /* return domain interfaces info */
> +    VIR_DOMAIN_STATS_BLOCK = (1 << 5), /* return domain block info */
>  } virDomainStatsTypes;
>  
>  typedef enum {
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 099404b..cabfb91 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -21579,6 +21579,19 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
>   * "net.<num>.tx.errs" - transmission errors.
>   * "net.<num>.tx.drop" - transmit packets dropped.
>   *
> + * VIR_DOMAIN_STATS_BLOCK: Return block devices statistics.
> + * The typed paramer keys are in this format:

s/paramer/parameter/

> + * "block.count" - number of block devices on this domain.
> + * "block.<num>.name" - name of the block device <num>.

Does this match up to the <target dev='vda'/> name?

> + * "block.<num>.rd.reqs" - number of read requests.
> + * "block.<num>.rd.bytes" - number of read bytes.
> + * "block.<num>.rd.times" - total time (ns) spent on reads.
> + * "block.<num>.wr.reqs" - number of write requests
> + * "block.<num>.wr.bytes" - number of written bytes.
> + * "block.<num>.wr.times" - total time (ns) spent on writes.
> + * "block.<num>.fl.reqs" - total flush requests
> + * "block.<num>.fl.times" - total time (ns) spent on cache flushing

Missing types.  Inconsistent on whether you use a trailing '.'

Just because qemu doesn't report a meaningful errs does not mean that
the public API should omit it.  Hypervisors need only return the subset
of typed parameters that make sense, but we ought to document a typed
parameter for all possible fields for the API calls that this is copying
from.

> + *
>   * Using 0 for @stats returns all stats groups supported by the given
>   * hypervisor.
>   *
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 069a15d..977e8c7 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -17542,6 +17542,70 @@ qemuDomainGetStatsInterface(virConnectPtr conn ATTRIBUTE_UNUSED,
>  
>  #undef QEMU_ADD_NET_PARAM
>  
> +#define QEMU_ADD_BLOCK_PARAM_LL(RECORD, MAXPARAMS, NUM, NAME, VALUE) \
> +do { \
> +    char param_name[NAME_MAX]; \

Oversized.


> +static int
> +qemuDomainGetStatsBlock(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                        virDomainObjPtr dom,
> +                        virDomainStatsRecordPtr record,
> +                        int *maxparams,
> +                        unsigned int privflags ATTRIBUTE_UNUSED)
> +{
> +    size_t i;
> +    int ret = -1;
> +    int nstats = dom->def->ndisks;

ndisks may include such things as a cdrom drive with no media...

> +    struct qemuBlockStats *stats = NULL;
> +    virQEMUDriverPtr driver = conn->privateData;
> +
> +    if (VIR_ALLOC_N(stats, nstats) < 0)
> +        return -1;
> +
> +    if (qemuDomainGetBlockStats(driver, dom, stats, nstats) != nstats)

...so it is possible that nstats might be less than ndisks.  Is that
really a reason to return failure, instead of at least reporting the
information that was available?

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140902/57b909a0/attachment-0001.sig>