[libvirt] [PATCH RFC 2/7] libxl: implement virDomainMemorystats

[Joao Martins]

On 09/17/2015 11:16 PM, Jim Fehlig wrote:
> On 09/08/2015 02:27 AM, Joao Martins wrote:
>> Introduce support for domainMemoryStats API call, which
>> consequently enables the use of `virsh dommemstat` command to
>> query for memory statistics of a domain. We support
>> the following statistics: balloon info, available and currently
>> in use. swap-in, swap-out, major-faults, minor-faults require
>> cooperation of the guest and thus currently not supported.
>>
>> We build on the data returned from libxl_domain_info and deliver
>> it in the virDomainMemoryStat format.
>>
>> Signed-off-by: Joao Martins <joao.m.martins at oracle.com>
>> ---
>>   src/libxl/libxl_driver.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 68 insertions(+)
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 101e4c7..43e9e47 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -4747,6 +4747,73 @@ libxlDomainGetCPUStats(virDomainPtr dom,
>>       return ret;
>>   }
>>   
>> +#define LIBXL_SET_MEMSTAT(TAG, VAL) \
>> +        if (i < nr_stats) { \
>> +            stats[i].tag = TAG; \
>> +            stats[i].val = VAL; \
>> +            i++; \
>> +        }
>> +
>> +static int
>> +libxlDomainMemoryStats(virDomainPtr dom,
>> +                       virDomainMemoryStatPtr stats,
>> +                       unsigned int nr_stats,
>> +                       unsigned int flags)
>> +{
>> +    libxlDriverPrivatePtr driver = dom->conn->privateData;
>> +    libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
>> +    virDomainObjPtr vm;
>> +    libxl_dominfo d_info;
>> +    unsigned mem, maxmem;
>> +    size_t i = 0;
>> +    int ret = -1;
>> +
>> +    virCheckFlags(0, -1);
>> +
>> +    if (!(vm = libxlDomObjFromDomain(dom)))
>> +        goto cleanup;
>> +
>> +    if (virDomainMemoryStatsEnsureACL(dom->conn, vm->def) < 0)
>> +        goto cleanup;
>> +
>> +    if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_QUERY) < 0)
>> +        goto cleanup;
>> +
>> +    if (!virDomainObjIsActive(vm)) {
>> +        virReportError(VIR_ERR_OPERATION_INVALID,
>> +                       "%s", _("domain is not running"));
>> +        goto endjob;
>> +    }
>> +
>> +    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 -1;
> 
> goto endjob; ?
> 
Yeah. Will fix that for v2.

> Also, as you've already noted, another case of missing libxl_dominfo_dispose(). 
> But I'll stop mentioning those now :-).
> 
OK.

Thanks,
Joao

> Regards,
> Jim
> 
>> +    }
>> +    mem = d_info.current_memkb;
>> +    maxmem = d_info.max_memkb;
>> +
>> +    LIBXL_SET_MEMSTAT(VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, maxmem - mem);
>> +    LIBXL_SET_MEMSTAT(VIR_DOMAIN_MEMORY_STAT_AVAILABLE, maxmem);
>> +    LIBXL_SET_MEMSTAT(VIR_DOMAIN_MEMORY_STAT_RSS, mem);
>> +
>> +    ret = i;
>> +
>> + endjob:
>> +    if (!libxlDomainObjEndJob(driver, vm)) {
>> +        virObjectUnlock(vm);
>> +        vm = NULL;
>> +    }
>> +
>> + cleanup:
>> +    if (vm)
>> +        virObjectUnlock(vm);
>> +    return ret;
>> +}
>> +
>> +#undef LIBXL_SET_MEMSTAT
>> +
>>   static int
>>   libxlConnectDomainEventRegisterAny(virConnectPtr conn, virDomainPtr dom, int eventID,
>>                                      virConnectDomainEventGenericCallback callback,
>> @@ -5340,6 +5407,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
>>   #endif
>>       .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
>>       .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1 */
>> +    .domainMemoryStats = libxlDomainMemoryStats, /* 1.2.20 */
>>       .domainGetCPUStats = libxlDomainGetCPUStats, /* 1.2.20 */
>>       .connectDomainEventRegister = libxlConnectDomainEventRegister, /* 0.9.0 */
>>       .connectDomainEventDeregister = libxlConnectDomainEventDeregister, /* 0.9.0 */
>