[libvirt] [RFC] New libvirt API for domain memory statistics reporting

When using ballooning to manage overcommitted memory on a host, a system for guests to communicate their memory usage to the host can provide information that will minimize the impact of ballooning on the guests while maximizing efficient use of host memory. The design of such a communication channel was recently added to version 0.8.2 of the VirtIO Spec (See Appendix G): - http://ozlabs.org/~rusty/virtio-spec/virtio-spec-0.8.2.pdf Host-side and guest-side implementations have been accepted for inclusion in their respective projects: - Guest: http://lkml.org/lkml/2009/11/30/274 - Host: http://lists.gnu.org/archive/html/qemu-devel/2009-12/msg00380.html The following patch series implements a new libvirt interface to expose these memory statistics. Thank you for your review and comments. [PATCH 1/6] domMemStats: Add domainMemStats method to struct _virDriver [PATCH 2/6] domMemStats: Add public symbol to libvirt API [PATCH 3/6] qemu-driver: Enable domainMemStats in the qemu driver [PATCH 4/6] remote-driver: Add domainMemStats support [PATCH 5/6] virsh: Enable virDomainMemStats as a new command [PATCH 6/6] python: Add python bindings for virDomainMemStats

Set up the types for the domainMemStats function and insert it into the virDriver structure definition. Because of static initializers, update every driver and set the new field to NULL. Note: The changes in python/* are to fix compiler errors. The actual python binding is implemented in a later patch. Signed-off-by: Adam Litke <agl@us.ibm.com> To: libvirt list <libvir-list@redhat.com> --- include/libvirt/libvirt.h.in | 31 +++++++++++++++++++++++++++++++ python/generator.py | 4 +++- src/driver.h | 6 ++++++ src/esx/esx_driver.c | 1 + src/lxc/lxc_driver.c | 1 + src/opennebula/one_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_driver.c | 1 + src/remote/remote_driver.c | 1 + src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_tmpl.c | 1 + src/xen/xen_driver.c | 1 + 14 files changed, 51 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index f51a565..e430599 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -333,6 +333,34 @@ struct _virDomainInterfaceStats { */ typedef virDomainInterfaceStatsStruct *virDomainInterfaceStatsPtr; +/** + * virDomainMemStats: + * + * Memory stats for virDomainMemStats. + * + * Hypervisors may return a field set to ((long long)-1) which indicates + * that the hypervisor or guest does not support that statistic. + * + * NB. Here 'long long' means 64 bit integer. + */ +typedef struct _virDomainMemStats virDomainMemStatsStruct; + +struct _virDomainMemStats { + unsigned long long swap_in; + unsigned long long swap_out; + unsigned long long major_fault; + unsigned long long minor_fault; + unsigned long long mem_free; + unsigned long long mem_tot; +}; + +/** + * virDomainMemStatsPtr: + * + * A pointer to a virDomainMemStats structure + */ +typedef virDomainMemStatsStruct *virDomainMemStatsPtr; + /* Domain migration flags. */ typedef enum { @@ -633,6 +661,9 @@ int virDomainInterfaceStats (virDomainPtr dom, const char *path, virDomainInterfaceStatsPtr stats, size_t size); +int virDomainMemStats (virDomainPtr dom, + virDomainMemStatsPtr stats, + size_t size); int virDomainBlockPeek (virDomainPtr dom, const char *path, unsigned long long offset, diff --git a/python/generator.py b/python/generator.py index 21b4137..fef20fb 100755 --- a/python/generator.py +++ b/python/generator.py @@ -160,7 +160,8 @@ def enum(type, name, value): functions_failed = [] functions_skipped = [ - "virConnectListDomains" + "virConnectListDomains", + "virDomainMemStats" ] skipped_modules = { @@ -170,6 +171,7 @@ skipped_types = { # 'int *': "usually a return type", 'virConnectDomainEventCallback': "No function types in python", 'virEventAddHandleFunc': "No function types in python", + 'virDomainMemStatsPtr': "Not implemented" } ####################################################################### diff --git a/src/driver.h b/src/driver.h index 0c8f923..42615c5 100644 --- a/src/driver.h +++ b/src/driver.h @@ -226,6 +226,11 @@ typedef int struct _virDomainInterfaceStats *stats); typedef int + (*virDrvDomainMemStats) + (virDomainPtr domain, + struct _virDomainMemStats *stats); + +typedef int (*virDrvDomainBlockPeek) (virDomainPtr domain, const char *path, @@ -406,6 +411,7 @@ struct _virDriver { virDrvDomainMigrateFinish domainMigrateFinish; virDrvDomainBlockStats domainBlockStats; virDrvDomainInterfaceStats domainInterfaceStats; + virDrvDomainMemStats domainMemStats; virDrvDomainBlockPeek domainBlockPeek; virDrvDomainMemoryPeek domainMemoryPeek; virDrvNodeGetCellsFreeMemory nodeGetCellsFreeMemory; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index e063b46..3e94686 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3263,6 +3263,7 @@ static virDriver esxDriver = { esxDomainMigrateFinish, /* domainMigrateFinish */ NULL, /* domainBlockStats */ NULL, /* domainInterfaceStats */ + NULL, /* domainMemStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* nodeGetCellsFreeMemory */ diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 0b614e3..a890c67 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2312,6 +2312,7 @@ static virDriver lxcDriver = { NULL, /* domainMigrateFinish */ NULL, /* domainBlockStats */ NULL, /* domainInterfaceStats */ + NULL, /* domainMemStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ diff --git a/src/opennebula/one_driver.c b/src/opennebula/one_driver.c index 9bcd5c3..3702d03 100644 --- a/src/opennebula/one_driver.c +++ b/src/opennebula/one_driver.c @@ -776,6 +776,7 @@ static virDriver oneDriver = { NULL, /* domainMigrateFinish */ NULL, /* domainBlockStats */ NULL, /* domainInterfaceStats */ + NULL, /* domainMemStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* nodeGetCellsFreeMemory */ diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index f64ad1e..4ba9f47 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1421,6 +1421,7 @@ static virDriver openvzDriver = { NULL, /* domainMigrateFinish */ NULL, /* domainBlockStats */ NULL, /* domainInterfaceStats */ + NULL, /* domainMemStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* nodeGetCellsFreeMemory */ diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index ef465ed..f275283 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1366,6 +1366,7 @@ virDriver phypDriver = { NULL, /* domainMigrateFinish */ NULL, /* domainBlockStats */ NULL, /* domainInterfaceStats */ + NULL, /* domainMemStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ NULL, /* nodeGetCellsFreeMemory */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c544c4b..35c397d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7124,6 +7124,7 @@ static virDriver qemuDriver = { NULL, /* domainMigrateFinish */ qemudDomainBlockStats, /* domainBlockStats */ qemudDomainInterfaceStats, /* domainInterfaceStats */ + NULL, /* domainMemStats */ qemudDomainBlockPeek, /* domainBlockPeek */ qemudDomainMemoryPeek, /* domainMemoryPeek */ nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index bf001eb..0bfe730 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8437,6 +8437,7 @@ static virDriver remote_driver = { remoteDomainMigrateFinish, /* domainMigrateFinish */ remoteDomainBlockStats, /* domainBlockStats */ remoteDomainInterfaceStats, /* domainInterfaceStats */ + NULL, /* domainMemStats */ remoteDomainBlockPeek, /* domainBlockPeek */ remoteDomainMemoryPeek, /* domainMemoryPeek */ remoteNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 0541a73..b82fb1b 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4546,6 +4546,7 @@ static virDriver testDriver = { NULL, /* domainMigrateFinish */ testDomainBlockStats, /* domainBlockStats */ testDomainInterfaceStats, /* domainInterfaceStats */ + NULL, /* domainMemStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ testNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 9a7fe42..10b62d9 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1850,6 +1850,7 @@ static virDriver umlDriver = { NULL, /* domainMigrateFinish */ NULL, /* domainBlockStats */ NULL, /* domainInterfaceStats */ + NULL, /* domainMemStats */ umlDomainBlockPeek, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 4f43901..6b6656e 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -6451,6 +6451,7 @@ virDriver NAME(Driver) = { NULL, /* domainMigrateFinish */ NULL, /* domainBlockStats */ NULL, /* domainInterfaceStats */ + NULL, /* domainMemStats */ NULL, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 5273a11..5a5eca9 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1714,6 +1714,7 @@ static virDriver xenUnifiedDriver = { xenUnifiedDomainMigrateFinish, /* domainMigrateFinish */ xenUnifiedDomainBlockStats, /* domainBlockStats */ xenUnifiedDomainInterfaceStats, /* domainInterfaceStats */ + NULL, /* domainMemStats */ xenUnifiedDomainBlockPeek, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ xenUnifiedNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ -- 1.6.5

Add a new function 'virDomainMemStats' to the libvirt API. Export it via libvirt_public.syms Signed-off-by: Adam Litke <agl@us.ibm.com> To: libvirt list <libvir-list@redhat.com> --- src/libvirt.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 2 files changed, 51 insertions(+), 0 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index b14942d..d632bd7 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -4045,6 +4045,56 @@ error: } /** + * virDomainMemStats: + * @dom: pointer to the domain object + * @stats: memory stats structure (returned) + * @size: size of memory stats structure + * + * This function returns memory statistics for the domain. + * + * Individual fields within the stats structure may be returned + * as -1, which indicates that the hypervisor or guest does not + * support that particular statistic. + * + * Returns: 0 in case of success or -1 in case of failure. + */ +int +virDomainMemStats (virDomainPtr dom, virDomainMemStatsPtr stats, + size_t size) +{ + virConnectPtr conn; + struct _virDomainMemStats stats2 = { -1, -1, -1, -1, -1, -1 }; + DEBUG("domain=%p, stats=%p, size=%zi", dom, stats, size); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN (dom)) { + virLibDomainError (NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + return -1; + } + if (!stats || size > sizeof stats2) { + virLibDomainError (dom, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + conn = dom->conn; + + if (conn->driver->domainMemStats) { + if (conn->driver->domainMemStats (dom, &stats2) == -1) + goto error; + + memcpy (stats, &stats2, size); + return 0; + } + + virLibDomainError (dom, VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + /* Copy to connection error object for back compatability */ + virSetConnError(dom->conn); + return -1; +} + +/** * virDomainBlockPeek: * @dom: pointer to the domain object * @path: path to the block device diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 8921c1a..009c0ba 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -327,6 +327,7 @@ LIBVIRT_0.7.2 { virStreamAbort; virStreamFree; virDomainMigrateToURI; + virDomainMemStats; } LIBVIRT_0.7.1; # .... define new API here using predicted next version number .... -- 1.6.5

Support for memory statistics reporting is accepted for qemu inclusion. Statistics are reported via the monitor command 'info balloon' as a comma seprated list: (qemu) info balloon balloon: actual=1024,mem_swapped_in=0,mem_swapped_out=0,major_page_faults=88,minor_page_faults=105535,free_mem=1017065472,total_mem=1045229568 Libvirt, qemu, and the guest operating system may support a subset of the statistics defined by the virtio spec. Thus, only statistics recognized by all components will be reported. All others will be returned as -1. Signed-off-by: Adam Litke <agl@us.ibm.com> To: libvirt list <libvir-list@redhat.com> --- src/qemu/qemu_driver.c | 40 +++++++++++++++++++++++++++-- src/qemu/qemu_monitor_text.c | 56 ++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_monitor_text.h | 3 +- 3 files changed, 93 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 35c397d..60c0da1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3077,7 +3077,7 @@ static int qemudDomainGetInfo(virDomainPtr dom, info->maxMem = vm->def->maxmem; if (virDomainIsActive(vm)) { - err = qemuMonitorGetBalloonInfo(vm, &balloon); + err = qemuMonitorGetBalloonInfo(vm, &balloon, NULL); if (err < 0) goto cleanup; @@ -3880,7 +3880,7 @@ static char *qemudDomainDumpXML(virDomainPtr dom, /* Refresh current memory based on balloon info */ if (virDomainIsActive(vm)) { - err = qemuMonitorGetBalloonInfo(vm, &balloon); + err = qemuMonitorGetBalloonInfo(vm, &balloon, NULL); if (err < 0) goto cleanup; if (err > 0) @@ -5563,6 +5563,40 @@ qemudDomainInterfaceStats (virDomainPtr dom, #endif static int +qemudDomainMemStats (virDomainPtr dom, + struct _virDomainMemStats *stats) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + struct _virDomainMemStats stats2 = { -1, -1, -1, -1, -1, -1 }; + int ret = -1; + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (virDomainIsActive(vm)) { + memcpy (stats, &stats2, sizeof stats); + ret = qemuMonitorGetBalloonInfo(vm, NULL, stats); + if (ret > 0) + ret = 0; + } + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + +static int qemudDomainBlockPeek (virDomainPtr dom, const char *path, unsigned long long offset, size_t size, @@ -7124,7 +7158,7 @@ static virDriver qemuDriver = { NULL, /* domainMigrateFinish */ qemudDomainBlockStats, /* domainBlockStats */ qemudDomainInterfaceStats, /* domainInterfaceStats */ - NULL, /* domainMemStats */ + qemudDomainMemStats, /* domainMemStats */ qemudDomainBlockPeek, /* domainBlockPeek */ qemudDomainMemoryPeek, /* domainMemoryPeek */ nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 66526dc..f785f7c 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -650,6 +650,52 @@ error: return 0; } +/* The reply from the 'info balloon' command may contain additional memory + * statistics in the form: '[,<tag>=<val>]*' + */ +static void qemuMonitorParseExtraBalloonInfo(const virDomainObjPtr vm, + char *text, + virDomainMemStatsPtr stats) +{ + char *dummy, *p = text; + while (*p) { + if (STRPREFIX (p, ",mem_swapped_in=")) { + p += 16; + if (virStrToLong_ull (p, &dummy, 10, &stats->swap_in)) + DEBUG ("%s: error reading mem_swapped_in: %s", + vm->def->name, p); + } else if (STRPREFIX (p, ",mem_swapped_out=")) { + p += 17; + if (virStrToLong_ull (p, &dummy, 10, &stats->swap_out)) + DEBUG ("%s: error reading mem_swapped_out: %s", + vm->def->name, p); + } else if (STRPREFIX (p, ",major_page_faults=")) { + p += 19; + if (virStrToLong_ull (p, &dummy, 10, &stats->major_fault)) + DEBUG ("%s: error reading major_page_faults: %s", + vm->def->name, p); + } else if (STRPREFIX (p, ",minor_page_faults=")) { + p += 19; + if (virStrToLong_ull (p, &dummy, 10, &stats->minor_fault)) + DEBUG ("%s: error reading minor_page_faults: %s", + vm->def->name, p); + } else if (STRPREFIX (p, ",free_mem=")) { + p += 10; + if (virStrToLong_ull (p, &dummy, 10, &stats->mem_free)) + DEBUG ("%s: error reading free_mem: %s", + vm->def->name, p); + } else if (STRPREFIX (p, ",total_mem=")) { + p += 11; + if (virStrToLong_ull (p, &dummy, 10, &stats->mem_tot)) + DEBUG ("%s: error reading total_mem: %s", + vm->def->name, p); + } + + /* Skip to the next label */ + p = strchr (p, ','); + if (!p) break; + } +} /* The reply from QEMU contains 'ballon: actual=421' where value is in MB */ @@ -660,7 +706,8 @@ error: * or -1 on failure */ int qemuMonitorGetBalloonInfo(const virDomainObjPtr vm, - unsigned long *currmem) + unsigned long *currmem, + virDomainMemStatsPtr stats) { char *reply = NULL; int ret = -1; @@ -683,7 +730,12 @@ int qemuMonitorGetBalloonInfo(const virDomainObjPtr vm, _("could not parse memory balloon allocation from '%s'"), reply); goto cleanup; } - *currmem = memMB * 1024; + if (currmem != NULL) + *currmem = memMB * 1024; + offset = strchr(reply, ','); + if (stats != NULL && offset != NULL) { + qemuMonitorParseExtraBalloonInfo(vm, offset, stats); + } ret = 1; } else { /* We don't raise an error here, since its to be expected that diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index 9175456..4e9b581 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -41,7 +41,8 @@ int qemuMonitorSystemPowerdown(const virDomainObjPtr vm); int qemuMonitorGetCPUInfo(const virDomainObjPtr vm, int **pids); int qemuMonitorGetBalloonInfo(const virDomainObjPtr vm, - unsigned long *currmem); + unsigned long *currmem, + virDomainMemStatsPtr stats); int qemuMonitorGetBlockStatsInfo(const virDomainObjPtr vm, const char *devname, long long *rd_req, -- 1.6.5

This patch is boiler-plate code to enable domainMemStats pass-through in the remote driver. Signed-off-by: Adam Litke <agl@us.ibm.com> To: libvirt list <libvir-list@redhat.com> --- daemon/remote.c | 35 +++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 36 +++++++++++++++++++++++++++++++++++- src/remote/remote_protocol.c | 26 ++++++++++++++++++++++++++ src/remote/remote_protocol.h | 20 ++++++++++++++++++++ src/remote/remote_protocol.x | 15 +++++++++++++++ 5 files changed, 131 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 4296fc3..fb11205 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -688,6 +688,41 @@ remoteDispatchDomainInterfaceStats (struct qemud_server *server ATTRIBUTE_UNUSED } static int +remoteDispatchDomainMemStats (struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error *rerr, + remote_domain_mem_stats_args *args, + remote_domain_mem_stats_ret *ret) +{ + virDomainPtr dom; + struct _virDomainMemStats stats; + + dom = get_nonnull_domain (conn, args->dom); + if (dom == NULL) { + remoteDispatchConnError(rerr, conn); + return -1; + } + + if (virDomainMemStats (dom, &stats, sizeof stats) == -1) { + virDomainFree (dom); + remoteDispatchConnError(rerr, conn); + return -1; + } + virDomainFree (dom); + + ret->swap_in = stats.swap_in; + ret->swap_out = stats.swap_out; + ret->major_fault = stats.major_fault; + ret->minor_fault = stats.minor_fault; + ret->mem_free = stats.mem_free; + ret->mem_tot = stats.mem_tot; + + return 0; +} + +static int remoteDispatchDomainBlockPeek (struct qemud_server *server ATTRIBUTE_UNUSED, struct qemud_client *client ATTRIBUTE_UNUSED, virConnectPtr conn, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 0bfe730..be661bd 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3208,6 +3208,40 @@ done: } static int +remoteDomainMemStats (virDomainPtr domain, struct _virDomainMemStats *stats) +{ + int rv = -1; + remote_domain_mem_stats_args args; + remote_domain_mem_stats_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain (&args.dom, domain); + memset (&ret, 0, sizeof ret); + + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_MEM_STATS, + (xdrproc_t) xdr_remote_domain_mem_stats_args, + (char *) &args, + (xdrproc_t) xdr_remote_domain_mem_stats_ret, + (char *) &ret) == -1) + goto done; + + stats->swap_in = ret.swap_in; + stats->swap_out = ret.swap_out; + stats->major_fault = ret.major_fault; + stats->minor_fault = ret.minor_fault; + stats->mem_free = ret.mem_free; + stats->mem_tot = ret.mem_tot; + + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainBlockPeek (virDomainPtr domain, const char *path, unsigned long long offset, @@ -8437,7 +8471,7 @@ static virDriver remote_driver = { remoteDomainMigrateFinish, /* domainMigrateFinish */ remoteDomainBlockStats, /* domainBlockStats */ remoteDomainInterfaceStats, /* domainInterfaceStats */ - NULL, /* domainMemStats */ + remoteDomainMemStats, /* domainMemStats */ remoteDomainBlockPeek, /* domainBlockPeek */ remoteDomainMemoryPeek, /* domainMemoryPeek */ remoteNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ diff --git a/src/remote/remote_protocol.c b/src/remote/remote_protocol.c index 1449ed4..5ed5712 100644 --- a/src/remote/remote_protocol.c +++ b/src/remote/remote_protocol.c @@ -588,6 +588,32 @@ xdr_remote_domain_interface_stats_ret (XDR *xdrs, remote_domain_interface_stats_ } bool_t +xdr_remote_domain_mem_stats_args (XDR *xdrs, remote_domain_mem_stats_args *objp) +{ + if (!xdr_remote_nonnull_domain (xdrs, &objp->dom)) + return FALSE; + return TRUE; +} + +bool_t +xdr_remote_domain_mem_stats_ret (XDR *xdrs, remote_domain_mem_stats_ret *objp) +{ + if (!xdr_uint64_t (xdrs, &objp->swap_in)) + return FALSE; + if (!xdr_uint64_t (xdrs, &objp->swap_out)) + return FALSE; + if (!xdr_uint64_t (xdrs, &objp->major_fault)) + return FALSE; + if (!xdr_uint64_t (xdrs, &objp->minor_fault)) + return FALSE; + if (!xdr_uint64_t (xdrs, &objp->mem_free)) + return FALSE; + if (!xdr_uint64_t (xdrs, &objp->mem_tot)) + return FALSE; + return TRUE; +} + +bool_t xdr_remote_domain_block_peek_args (XDR *xdrs, remote_domain_block_peek_args *objp) { diff --git a/src/remote/remote_protocol.h b/src/remote/remote_protocol.h index d87e8bc..5b274bc 100644 --- a/src/remote/remote_protocol.h +++ b/src/remote/remote_protocol.h @@ -302,6 +302,21 @@ struct remote_domain_interface_stats_ret { }; typedef struct remote_domain_interface_stats_ret remote_domain_interface_stats_ret; +struct remote_domain_mem_stats_args { + remote_nonnull_domain dom; +}; +typedef struct remote_domain_mem_stats_args remote_domain_mem_stats_args; + +struct remote_domain_mem_stats_ret { + uint64_t swap_in; + uint64_t swap_out; + uint64_t major_fault; + uint64_t minor_fault; + uint64_t mem_free; + uint64_t mem_tot; +}; +typedef struct remote_domain_mem_stats_ret remote_domain_mem_stats_ret; + struct remote_domain_block_peek_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -1689,6 +1704,7 @@ enum remote_procedure { REMOTE_PROC_SECRET_UNDEFINE = 146, REMOTE_PROC_SECRET_LOOKUP_BY_USAGE = 147, REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL = 148, + REMOTE_PROC_DOMAIN_MEM_STATS = 149, }; typedef enum remote_procedure remote_procedure; @@ -1764,6 +1780,8 @@ extern bool_t xdr_remote_domain_block_stats_args (XDR *, remote_domain_block_st extern bool_t xdr_remote_domain_block_stats_ret (XDR *, remote_domain_block_stats_ret*); extern bool_t xdr_remote_domain_interface_stats_args (XDR *, remote_domain_interface_stats_args*); extern bool_t xdr_remote_domain_interface_stats_ret (XDR *, remote_domain_interface_stats_ret*); +extern bool_t xdr_remote_domain_mem_stats_args (XDR *, remote_domain_mem_stats_args*); +extern bool_t xdr_remote_domain_mem_stats_ret (XDR *, remote_domain_mem_stats_ret*); extern bool_t xdr_remote_domain_block_peek_args (XDR *, remote_domain_block_peek_args*); extern bool_t xdr_remote_domain_block_peek_ret (XDR *, remote_domain_block_peek_ret*); extern bool_t xdr_remote_domain_memory_peek_args (XDR *, remote_domain_memory_peek_args*); @@ -2019,6 +2037,8 @@ extern bool_t xdr_remote_domain_block_stats_args (); extern bool_t xdr_remote_domain_block_stats_ret (); extern bool_t xdr_remote_domain_interface_stats_args (); extern bool_t xdr_remote_domain_interface_stats_ret (); +extern bool_t xdr_remote_domain_mem_stats_args (); +extern bool_t xdr_remote_domain_mem_stats_ret (); extern bool_t xdr_remote_domain_block_peek_args (); extern bool_t xdr_remote_domain_block_peek_ret (); extern bool_t xdr_remote_domain_memory_peek_args (); diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 2b3c03b..ce4a511 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -401,6 +401,19 @@ struct remote_domain_interface_stats_ret { hyper tx_drop; }; +struct remote_domain_mem_stats_args { + remote_nonnull_domain dom; +}; + +struct remote_domain_mem_stats_ret { + unsigned hyper swap_in; + unsigned hyper swap_out; + unsigned hyper major_fault; + unsigned hyper minor_fault; + unsigned hyper mem_free; + unsigned hyper mem_tot; +}; + struct remote_domain_block_peek_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -1533,6 +1546,8 @@ enum remote_procedure { REMOTE_PROC_SECRET_LOOKUP_BY_USAGE = 147, REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL = 148 + + REMOTE_PROC_DOMAIN_MEM_STATS = 149, }; -- 1.6.5

Define a new command 'dommemstat' to report domain memory statistics. The output format is inspired by 'domblkstat' and 'domifstat' and consists of tag/value pairs, one per line. The command can complete successfully and print no output if virDomainMemStats is supported by the driver, but not the guest operating system. Sample output: swap_in 0 swap_out 0 major_fault 54 minor_fault 58259 mem_free 499384320 mem_tot 514531328 All stats referring to a quantity of memory (eg. all above except major and minor faults) represent the quantity in bytes. Signed-off-by: Adam Litke <agl@us.ibm.com> To: libvirt list <libvir-list@redhat.com> --- tools/virsh.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 51 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 46c5454..a5133d5 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -874,6 +874,56 @@ cmdDomIfstat (vshControl *ctl, const vshCmd *cmd) } /* + * "dommemstat" command + */ +static const vshCmdInfo info_dommemstat[] = { + {"help", gettext_noop("get memory stats for a domain")}, + {"desc", gettext_noop("Get memory stats for a running domain.")}, + {NULL,NULL} +}; + +static const vshCmdOptDef opts_dommemstat[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")}, + {NULL, 0, 0, NULL} +}; + +static int +cmdDomMemStat(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + char *name; + struct _virDomainMemStats stats; + + if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) + return FALSE; + + if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) + return FALSE; + + if (virDomainMemStats (dom, &stats, sizeof stats) == -1) { + vshError(ctl, _("Failed to get memory stats for domain %s"), name); + virDomainFree(dom); + return FALSE; + } + + if (stats.swap_in != (uint64_t)-1) + vshPrint (ctl, "swap_in %llu\n", stats.swap_in); + if (stats.swap_out != (uint64_t)-1) + vshPrint (ctl, "swap_out %llu\n", stats.swap_out); + if (stats.major_fault != (uint64_t)-1) + vshPrint (ctl, "major_fault %llu\n", stats.major_fault); + if (stats.minor_fault != (uint64_t)-1) + vshPrint (ctl, "minor_fault %llu\n", stats.minor_fault); + if (stats.mem_free != (uint64_t)-1) + vshPrint (ctl, "mem_free %llu\n", stats.mem_free); + if (stats.mem_tot != (uint64_t)-1) + vshPrint (ctl, "mem_tot %llu\n", stats.mem_tot); + + virDomainFree(dom); + return TRUE; +} + +/* * "suspend" command */ static const vshCmdInfo info_suspend[] = { @@ -7183,6 +7233,7 @@ static const vshCmdDef commands[] = { {"domstate", cmdDomstate, opts_domstate, info_domstate}, {"domblkstat", cmdDomblkstat, opts_domblkstat, info_domblkstat}, {"domifstat", cmdDomIfstat, opts_domifstat, info_domifstat}, + {"dommemstat", cmdDomMemStat, opts_dommemstat, info_dommemstat}, {"domxml-from-native", cmdDomXMLFromNative, opts_domxmlfromnative, info_domxmlfromnative}, {"domxml-to-native", cmdDomXMLToNative, opts_domxmltonative, info_domxmltonative}, {"dumpxml", cmdDumpXML, opts_dumpxml, info_dumpxml}, -- 1.6.5

Enable virDomainMemStats in the python API. dom.memStats() will return a dictionary containing the supported statistics. A dictionary is required because the meaining of each quantity cannot be inferred from its index in a list. Signed-off-by: Adam Litke <agl@us.ibm.com> To: libvirt list <libvir-list@redhat.com> --- python/generator.py | 1 + python/libvirt-override-api.xml | 5 ++++ python/libvirt-override.c | 41 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 0 deletions(-) diff --git a/python/generator.py b/python/generator.py index fef20fb..8721847 100755 --- a/python/generator.py +++ b/python/generator.py @@ -282,6 +282,7 @@ skip_impl = ( 'virNetworkGetAutostart', 'virDomainBlockStats', 'virDomainInterfaceStats', + 'virDomainMemStats', 'virNodeGetCellsFreeMemory', 'virDomainGetSchedulerType', 'virDomainGetSchedulerParameters', diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 148b89b..f524d22 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -100,6 +100,11 @@ <arg name='domain' type='virDomainPtr' info='a domain object'/> <arg name='path' type='char *' info='the path for the interface device'/> </function> + <function name='virDomainMemStats' file='python'> + <info>Extracts memory statistics for a domain</info> + <return type='virDomainMemStats' info='a dictionary of statistics'/> + <arg name='domain' type='virDomainPtr' info='a domain object'/> + </function> <function name="virNodeGetCellsFreeMemory" file='python'> <info>Returns the available memory for a list of cells</info> <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 5d24fd2..b7ae77a 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -114,6 +114,46 @@ libvirt_virDomainInterfaceStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) return(info); } +static PyObject * +libvirt_virDomainMemStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { + virDomainPtr domain; + PyObject *pyobj_domain; + int c_retval; + virDomainMemStatsStruct stats; + PyObject *info; + + if (!PyArg_ParseTuple(args, (char *)"O:virDomainMemStats", &pyobj_domain)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + c_retval = virDomainMemStats(domain, &stats, sizeof(stats)); + if (c_retval < 0) + return VIR_PY_NONE; + + /* convert to a Python dictionary */ + if ((info = PyDict_New()) == NULL) + return VIR_PY_NONE; + + if (stats.swap_in != -1) + PyDict_SetItem(info, libvirt_constcharPtrWrap("swap_in"), + PyLong_FromUnsignedLongLong(stats.swap_in)); + if (stats.swap_out != -1) + PyDict_SetItem(info, libvirt_constcharPtrWrap("swap_out"), + PyLong_FromUnsignedLongLong(stats.swap_out)); + if (stats.major_fault != -1) + PyDict_SetItem(info, libvirt_constcharPtrWrap("major_fault"), + PyLong_FromUnsignedLongLong(stats.major_fault)); + if (stats.minor_fault != -1) + PyDict_SetItem(info, libvirt_constcharPtrWrap("minor_fault"), + PyLong_FromUnsignedLongLong(stats.minor_fault)); + if (stats.mem_free != -1) + PyDict_SetItem(info, libvirt_constcharPtrWrap("mem_free"), + PyLong_FromUnsignedLongLong(stats.mem_free)); + if (stats.mem_tot != -1) + PyDict_SetItem(info, libvirt_constcharPtrWrap("mem_tot"), + PyLong_FromUnsignedLongLong(stats.mem_tot)); + return info; +} static PyObject * libvirt_virDomainGetSchedulerType(PyObject *self ATTRIBUTE_UNUSED, @@ -2414,6 +2454,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virNetworkGetAutostart", libvirt_virNetworkGetAutostart, METH_VARARGS, NULL}, {(char *) "virDomainBlockStats", libvirt_virDomainBlockStats, METH_VARARGS, NULL}, {(char *) "virDomainInterfaceStats", libvirt_virDomainInterfaceStats, METH_VARARGS, NULL}, + {(char *) "virDomainMemStats", libvirt_virDomainMemStats, METH_VARARGS, NULL}, {(char *) "virNodeGetCellsFreeMemory", libvirt_virNodeGetCellsFreeMemory, METH_VARARGS, NULL}, {(char *) "virDomainGetSchedulerType", libvirt_virDomainGetSchedulerType, METH_VARARGS, NULL}, {(char *) "virDomainGetSchedulerParameters", libvirt_virDomainGetSchedulerParameters, METH_VARARGS, NULL}, -- 1.6.5

On Tue, Dec 08, 2009 at 02:57:15PM -0500, Adam Litke wrote:
Set up the types for the domainMemStats function and insert it into the virDriver structure definition. Because of static initializers, update every driver and set the new field to NULL.
include/libvirt/libvirt.h.in | 31 +++++++++++++++++++++++++++++++
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index f51a565..e430599 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -333,6 +333,34 @@ struct _virDomainInterfaceStats { */ typedef virDomainInterfaceStatsStruct *virDomainInterfaceStatsPtr;
+/** + * virDomainMemStats: + * + * Memory stats for virDomainMemStats. + * + * Hypervisors may return a field set to ((long long)-1) which indicates + * that the hypervisor or guest does not support that statistic. + * + * NB. Here 'long long' means 64 bit integer. + */ +typedef struct _virDomainMemStats virDomainMemStatsStruct; + +struct _virDomainMemStats { + unsigned long long swap_in; + unsigned long long swap_out; + unsigned long long major_fault; + unsigned long long minor_fault; + unsigned long long mem_free; + unsigned long long mem_tot; +}; + +/** + * virDomainMemStatsPtr: + * + * A pointer to a virDomainMemStats structure + */ +typedef virDomainMemStatsStruct *virDomainMemStatsPtr; +
+int virDomainMemStats (virDomainPtr dom, + virDomainMemStatsPtr stats, + size_t size); int virDomainBlockPeek (virDomainPtr dom, const char *path, unsigned long long offset,
This is the part where we need discussion, if we introduce an new API it cannot be tuned for a single hypervisor, and as raised already the amount of informations exposed there don't match what other are providing. IMHO everything else is rather simple compared to the API definition. The problem of a struct like this is that it doesn't allow expansion, which is why I assume you added the size argument, but it's different from what we did for the other entry points, I'm not sure it will really be sufficient to handle all cases, for example if you remote call to a libvirtd on an older version how do you handle the structure size change and mostly the initialization of the fields unknown on the remote end. The semantic of all your fields need to be precisely described (there isn't even a comment for them !) for examples can you formally define the difference of semantic between a major_fault and a minor_fault ? Would mem_free include usage say by the buffer cache on linux ? What mem_tot means the memory used ? the memory available ? etc ... You need to define precisely the semantic of all the values you want to export, then we can compare with the semantic exported by other hypervisors, and make a decision. Without it, there is now way we can make use of that patch or API in general. thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

2009/12/8 Adam Litke <agl@us.ibm.com>:
When using ballooning to manage overcommitted memory on a host, a system for guests to communicate their memory usage to the host can provide information that will minimize the impact of ballooning on the guests while maximizing efficient use of host memory.
The design of such a communication channel was recently added to version 0.8.2 of the VirtIO Spec (See Appendix G): - http://ozlabs.org/~rusty/virtio-spec/virtio-spec-0.8.2.pdf
Host-side and guest-side implementations have been accepted for inclusion in their respective projects: - Guest: http://lkml.org/lkml/2009/11/30/274 - Host: http://lists.gnu.org/archive/html/qemu-devel/2009-12/msg00380.html
The following patch series implements a new libvirt interface to expose these memory statistics. Thank you for your review and comments.
[PATCH 1/6] domMemStats: Add domainMemStats method to struct _virDriver [PATCH 2/6] domMemStats: Add public symbol to libvirt API [PATCH 3/6] qemu-driver: Enable domainMemStats in the qemu driver [PATCH 4/6] remote-driver: Add domainMemStats support [PATCH 5/6] virsh: Enable virDomainMemStats as a new command [PATCH 6/6] python: Add python bindings for virDomainMemStats
Some general comments: - You should name it virDomainMemoryStats to stay in sync with the naming scheme. - The struct members are an exact copy of that what virtio provides, maybe we should see what different hypervisors can provide, discuss what the useful parts are and define a general superset. For example VMware ESX provides more/other memory statistics: http://www.vmware.com/support/developer/vc-sdk/visdk400pubs/ReferenceGuide/m... - You're missing descriptions (including context and meaning) and units for the members of the stats struct. Matthias

On Wed, Dec 9, 2009 at 9:41 AM, Matthias Bolte <matthias.bolte@googlemail.com> wrote:
Some general comments:
- You should name it virDomainMemoryStats to stay in sync with the naming scheme.
- The struct members are an exact copy of that what virtio provides, maybe we should see what different hypervisors can provide, discuss what the useful parts are and define a general superset. For example VMware ESX provides more/other memory statistics: http://www.vmware.com/support/developer/vc-sdk/visdk400pubs/ReferenceGuide/m...
Yes, in LXC case, we will get such memory statistics via cgroup.memory and they are like: cache 2375860224 rss 57614336 mapped_file 3731456 pgpgin 131483871 pgpgout 130889761 swap 32768 inactive_anon 32731136 active_anon 24948736 inactive_file 1878589440 active_file 497172480 unevictable 0 ... ozaki-r

On Tue, Dec 08, 2009 at 02:57:14PM -0500, Adam Litke wrote:
When using ballooning to manage overcommitted memory on a host, a system for guests to communicate their memory usage to the host can provide information that will minimize the impact of ballooning on the guests while maximizing efficient use of host memory.
The design of such a communication channel was recently added to version 0.8.2 of the VirtIO Spec (See Appendix G): - http://ozlabs.org/~rusty/virtio-spec/virtio-spec-0.8.2.pdf
Host-side and guest-side implementations have been accepted for inclusion in their respective projects: - Guest: http://lkml.org/lkml/2009/11/30/274 - Host: http://lists.gnu.org/archive/html/qemu-devel/2009-12/msg00380.html
The following patch series implements a new libvirt interface to expose these memory statistics. Thank you for your review and comments.
I've looked at the code in all the patches, and it all looks good to me. As others have said the main issue at hand is to get a better definition of the set of statistics we're going to have in the public struct. Even though you have included a size_t parameter in the public API, this does not get usa free ride for the future, because the set of fields is also part of the RPC wire protocol ABI, and that is not so simple to deal with. This is a going to be quite a popular/useful API for all of our hypervisor drivers. It definitely something we can easily implement for VMWare ESX, OpenVZ, and LXC drivers too. So we should try to get a struct definition that covers the memory stats required by all Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Thu, 2009-12-10 at 11:54 +0000, Daniel P. Berrange wrote:
I've looked at the code in all the patches, and it all looks good to me. As others have said the main issue at hand is to get a better definition of the set of statistics we're going to have in the public struct.
Even though you have included a size_t parameter in the public API, this does not get usa free ride for the future, because the set of fields is also part of the RPC wire protocol ABI, and that is not so simple to deal with.
This is a going to be quite a popular/useful API for all of our hypervisor drivers. It definitely something we can easily implement for VMWare ESX, OpenVZ, and LXC drivers too. So we should try to get a struct definition that covers the memory stats required by all
Thanks for your review. I am working on an alternative way of marshaling the statistics. I will work on providing more concise definitions of each current statistic as well. -- Thanks, Adam
participants (5)
-
Adam Litke
-
Daniel P. Berrange
-
Daniel Veillard
-
Matthias Bolte
-
Ryota Ozaki