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

[Joao Martins]

On 10/13/2015 11:02 PM, Jim Fehlig wrote:
> 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.
> 
Will remove it.

>>  }
>>  
>>  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!
> 
Glad to hear that!

> Regards,
> Jim
>