[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 1/8] libxl: implement virDomainGetCPUStats
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@xxxxxxxxxx> >> --- >> 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 > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |