[libvirt] [PATCH RFC 7/7] libxl: implement virDomainGetJobStats

[Jim Fehlig]

Joao Martins wrote:
> Introduces support for domainGetJobStats which has the same
> info as domainGetJobInfo but in a slightly different format.
> Another difference is that virDomainGetJobStats can also
> retrieve info on the most recently completed job. Though so
> far this is only used in the source node to know if the
> migration has been completed. But because we don't support
> completed jobs we will deliver an error.
> 
> Signed-off-by: Joao Martins <joao.m.martins at oracle.com>
> ---
>  src/libxl/libxl_driver.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index eaf6a67..31f0b39 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -5319,6 +5319,58 @@ libxlDomainGetJobInfo(virDomainPtr dom,
>      return ret;
>  }
>  
> +static int
> +libxlDomainGetJobStats(virDomainPtr dom,
> +                      int *type,
> +                      virTypedParameterPtr *params,
> +                      int *nparams,
> +                      unsigned int flags)

Indentation is off a bit.

> +{
> +    libxlDomainObjPrivatePtr priv;
> +    virDomainObjPtr vm;
> +    virDomainJobInfoPtr jobInfo;
> +    int ret = -1;
> +    int maxparams = 0;
> +
> +    /* VIR_DOMAIN_JOB_STATS_COMPLETED not supported yet */
> +    virCheckFlags(0, -1);
> +
> +    if (!(vm = libxlDomObjFromDomain(dom)))
> +        goto cleanup;
> +
> +    if (virDomainGetJobStatsEnsureACL(dom->conn, vm->def) < 0)
> +        goto cleanup;
> +
> +    priv = vm->privateData;
> +    jobInfo = priv->job.current;
> +    if (!priv->job.active) {
> +        *type = VIR_DOMAIN_JOB_NONE;
> +        *params = NULL;
> +        *nparams = 0;
> +        ret = 0;
> +        goto cleanup;
> +    }
> +
> +    /* In libxl we don't have an estimed completion time

s/estimed/estimated/ ?

> +     * thus we always set to unbounded and update time
> +     * for the active job. */
> +    if (libxlDomainJobUpdateTime(&priv->job) < 0)
> +        goto cleanup;
> +
> +    if (virTypedParamsAddULLong(params, nparams, &maxparams,
> +                                VIR_DOMAIN_JOB_TIME_ELAPSED,
> +                                jobInfo->timeElapsed) < 0)
> +        goto cleanup;
> +
> +    *type = jobInfo->type;
> +    ret = 0;
> +
> + cleanup:
> +    if (vm)
> +        virObjectUnlock(vm);
> +    return ret;
> +}
> +
>  #undef LIBXL_SET_MEMSTAT
>  
>  #define LIBXL_RECORD_UINT(error, key, value, ...) \
> @@ -6181,6 +6233,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
>      .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
>      .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
>      .domainGetJobInfo = libxlDomainGetJobInfo, /* 1.2.20 */
> +    .domainGetJobStats = libxlDomainGetJobStats, /* 1.2.20 */

1.2.21.

Other than the nits, this one looks good too. Looking forward to a V2 of this
series, and your series supporting migration V3 and p2p migration. Perhaps the
latter will need some rebasing now that Nikolay's series [1] has been committed.

Regards,
Jim

[1] https://www.redhat.com/archives/libvir-list/2015-October/msg00049.html