On Fri, Feb 20, 2015 at 10:15:00 -0500, John Ferlan wrote:
On 02/18/2015 09:16 AM, Peter Krempa wrote:
As there are two possible approaches to define a domain's memory size - one used with legacy, non-NUMA VMs configured in the <memory> element and per-node based approach on NUMA machines - the user needs to make sure that both are specified correctly in the NUMA case.
To avoid this burden on the user I'd like to replace the NUMA case with automatic totaling of the memory size. To achieve this I need to replace direct access to the virDomainMemtune's 'max_balloon' field with two separate getters depending on the desired size.
The two sizes are needed as: 1) Startup memory size doesn't include memory modules in some hypervisors. 2) After startup these count as the usable memory size.
Note that the comments for the functions are future aware and document state that will be present after a few later patches. --- src/conf/domain_conf.c | 66 ++++++++++++++++++++++++++++++++++------ src/conf/domain_conf.h | 4 +++ src/hyperv/hyperv_driver.c | 2 +- src/libvirt_private.syms | 3 ++ src/libxl/libxl_conf.c | 2 +- src/libxl/libxl_driver.c | 8 ++--- src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_driver.c | 10 +++--- src/lxc/lxc_fuse.c | 4 +-- src/lxc/lxc_native.c | 4 +-- src/openvz/openvz_driver.c | 2 +- src/parallels/parallels_driver.c | 2 +- src/parallels/parallels_sdk.c | 12 ++++---- src/phyp/phyp_driver.c | 11 ++++--- src/qemu/qemu_command.c | 18 +++++++---- src/qemu/qemu_driver.c | 19 ++++++------ src/qemu/qemu_hotplug.c | 8 +++-- src/qemu/qemu_process.c | 2 +- src/test/test_driver.c | 8 ++--- src/uml/uml_driver.c | 8 ++--- src/vbox/vbox_common.c | 4 +-- src/vmware/vmware_driver.c | 2 +- src/vmx/vmx.c | 12 ++++---- src/xen/xm_internal.c | 14 ++++----- src/xenapi/xenapi_driver.c | 2 +- src/xenapi/xenapi_utils.c | 4 +-- src/xenconfig/xen_common.c | 8 +++-- src/xenconfig/xen_sxpr.c | 9 +++--- 28 files changed, 160 insertions(+), 90 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6ef499d..e95851a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3172,24 +3172,26 @@ virDomainDefPostParseInternal(virDomainDefPtr def, return -1; }
- if (def->mem.cur_balloon > def->mem.max_balloon) { + if (def->mem.cur_balloon > virDomainDefGetMemoryCurrent(def)) { /* Older libvirt could get into this situation due to * rounding; if the discrepancy is less than 4MiB, we silently * round down, otherwise we flag the issue. */ if (VIR_DIV_UP(def->mem.cur_balloon, 4096) > - VIR_DIV_UP(def->mem.max_balloon, 4096)) { + VIR_DIV_UP(virDomainDefGetMemoryCurrent(def), 4096)) { virReportError(VIR_ERR_XML_ERROR, _("current memory '%lluk' exceeds " "maximum '%lluk'"), - def->mem.cur_balloon, def->mem.max_balloon); + def->mem.cur_balloon, + virDomainDefGetMemoryCurrent(def)); return -1; } else { VIR_DEBUG("Truncating current %lluk to maximum %lluk", - def->mem.cur_balloon, def->mem.max_balloon); - def->mem.cur_balloon = def->mem.max_balloon; + def->mem.cur_balloon, + virDomainDefGetMemoryCurrent(def)); + def->mem.cur_balloon = virDomainDefGetMemoryCurrent(def); } } else if (def->mem.cur_balloon == 0) { - def->mem.cur_balloon = def->mem.max_balloon; + def->mem.cur_balloon = virDomainDefGetMemoryCurrent(def); }
/* @@ -6882,6 +6884,51 @@ virDomainParseMemory(const char *xpath, }
+/** + * virDomainDefGetMemoryInitial: + * @def: domain definition + * + * Returns the size of the initial amount of guest memory. The initial amount + * is the memory size is either the configured amount in the <memory> element + * or the sum of memory sizes of NUMA nodes in case NUMA is enabled in @def. + */ +unsigned long long +virDomainDefGetMemoryInitial(virDomainDefPtr def) +{ + return def->mem.max_balloon; +} + + +/** + * virDomainDefSetMemoryInitial: + * @def: domain definition + * @size: size to set + * + * Sets the initial memory size in @def. + */ +void +virDomainDefSetMemoryInitial(virDomainDefPtr def, + unsigned long long size) +{ + def->mem.max_balloon = size; +}
Odd concept - changing Initial memory, but I don't have a better name in mind...
+ + +/** + * virDomainDefGetMemoryCurrent: + * @def: domain definition + * + * Returns the current maximum memory size usable by the domain described by + * @def. This size is a sum of size returned by virDomainDefGetMemoryInitial + * and possible additional memory devices. + */ +unsigned long long +virDomainDefGetMemoryCurrent(virDomainDefPtr def) +{ + return virDomainDefGetMemoryInitial(def); +}
Ok - so part of me expect this one to return mem.cur_balloon - perhaps a name change to GetMemoryUsable would help?
I'll try using GetMemoryAvailable for the next version.
and of course begs the question why not bite the bullet and return mem.cur_balloon in a GetMemoryCurrent accessor...
I didn't need it currently. I'll look into it separately if I find it necessary.
+ + static int virDomainControllerModelTypeFromString(const virDomainControllerDef *def, const char *model) @@ -15956,10 +16003,11 @@ virDomainDefCheckABIStability(virDomainDefPtr src, goto error; }
- if (src->mem.max_balloon != dst->mem.max_balloon) { + if (virDomainDefGetMemoryInitial(src) != virDomainDefGetMemoryInitial(dst)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain max memory %lld does not match source %lld"), - dst->mem.max_balloon, src->mem.max_balloon); + virDomainDefGetMemoryInitial(dst), + virDomainDefGetMemoryInitial(src)); goto error; }
Since the "future" (I assume) is that MemoryCurrent will include the hot (un)plug memory - does it make sense to also compare src/dst MemoryCurrent values even though for now it's essentially the same value? Although I believe that gets done in your later series and it's hard to keep them in my memory.
The memory modules will be compared individually so it doesn't matter much.
Of the remaining uses of GetInitial and {Get|Set}Current the ones I was wondering about are:
virLXCCgroupSetupMemTune libxlMakeDomBuildInfo phypBuildLpar
Wouldn't they be the corollary to qemuBuildCommandLine w/r/t using Initial instead of Current in order to define the memory for the guest as it starts up?
They are. I'll change them to use the Current/Available accessor although it won't make a difference for now as memory hotplug is explicitly forbidden for drivers other than qemu.
John
Peter