[libvirt] [PATCH v2 09/12] getstats: prepare for dynamic block.count stat

[Peter Krempa]

On 12/16/14 09:04, Eric Blake wrote:
> A coming patch will make it optionally possible to list backing
> chain block stats; in this mode of operation, block.counts is no
> longer the number of <disks> in the domain, but the number of
> blocks in the array being reported.  We still want block.count
> listed first, but rather than iterate the tree twice (once to
> count, and once to list stats), it's easier to just touch things
> up after the fact.
> 
> * src/qemu/qemu_driver.c (qemuDomainGetStatsBlock): Compute count
> after the fact.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/qemu/qemu_driver.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index ef73502..1127312 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -18542,13 +18542,14 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
>                          int *maxparams,
>                          unsigned int privflags)
>  {
> -    size_t i;
> +    size_t i = 0;
>      int ret = -1;
>      int rc;
>      virHashTablePtr stats = NULL;
>      qemuDomainObjPrivatePtr priv = dom->privateData;
>      bool abbreviated = false;
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    int count_index = -1;
> 
>      if (!HAVE_JOB(privflags) || !virDomainObjIsActive(dom)) {
>          abbreviated = true; /* it's ok, just go ahead silently */
> @@ -18565,9 +18566,13 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
>          }
>      }
> 
> -    QEMU_ADD_COUNT_PARAM(record, maxparams, "block", dom->def->ndisks);
> +    /* When listing backing chains, it's easier to fix up the count
> +     * after the iteration than it is to iterate twice; but we still
> +     * want count listed first.  */
> +    count_index = record->nparams;

The index is remembered prior ..

> +    QEMU_ADD_COUNT_PARAM(record, maxparams, "block", 0);

to adding the element into the array. As the macro may jump to cleanup
precisely at this moment, if adding of the element failed you'd ...

> 
> -    for (i = 0; i < dom->def->ndisks; i++) {
> +    for (; i < dom->def->ndisks; i++) {
>          qemuBlockStats *entry;
>          virDomainDiskDefPtr disk = dom->def->disks[i];
> 
> @@ -18627,6 +18632,8 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
>      ret = 0;
> 
>   cleanup:
> +    if (count_index >= 0)
> +        record->params[count_index].value.ui = i;

Touch a non-existent field here.

if (ret == 0 && count_index >= 0) ...


>      virHashFree(stats);
>      virObjectUnref(cfg);
>      return ret;
> 

ACK with the tweak above.

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/20141216/70567d51/attachment-0001.sig>