
On 09/17/2015 04:09 AM, Jim Fehlig wrote:
On 09/08/2015 02:27 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@oracle.com> --- src/libxl/libxl_driver.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 5f69b49..101e4c7 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"
@@ -4638,6 +4640,114 @@ 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; + } +
I see that xl calls libxl_dominfo_dispose() after successful libxl_domain_info(). We might be missing that elsewhere in the libxl driver.
You're right. And I don't do that in all of the other patches of this series that use libxl_domain_info. Tracking down these calls only libxlReconnectDomain and libxlDomainGetInfo are missing the dispose. I will send a patch for these two.
+ if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_CPU_STATS_CPUTIME, + VIR_TYPED_PARAM_ULLONG, d_info.cpu_time) < 0) + return ret; + + return nparams; +} + +static int +libxlDomainGetPerCPUStats(libxlDriverPrivatePtr driver, + virDomainObjPtr vm, + virTypedParameterPtr params, + unsigned int nparams, + int start_cpu, + unsigned int ncpus)
Indentation of parameters is off.
This one slipped slipped me, will fix that. I ran make syntax-check again and there is one two other tiny issue regarding BlockStats macros, so I am including those in v2.
+{ + 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); + goto cleanup; + } + + 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; + + libxl_vcpuinfo_dispose(&vcpuinfo[i]);
The call to libxl_vcpuinfo_dispose() can be removed
+ } + ret = nparams; + + cleanup: + VIR_FREE(vcpuinfo);
if the VIR_FREE() is replaced with libxl_vcpuinfo_list_free().
Got it.
Other than the few nits, looks good.
BTW, thanks for the patches! But be warned: I may make slow progress reviewing them.
Regards, Jim
+ 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) @@ -5230,6 +5340,7 @@ static virHypervisorDriver libxlHypervisorDriver = { #endif .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */ .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */ + .domainGetCPUStats = libxlDomainGetCPUStats, /* 1.2.20 */ .connectDomainEventRegister = libxlConnectDomainEventRegister, /* 0.9.0 */ .connectDomainEventDeregister = libxlConnectDomainEventDeregister, /* 0.9.0 */ .domainManagedSave = libxlDomainManagedSave, /* 0.9.2 */