[libvirt] [PATCH 11/12] getstats: split block stats reporting for easier recursion

[Peter Krempa]

On 12/06/14 09:14, Eric Blake wrote:
> In order to report stats on backing chains, we need to separate
> the output of stats for one block from how we traverse blocks.
> 
> * src/qemu/qemu_driver.c (qemuDomainGetStatsBlock): Split...
> (qemuDomainGetStatsOneBlock): ...into new helper.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/qemu/qemu_driver.c | 127 ++++++++++++++++++++++++++++++-------------------
>  1 file changed, 77 insertions(+), 50 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index cb80f49..feaa4a2 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -18529,6 +18529,79 @@ do { \
>          goto cleanup; \
>  } while (0)
> 
> +
> +static int
> +qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
> +                           virQEMUDriverConfigPtr cfg,
> +                           virDomainObjPtr dom,
> +                           virDomainStatsRecordPtr record,
> +                           int *maxparams,
> +                           virDomainDiskDefPtr disk,
> +                           virStorageSourcePtr src,

This is again a bit strange. I think that for other than top-level disks
also should be gathered via monitor. In that case we'll need a way to
'alias' them from qemu's point of view in an uniform way - perhaps -
node names?

> +                           size_t block_idx,
> +                           bool abbreviated,
> +                           virHashTablePtr stats)
> +{
> +    qemuBlockStats *entry;
> +    int ret = -1;
> +
> +    QEMU_ADD_NAME_PARAM(record, maxparams, "block", "name", block_idx,
> +                        disk->dst);
> +    if (virStorageSourceIsLocalStorage(src) && src->path)
> +        QEMU_ADD_NAME_PARAM(record, maxparams, "block", "path",
> +                            block_idx, src->path);
> +
> +    if (abbreviated || !disk->info.alias ||
> +        !(entry = virHashLookup(stats, disk->info.alias))) {
> +        if (qemuStorageLimitsRefresh(driver, cfg, dom,
> +                                     disk, src, NULL, NULL) < 0)

...

> +            goto cleanup;
> +        if (src->allocation)
> +            QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
> +                                     "allocation", src->allocation);
> +        if (src->capacity)
> +            QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
> +                                     "capacity", src->capacity);
> +        if (src->physical)
> +            QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
> +                                     "physical", src->physical);
> +        ret = 0;
> +        goto cleanup;
> +    }
> +
> +    QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +                            "rd.reqs", entry->rd_req);
> +    QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +                            "rd.bytes", entry->rd_bytes);
> +    QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +                            "rd.times", entry->rd_total_times);
> +    QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +                            "wr.reqs", entry->wr_req);
> +    QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +                            "wr.bytes", entry->wr_bytes);
> +    QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +                            "wr.times", entry->wr_total_times);
> +    QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +                            "fl.reqs", entry->flush_req);
> +    QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx,
> +                            "fl.times", entry->flush_total_times);
> +
> +    QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
> +                             "allocation", entry->wr_highest_offset);
> +
> +    if (entry->capacity)
> +        QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
> +                                 "capacity", entry->capacity);
> +    if (entry->physical)
> +        QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
> +                                 "physical", entry->physical);
> +
> +    ret = 0;
> + cleanup:
> +    return ret;
> +}
> +
> +
>  static int
>  qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
>                          virDomainObjPtr dom,

The recursive approach will be necessary, but I don't really like this
step where we gather the data differently for non-toplevel images.

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/20141208/750c4f0b/attachment-0001.sig>