Re: [Xen-devel] [PATCH v3 2/8] libxl: implement virDomainMemorystats

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On November 17, 2015 6:15:38 PM EST, Jim Fehlig <jfehlig@xxxxxxxx> wrote:
>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@xxxxxxxxxx>
>> ---
>> Changes since v1:
>>  - Cleanup properly after error fetching domain stats
>>  - Dispose libxl_dominfo after succesfull call to
>>  libxl_domain_info()
>>  - Bump version to 1.2.22
>> ---
>>  src/libxl/libxl_driver.c | 70
>++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 70 insertions(+)
>> 
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 50f6e34..f4fc7bc 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -4749,6 +4749,75 @@ 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);
>> +        goto endjob;
>> +    }
>> +    mem = d_info.current_memkb;
>> +    maxmem = d_info.max_memkb;
>> +
>> +    LIBXL_SET_MEMSTAT(VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, maxmem
>- mem);
>
>Should this just be 'mem'? On a domain with
>
>  <memory unit='KiB'>1048576</memory>
>  <currentMemory unit='KiB'>1048576</currentMemory>
>
>I see
>
># virsh dommemstat test
>actual 1024
>available 1049600
>rss 1048576
>
>The documentation for VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON only says
>"Current
>balloon value (in KB)". I (perhaps incorrectly) interpret that as the
>amount of
>memory currently assigned to the domain.
>
>Also, I wonder if we should provide rss for Xen domains? I'm not sure
>it makes
>much sense since Xen domains are more than just a process running on
>the host.
>AFAIK, rss would always be set to d_info.current_memkb even if a domain
>was not
>actually using all the memory.
>

You could go negative with that - say dom0 has 8GB and your guest has 16GB. 
From a Linux dom0 POV we are -8GB, and since tools would most likely use 
unsigned int the sum of RSS would minus total memory would be negative,but that 
is 0xffff... So the tools might show the guest consuming gobs of memory?


Perhaps just how much QEMU is consuming (if running)?

>Regards,
>Jim
>
>> +    LIBXL_SET_MEMSTAT(VIR_DOMAIN_MEMORY_STAT_AVAILABLE, maxmem);
>> +    LIBXL_SET_MEMSTAT(VIR_DOMAIN_MEMORY_STAT_RSS, mem);
>> +
>> +    ret = i;
>> +
>> +    libxl_dominfo_dispose(&d_info);
>> +
>> + 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,
>> @@ -5342,6 +5411,7 @@ static virHypervisorDriver
>libxlHypervisorDriver = {
>>  #endif
>>      .nodeGetFreeMemory = libxlNodeGetFreeMemory, /* 0.9.0 */
>>      .nodeGetCellsFreeMemory = libxlNodeGetCellsFreeMemory, /* 1.1.1
>*/
>> +    .domainMemoryStats = libxlDomainMemoryStats, /* 1.2.22 */
>>      .domainGetCPUStats = libxlDomainGetCPUStats, /* 1.2.22 */
>>      .connectDomainEventRegister = libxlConnectDomainEventRegister,
>/* 0.9.0 */
>>      .connectDomainEventDeregister =
>libxlConnectDomainEventDeregister, /* 0.9.0 */
>
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@xxxxxxxxxxxxx
>http://lists.xen.org/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel