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

Re: [libvirt] [PATCHv5 1/8] qemu: bulk stats: extend internal collection API



On 09/15/14 10:48, Francesco Romani wrote:
> Future patches which will implement more
> bulk stats groups for QEMU will need to access
> the connection object.
> 
> To accomodate that, a few changes are needed:
> 
> * enrich internal prototype to pass qemu driver object.
> * add per-group flag to mark if one collector needs
>   monitor access or not.
> * if at least one collector of the requested stats
>   needs monitor access, thus we must start a query job
>   for each domain. The specific collectors will
>   run nested monitor jobs inside that.
> * although requested, monitor could be not available.
>   pass a flag to workers to signal the availability
>   of monitor, in order to gather as much data as
>   is possible anyway.
> 
> Signed-off-by: Francesco Romani <fromani redhat com>
> ---
>  src/qemu/qemu_driver.c | 62 +++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 54 insertions(+), 8 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 73edda3..39e9d27 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c

> @@ -17525,12 +17556,22 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
>              !virConnectGetAllDomainStatsCheckACL(conn, dom->def))
>              continue;
>  
> -        if (qemuDomainGetStats(conn, dom, stats, &tmp, flags) < 0)
> -            goto cleanup;
> +        if (HAVE_MONITOR(domflags) &&
> +             qemuDomainObjBeginJob(driver, dom, QEMU_JOB_QUERY) < 0)
> +            /* As it was never requested. Gather as much as possible anyway. */
> +            domflags &= ~QEMU_DOMAIN_STATS_HAVE_MONITOR;
> +
> +        if (qemuDomainGetStats(conn, dom, stats, &tmp, domflags) < 0)
> +            goto endjob;
>  
>          if (tmp)
>              tmpstats[nstats++] = tmp;
>  
> +        if (HAVE_MONITOR(domflags) && !qemuDomainObjEndJob(driver, dom)) {
> +            dom = NULL;
> +            goto cleanup;

I'd rather "continue;" here as a vanishing domain shouldn't be a problem
for this code.

> +        }
> +
>          virObjectUnlock(dom);
>          dom = NULL;
>      }

ACK with the nit above fixed. (I made the change locally, thus no need
to re-send this one).

Peter


Attachment: signature.asc
Description: OpenPGP digital signature


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