Re: [libvirt] [PATCH RFC 7/7] libxl: implement virDomainGetJobStats
On 10/13/2015 11:14 PM, Jim Fehlig wrote:
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(a)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.
Cool! Perhaps the v3 patch is no longer needed IIUC, given that Nikolay's
migration refactor series fix the backward compatibility issues with the
different protocol versions for drivers implementing only v3 params (such as
libxl and vz). For example, ToURI2 works now without v3 support.
Thanks for the time reviewing this series!
Joao