[libvirt] [PATCH RFC 6/7] libxl: implement virDomainGetJobInfo

[Jim Fehlig]

Joao Martins wrote:
> Introduce support for domainGetJobInfo to get info about the
> ongoing job. If the job is active it will update the
> timeElapsed which is computed with the "started" field added to
> struct libxlDomainJobObj.  For now we support just the very basic
> info and all jobs have VIR_DOMAIN_JOB_UNBOUNDED (i.e. no completion
> time estimation) plus timeElapsed computed.
> 
> Openstack Kilo uses the Job API to monitor live-migration
> progress which is currently inexistent in libxl driver and

s/inexistent/nonexistent/ ? I think inexistent is a rarely used form of
nonexistent, but then again I'm no English major :-).

> therefore leads to a crash in the nova compute node. Right
> now, migration doesn't use jobs in the source node and will
> return VIR_DOMAIN_JOB_NONE. Though nova handles this case and
> will migrate it properly instead of crashing.
> 
> Signed-off-by: Joao Martins <joao.m.martins at oracle.com>
> ---
>  src/libxl/libxl_domain.c | 29 +++++++++++++++++++++++++++++
>  src/libxl/libxl_domain.h |  6 ++++++
>  src/libxl/libxl_driver.c | 38 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 73 insertions(+)
> 
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 40dcea1..d665615 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -74,6 +74,10 @@ libxlDomainObjInitJob(libxlDomainObjPrivatePtr priv)
>      if (virCondInit(&priv->job.cond) < 0)
>          return -1;
>  
> +    if (VIR_ALLOC(priv->job.current) < 0)
> +        return -1;
> +
> +    memset(priv->job.current, 0, sizeof(*(priv->job.current)));
>      return 0;
>  }
>  
> @@ -84,12 +88,14 @@ libxlDomainObjResetJob(libxlDomainObjPrivatePtr priv)
>  
>      job->active = LIBXL_JOB_NONE;
>      job->owner = 0;
> +

Spurious whitespace change.

>  }
>  
>  static void
>  libxlDomainObjFreeJob(libxlDomainObjPrivatePtr priv)
>  {
>      ignore_value(virCondDestroy(&priv->job.cond));
> +    VIR_FREE(priv->job.current);
>  }
>  
>  /* Give up waiting for mutex after 30 seconds */
> @@ -131,6 +137,8 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED,
>      VIR_DEBUG("Starting job: %s", libxlDomainJobTypeToString(job));
>      priv->job.active = job;
>      priv->job.owner = virThreadSelfID();
> +    priv->job.started = now;
> +    priv->job.current->type = VIR_DOMAIN_JOB_UNBOUNDED;
>  
>      return 0;
>  
> @@ -179,6 +187,27 @@ libxlDomainObjEndJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED,
>      return virObjectUnref(obj);
>  }
>  
> +int
> +libxlDomainJobUpdateTime(struct libxlDomainJobObj *job)
> +{
> +    virDomainJobInfoPtr jobInfo = job->current;
> +    unsigned long long now;
> +
> +    if (!job->started)
> +        return 0;
> +
> +    if (virTimeMillisNow(&now) < 0)
> +        return -1;
> +
> +    if (now < job->started) {
> +        job->started = 0;
> +        return 0;
> +    }
> +
> +    jobInfo->timeElapsed = now - job->started;
> +    return 0;
> +}
> +
>  static void *
>  libxlDomainObjPrivateAlloc(void)
>  {
> diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
> index 44b3e0b..1c1eba3 100644
> --- a/src/libxl/libxl_domain.h
> +++ b/src/libxl/libxl_domain.h
> @@ -53,6 +53,8 @@ struct libxlDomainJobObj {
>      virCond cond;                       /* Use to coordinate jobs */
>      enum libxlDomainJob active;         /* Currently running job */
>      int owner;                          /* Thread which set current job */
> +    unsigned long long started;         /* When the job started */
> +    virDomainJobInfoPtr current;        /* Statistics for the current job */
>  };
>  
>  typedef struct _libxlDomainObjPrivate libxlDomainObjPrivate;
> @@ -88,6 +90,10 @@ libxlDomainObjEndJob(libxlDriverPrivatePtr driver,
>                       virDomainObjPtr obj)
>      ATTRIBUTE_RETURN_CHECK;
>  
> +int
> +libxlDomainJobUpdateTime(struct libxlDomainJobObj *job)
> +    ATTRIBUTE_RETURN_CHECK;
> +
>  void
>  libxlDomainEventQueue(libxlDriverPrivatePtr driver,
>                        virObjectEventPtr event);
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 01e97fd..eaf6a67 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -5282,6 +5282,43 @@ libxlDomainMemoryStats(virDomainPtr dom,
>      return ret;
>  }
>  
> +static int
> +libxlDomainGetJobInfo(virDomainPtr dom,
> +                      virDomainJobInfoPtr info)
> +{
> +    libxlDomainObjPrivatePtr priv;
> +    virDomainObjPtr vm;
> +    int ret = -1;
> +
> +    if (!(vm = libxlDomObjFromDomain(dom)))
> +        goto cleanup;
> +
> +    if (virDomainGetJobInfoEnsureACL(dom->conn, vm->def) < 0)
> +        goto cleanup;
> +
> +    priv = vm->privateData;
> +    if (!priv->job.active) {
> +        memset(info, 0, sizeof(*info));
> +        info->type = VIR_DOMAIN_JOB_NONE;
> +        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;
> +
> +    memcpy(info, priv->job.current, sizeof(virDomainJobInfo));
> +    ret = 0;
> +
> + cleanup:
> +    if (vm)
> +        virObjectUnlock(vm);
> +    return ret;
> +}
> +
>  #undef LIBXL_SET_MEMSTAT
>  
>  #define LIBXL_RECORD_UINT(error, key, value, ...) \
> @@ -6143,6 +6180,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
>  #endif
>      .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
>      .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
> +    .domainGetJobInfo = libxlDomainGetJobInfo, /* 1.2.20 */

1.2.21.

Other than the nits, looks good!

Regards,
Jim