[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 2/8] libxl: implement virDomainMemorystats
On 11/18/2015 11:05 AM, Joao Martins wrote: > > On 11/17/2015 11:15 PM, Jim Fehlig 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. > I interpreted that it based on the size of the balloon instead of based on the > memory. But trying out with plain qemu (to see what the monitor socket > returns, > since qemu driver takes those values directly) and also libvirt+qemu and it > gave > me what you mentioned. VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON is indeed the > amount of memory currently assigned to the domain. Apologies for the > confusion. > >> 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. > I agree, but I mainly included RSS stat for the lack of a "current memory in > use". But since this is cleared and ACTUAL_BALLOON is not the size of the > balloon but to the actual memory, I guess this renders RSS stat a bit useless, > and perhaps misleading like you suggest. This is the hunk to be applied on top > of this patch, having tested already, and make {syntax-check,test}-ed. > > Regards, > Joao > > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index db13fd2..c1563c2 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -4799,9 +4799,8 @@ libxlDomainMemoryStats(virDomainPtr dom, > 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_ACTUAL_BALLOON, mem); > LIBXL_SET_MEMSTAT(VIR_DOMAIN_MEMORY_STAT_AVAILABLE, maxmem); > - LIBXL_SET_MEMSTAT(VIR_DOMAIN_MEMORY_STAT_RSS, mem); > > ret = i; Looks good, ACK with that squashed in. I've pushed it now. Regards, Jim _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |