4:58 a.m.
On 11/17/2015 02:59 AM, Jim Fehlig wrote:
On 11/13/2015 06:14 AM, Joao Martins wrote:
> Introduce support for domainGetCPUStats API call and consequently
> allow us to use `virsh cpu-stats`. The latter returns a more brief
> output than the one provided by`virsh vcpuinfo`.
>
> Signed-off-by: Joao Martins <joao.m.martins(a)oracle.com>
> ---
> Changes since v1:
> - Remove libxl_vcpuinfo_dispose() in favor or using
> libxl_vcpuinfo_list_free(), and also removing VIR_FREE call
> - Dispose libxl_dominfo after succesfull call to
> libxl_domain_info()
> - Fixed identation of parameters
> - Bump version to 1.2.22
> ---
> src/libxl/libxl_driver.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 110 insertions(+)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index fcdcbdb..50f6e34 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -73,6 +73,8 @@ VIR_LOG_INIT("libxl.libxl_driver");
> #define LIBXL_CONFIG_FORMAT_XM "xen-xm"
> #define LIBXL_CONFIG_FORMAT_SEXPR "xen-sxpr"
>
> +#define LIBXL_NB_TOTAL_CPU_STAT_PARAM 1
> +
> #define HYPERVISOR_CAPABILITIES "/proc/xen/capabilities"
> #define HYPERVISOR_XENSTORED "/dev/xen/xenstored"
>
> @@ -4641,6 +4643,113 @@ libxlDomainIsUpdated(virDomainPtr dom)
> }
>
> static int
> +libxlDomainGetTotalCPUStats(libxlDriverPrivatePtr driver,
> + virDomainObjPtr vm,
> + virTypedParameterPtr params,
> + unsigned int nparams)
> +{
> + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
> + libxl_dominfo d_info;
> + int ret = -1;
> +
> + if (nparams == 0)
> + return LIBXL_NB_TOTAL_CPU_STAT_PARAM;
> +
> + if (libxl_domain_info(cfg->ctx, &d_info, vm->def->id) != 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("libxl_domain_info failed for domain
'%d'"),
> + vm->def->id);
> + return ret;
> + }
> +
> + if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_CPU_STATS_CPUTIME,
> + VIR_TYPED_PARAM_ULLONG, d_info.cpu_time) < 0)
> + nparams = ret;
> +
> + libxl_dominfo_dispose(&d_info);
> + return nparams;
Having a local 'ret' variable but returning nparams was a bit confusing to me.
I've changed this in my local branch to
if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_CPU_STATS_CPUTIME,
VIR_TYPED_PARAM_ULLONG, d_info.cpu_time) < 0)
goto cleanup;
ret = nparams;
cleanup:
libxl_dominfo_dispose(&d_info);
return ret;
OK. I solely did it to avoid the additional goto, but wasn't so clear in the
end.
> +}
> +
> +static int
> +libxlDomainGetPerCPUStats(libxlDriverPrivatePtr driver,
> + virDomainObjPtr vm,
> + virTypedParameterPtr params,
> + unsigned int nparams,
> + int start_cpu,
> + unsigned int ncpus)
> +{
> + libxl_vcpuinfo *vcpuinfo;
> + int maxcpu, hostcpus;
> + size_t i;
> + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
> + int ret = -1;
> +
> + if (nparams == 0 && ncpus != 0)
> + return LIBXL_NB_TOTAL_CPU_STAT_PARAM;
> + else if (nparams == 0)
> + return vm->def->maxvcpus;
> +
> + if ((vcpuinfo = libxl_list_vcpu(cfg->ctx, vm->def->id, &maxcpu,
> + &hostcpus)) == NULL) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to list vcpus for domain '%d' with
libxenlight"),
> + vm->def->id);
> + return ret;
> + }
> +
> + for (i = start_cpu; i < maxcpu && i < ncpus; ++i) {
> + if (virTypedParameterAssign(¶ms[(i-start_cpu)],
> + VIR_DOMAIN_CPU_STATS_CPUTIME,
> + VIR_TYPED_PARAM_ULLONG,
> + vcpuinfo[i].vcpu_time) < 0)
> + goto cleanup;
> + }
> + ret = nparams;
> +
> + cleanup:
> + libxl_vcpuinfo_list_free(vcpuinfo, maxcpu);
> + return ret;
> +}
> +
> +static int
> +libxlDomainGetCPUStats(virDomainPtr dom,
> + virTypedParameterPtr params,
> + unsigned int nparams,
> + int start_cpu,
> + unsigned int ncpus,
> + unsigned int flags)
> +{
> + libxlDriverPrivatePtr driver = dom->conn->privateData;
> + virDomainObjPtr vm = NULL;
> + int ret = -1;
> +
> + virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
> +
> + if (!(vm = libxlDomObjFromDomain(dom)))
> + goto cleanup;
> +
> + if (virDomainGetCPUStatsEnsureACL(dom->conn, vm->def) < 0)
> + goto cleanup;
> +
> + if (!virDomainObjIsActive(vm)) {
> + virReportError(VIR_ERR_OPERATION_INVALID,
> + "%s", _("domain is not running"));
> + goto cleanup;
> + }
> +
> + if (start_cpu == -1)
> + ret = libxlDomainGetTotalCPUStats(driver, vm, params, nparams);
> + else
> + ret = libxlDomainGetPerCPUStats(driver, vm, params, nparams,
> + start_cpu, ncpus);
> +
> + cleanup:
> + if (vm)
> + virObjectUnlock(vm);
> + return ret;
> +}
> +
> +static int
> libxlConnectDomainEventRegisterAny(virConnectPtr conn, virDomainPtr dom, int
eventID,
> virConnectDomainEventGenericCallback callback,
> void *opaque, virFreeCallback freecb)
> @@ -5233,6 +5342,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
> #endif
> .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
> .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
> + .domainGetCPUStats = libxlDomainGetCPUStats, /* 1.2.22 */
ACK with the above nit fixed. I'll push this one too after verifying whether
next release is 1.2.22 or 1.3.0.
Great, Thanks! :D
Cheers,
Joao
Regards,
Jim