[libvirt] [PATCHv5 6/8] qemu: bulk stats: implement block group

[Peter Krempa]

On 09/15/14 10:48, Francesco Romani wrote:
> This patch implements the VIR_DOMAIN_STATS_BLOCK
> group of statistics.
> 
> To do so, an helper function to get the block stats
> of all the disks of a domain is added.
> 
> Signed-off-by: Francesco Romani <fromani at redhat.com>
> ---
>  include/libvirt/libvirt.h.in |   1 +
>  src/libvirt.c                |  20 +++++++
>  src/qemu/qemu_driver.c       |  81 ++++++++++++++++++++++++++
>  src/qemu/qemu_monitor.c      |  26 +++++++++
>  src/qemu/qemu_monitor.h      |  20 +++++++
>  src/qemu/qemu_monitor_json.c | 136 +++++++++++++++++++++++++++++--------------
>  src/qemu/qemu_monitor_json.h |   4 ++
>  7 files changed, 245 insertions(+), 43 deletions(-)
> 


> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 016499d..446b04b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -9664,6 +9664,7 @@ qemuDomainBlockStats(virDomainPtr dom,
>      return ret;
>  }
>  
> +

I've just noticed this spurious newline.

>  static int
>  qemuDomainBlockStatsFlags(virDomainPtr dom,
>                            const char *path,
> @@ -17621,6 +17622,85 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
>  
>  #undef QEMU_ADD_NET_PARAM
>  
> +/* expects a LL, but typed parameter must be ULL */
> +#define QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, num, name, value) \
> +do { \
> +    char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
> +    snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
> +             "block.%zu.%s", num, name); \
> +    if (value >= 0 && virTypedParamsAddULLong(&(record)->params, \
> +                                              &(record)->nparams, \
> +                                              maxparams, \
> +                                              param_name, \
> +                                              value) < 0) \
> +        goto cleanup; \
> +} while (0)
> +
> +static int
> +qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
> +                        virDomainObjPtr dom,
> +                        virDomainStatsRecordPtr record,
> +                        int *maxparams,
> +                        unsigned int privflags)
> +{
> +    size_t i;
> +    int ret = -1;
> +    int nstats = 0; 

nstats is here initialized to 0.

> +    qemuBlockStatsPtr stats = NULL;
> +    qemuDomainObjPrivatePtr priv = dom->privateData;
> +
> +    if (!HAVE_MONITOR(privflags) || !virDomainObjIsActive(dom))
> +        return 0; /* it's ok, just go ahead silently */
> +
> +    if (VIR_ALLOC_N(stats, dom->def->ndisks) < 0)
> +        return -1;
> +
> +    qemuDomainObjEnterMonitor(driver, dom);
> +
> +    nstats = qemuMonitorGetAllBlockStatsInfo(priv->mon, NULL,
> +                                             stats, nstats);

and not touched, thus it's still 0 when called here, which ...

> +
> +    qemuDomainObjExitMonitor(driver, dom);
> +
> +    if (nstats < 0) {
> +        virResetLastError();
> +        ret = 0; /* still ok, again go ahead silently */
> +        goto cleanup;
> +    }
> +
> +    QEMU_ADD_COUNT_PARAM(record, maxparams, "block", dom->def->ndisks);
> +
> +    for (i = 0; i < nstats; i++) {
> +        QEMU_ADD_NAME_PARAM(record, maxparams,
> +                            "block", i, dom->def->disks[i]->dst);
> +
> +        QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
> +                                "rd.reqs", stats[i].rd_req);
> +        QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
> +                                "rd.bytes", stats[i].rd_bytes);
> +        QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
> +                                "rd.times", stats[i].rd_total_times);
> +        QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
> +                                "wr.reqs", stats[i].wr_req);
> +        QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
> +                                "wr.bytes", stats[i].wr_bytes);
> +        QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
> +                                "wr.times", stats[i].wr_total_times);
> +        QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
> +                                "fl.reqs", stats[i].flush_req);
> +        QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
> +                                "fl.times", stats[i].flush_total_times);
> +    }
> +
> +    ret = 0;
> +
> + cleanup:
> +    VIR_FREE(stats);
> +    return ret;
> +}
> +
> +#undef QEMU_ADD_BLOCK_PARAM_LL
> +
>  #undef QEMU_ADD_NAME_PARAM
>  
>  #undef QEMU_ADD_COUNT_PARAM



> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 0c4832a..d45c41f 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c

> +
> +int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
> +                                        const char *dev_name,
> +                                        qemuBlockStatsPtr bstats,
> +                                        int nstats)
> +{
> +    int ret, count;
> +    size_t i;
> +    virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-blockstats",
> +                                                     NULL);
> +    virJSONValuePtr reply = NULL;
> +    virJSONValuePtr devices;
> +
>      if (!cmd)
>          return -1;
>  
> +    if (!bstats || nstats <= 0)
> +        return -1;

.. returns failure here.

> +
>      ret = qemuMonitorJSONCommand(mon, cmd, &reply);
>  
>      if (ret == 0)

nstats needs to be set to the disk count, otherwise this stats group
won't work.

Peter

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