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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list