[libvirt] [PATCHv2 0/7] Automaticaly fill <memory> element for NUMA enabled guests

This series changes the behavior of the <memory> element in case the guest has NUMA enabled and fills automatically the sum of sizes of numa nodes instead of relying on the user passing correct data. Peter Krempa (7): qemu: Forbid setting maximum memory size with the API with NUMA enabled qemu: lxc: Clarify error message when setting current memory conf: Replace access to def->mem.max_balloon with accessor functions phyp: Fix error messages mentioning memory qemu: command: Add helper to align memory sizes conf: numa: Add helper to count total memory size configured in NUMA conf: Automatically use NUMA memory size in case NUMA is enabled src/conf/domain_conf.c | 72 +++++++++++++++++++--- src/conf/domain_conf.h | 4 ++ src/conf/numa_conf.c | 13 ++++ src/conf/numa_conf.h | 2 + src/hyperv/hyperv_driver.c | 2 +- src/libvirt_private.syms | 4 ++ src/libxl/libxl_conf.c | 2 +- src/libxl/libxl_driver.c | 8 +-- src/lxc/lxc_cgroup.c | 2 +- src/lxc/lxc_driver.c | 28 +++++---- 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 | 15 +++-- src/qemu/qemu_command.c | 23 ++++--- src/qemu/qemu_domain.c | 21 +++++++ src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 49 ++++++++++----- 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 +-- .../qemuxml2argv-numatune-memnode.args | 2 +- 33 files changed, 243 insertions(+), 111 deletions(-) -- 2.2.2

NUMA enabled guest configuration explicitly specifies memory sizes for individual nodes. Allowing the virDomainSetMemoryFlags API (and friends) to change the total doesn't make sense as the individual node configs are not updated in that case. Forbid use of the API in case NUMA is specified. --- Notes: Version 2: - fixed typo in subject - fixed incomplete sentence in comment - clarified that LXC doesn't use NUMA to the extent where this change would make sense in the LXC driver src/qemu/qemu_driver.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bec05d4..27cb3bf 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2307,6 +2307,16 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Help clang 2.8 decipher the logic flow. */ sa_assert(persistentDef); + + /* resizing memory with NUMA nodes specified doesn't work as there + * is no way to decrease the individual node sizes with this API */ + if (virDomainNumaGetNodeCount(persistentDef->numa) > 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("maximum memory size of a domain with NUMA " + "nodes cannot be modified with this API")); + goto endjob; + } + persistentDef->mem.max_balloon = newmem; if (persistentDef->mem.cur_balloon > newmem) persistentDef->mem.cur_balloon = newmem; -- 2.2.2

On Wed, Feb 25, 2015 at 11:21:58 +0100, Peter Krempa wrote:
NUMA enabled guest configuration explicitly specifies memory sizes for individual nodes. Allowing the virDomainSetMemoryFlags API (and friends) to change the total doesn't make sense as the individual node configs are not updated in that case.
Forbid use of the API in case NUMA is specified. ---
OOPS, I forgot to clean my repository of the previous patches and accidentaly reposted them. Sorry for that. Peter

On Wed, Feb 25, 2015 at 11:21:58AM +0100, Peter Krempa wrote:
NUMA enabled guest configuration explicitly specifies memory sizes for individual nodes. Allowing the virDomainSetMemoryFlags API (and friends) to change the total doesn't make sense as the individual node configs are not updated in that case.
Forbid use of the API in case NUMA is specified. ---
Notes: Version 2: - fixed typo in subject - fixed incomplete sentence in comment - clarified that LXC doesn't use NUMA to the extent where this change would make sense in the LXC driver
src/qemu/qemu_driver.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bec05d4..27cb3bf 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2307,6 +2307,16 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Help clang 2.8 decipher the logic flow. */ sa_assert(persistentDef); + + /* resizing memory with NUMA nodes specified doesn't work as there + * is no way to decrease the individual node sizes with this API */
s/decrease/change/ as I believe increase isn't possible as well. ACK with this squashed in (or added flag that will proportionally change node memory sizes): diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index 471b583..60c97ad 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -2309,7 +2309,7 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, sa_assert(persistentDef); /* resizing memory with NUMA nodes specified doesn't work as there - * is no way to decrease the individual node sizes with this API */ + * is no way to changee individual node sizes with this API */ if (virDomainNumaGetNodeCount(persistentDef->numa) > 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("maximum memory size of a domain with NUMA " diff --git i/tools/virsh.pod w/tools/virsh.pod index 343f26f..408cac5 100644 --- i/tools/virsh.pod +++ w/tools/virsh.pod @@ -1937,7 +1937,10 @@ exclusive. If no flag is specified, behavior is different depending on hypervisor. Some hypervisors such as QEMU/KVM don't support live changes (especially -increasing) of the maximum memory limit. +increasing) of the maximum memory limit. Even persistent configuration changes +might not be performed with some hypervisors/configuration (e.g. on NUMA enabled +domains on QEMU). For complex configuration changes use command B<edit> +instead). I<size> is a scaled integer (see B<NOTES> above); it defaults to kibibytes (blocks of 1024 bytes) unless you provide a suffix (and the older option --

On Thu, Feb 26, 2015 at 01:20:29PM +0100, Martin Kletzander wrote:
On Wed, Feb 25, 2015 at 11:21:58AM +0100, Peter Krempa wrote:
NUMA enabled guest configuration explicitly specifies memory sizes for individual nodes. Allowing the virDomainSetMemoryFlags API (and friends) to change the total doesn't make sense as the individual node configs are not updated in that case.
Forbid use of the API in case NUMA is specified. ---
Notes: Version 2: - fixed typo in subject - fixed incomplete sentence in comment - clarified that LXC doesn't use NUMA to the extent where this change would make sense in the LXC driver
src/qemu/qemu_driver.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bec05d4..27cb3bf 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2307,6 +2307,16 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Help clang 2.8 decipher the logic flow. */ sa_assert(persistentDef); + + /* resizing memory with NUMA nodes specified doesn't work as there + * is no way to decrease the individual node sizes with this API */
s/decrease/change/ as I believe increase isn't possible as well.
ACK with this squashed in (or added flag that will proportionally change node memory sizes):
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index 471b583..60c97ad 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -2309,7 +2309,7 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, sa_assert(persistentDef);
/* resizing memory with NUMA nodes specified doesn't work as there - * is no way to decrease the individual node sizes with this API */ + * is no way to changee individual node sizes with this API */
s/changee/change/

On Thu, Feb 26, 2015 at 15:15:27 +0100, Martin Kletzander wrote:
On Thu, Feb 26, 2015 at 01:20:29PM +0100, Martin Kletzander wrote:
On Wed, Feb 25, 2015 at 11:21:58AM +0100, Peter Krempa wrote:
NUMA enabled guest configuration explicitly specifies memory sizes for individual nodes. Allowing the virDomainSetMemoryFlags API (and friends) to change the total doesn't make sense as the individual node configs are not updated in that case.
Forbid use of the API in case NUMA is specified. ---
Notes: Version 2: - fixed typo in subject - fixed incomplete sentence in comment - clarified that LXC doesn't use NUMA to the extent where this change would make sense in the LXC driver
src/qemu/qemu_driver.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bec05d4..27cb3bf 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2307,6 +2307,16 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Help clang 2.8 decipher the logic flow. */ sa_assert(persistentDef); + + /* resizing memory with NUMA nodes specified doesn't work as there + * is no way to decrease the individual node sizes with this API */
s/decrease/change/ as I believe increase isn't possible as well.
ACK with this squashed in (or added flag that will proportionally change node memory sizes):
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index 471b583..60c97ad 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -2309,7 +2309,7 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, sa_assert(persistentDef);
/* resizing memory with NUMA nodes specified doesn't work as there - * is no way to decrease the individual node sizes with this API */ + * is no way to changee individual node sizes with this API */
s/changee/change/
Pushed with the man page addition you've suggested. Thanks. Peter

Commit 60f7303c151cccdbe214b9f9ac59ecaf95cbf24b introduced the error message but it's unclear whether the persistent config or the live config tripped the message. Later the LXC driver copied the same code. Separate the message which will also clarify the code. --- Notes: Version 2: - reworded error message src/lxc/lxc_driver.c | 22 +++++++++++++--------- src/qemu/qemu_driver.c | 24 ++++++++++++++---------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 3adb21d..fea9599 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -742,18 +742,22 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, goto cleanup; } } else { - unsigned long oldmax = 0; - - if (flags & VIR_DOMAIN_AFFECT_LIVE) - oldmax = vm->def->mem.max_balloon; - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!oldmax || oldmax > persistentDef->mem.max_balloon) - oldmax = persistentDef->mem.max_balloon; + if (flags & VIR_DOMAIN_AFFECT_LIVE && + newmem > vm->def->mem.max_balloon) { + virReportError(VIR_ERR_INVALID_ARG, + _("new memory value '%lu' cannot be greater than " + "the maximum value '%llu' for the live instance"), + newmem, vm->def->mem.max_balloon); + goto cleanup; } - if (newmem > oldmax) { + if (flags & VIR_DOMAIN_AFFECT_CONFIG && + newmem > persistentDef->mem.max_balloon) { virReportError(VIR_ERR_INVALID_ARG, - "%s", _("Cannot set memory higher than max memory")); + _("new memory value '%lu' cannot be greater than " + "the maximum value '%llu' for the persistent " + "configuration"), + newmem, persistentDef->mem.max_balloon); goto cleanup; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 27cb3bf..bf19de8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2326,18 +2326,22 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, } else { /* resize the current memory */ - unsigned long oldmax = 0; - - if (flags & VIR_DOMAIN_AFFECT_LIVE) - oldmax = vm->def->mem.max_balloon; - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!oldmax || oldmax > persistentDef->mem.max_balloon) - oldmax = persistentDef->mem.max_balloon; + if (flags & VIR_DOMAIN_AFFECT_LIVE && + newmem > vm->def->mem.max_balloon) { + virReportError(VIR_ERR_INVALID_ARG, + _("new memory value '%lu' cannot be greater than " + "the maximum value '%llu' for the live instance"), + newmem, vm->def->mem.max_balloon); + goto endjob; } - if (newmem > oldmax) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("cannot set memory higher than max memory")); + if (flags & VIR_DOMAIN_AFFECT_CONFIG && + newmem > persistentDef->mem.max_balloon) { + virReportError(VIR_ERR_INVALID_ARG, + _("new memory value '%lu' cannot be greater than " + "the maximum value '%llu' for the persistent " + "configuration"), + newmem, persistentDef->mem.max_balloon); goto endjob; } -- 2.2.2

On Wed, Feb 25, 2015 at 11:22:00AM +0100, Peter Krempa wrote:
Commit 60f7303c151cccdbe214b9f9ac59ecaf95cbf24b introduced the error message but it's unclear whether the persistent config or the live config tripped the message. Later the LXC driver copied the same code.
Separate the message which will also clarify the code. ---
Notes: Version 2: - reworded error message
src/lxc/lxc_driver.c | 22 +++++++++++++--------- src/qemu/qemu_driver.c | 24 ++++++++++++++---------- 2 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 3adb21d..fea9599 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -742,18 +742,22 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, goto cleanup; } } else { - unsigned long oldmax = 0; - - if (flags & VIR_DOMAIN_AFFECT_LIVE) - oldmax = vm->def->mem.max_balloon; - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!oldmax || oldmax > persistentDef->mem.max_balloon) - oldmax = persistentDef->mem.max_balloon; + if (flags & VIR_DOMAIN_AFFECT_LIVE && + newmem > vm->def->mem.max_balloon) { + virReportError(VIR_ERR_INVALID_ARG, + _("new memory value '%lu' cannot be greater than " + "the maximum value '%llu' for the live instance"),
I sense disturbance in the force that this might clash in the future with "something" ;) Either remove "maximum" completely or change it to "memory" or something. Use neither "maximum" nor "current" in the commit message to make devs' users' lives easier. ACK with that changed. <rant> I must say I totally hate how the hot-plug turned out WRT naming. Still hoping we can do something with it. </rant>
+ newmem, vm->def->mem.max_balloon); + goto cleanup; }
- if (newmem > oldmax) { + if (flags & VIR_DOMAIN_AFFECT_CONFIG && + newmem > persistentDef->mem.max_balloon) { virReportError(VIR_ERR_INVALID_ARG, - "%s", _("Cannot set memory higher than max memory")); + _("new memory value '%lu' cannot be greater than " + "the maximum value '%llu' for the persistent " + "configuration"), + newmem, persistentDef->mem.max_balloon); goto cleanup; }
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 27cb3bf..bf19de8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2326,18 +2326,22 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
} else { /* resize the current memory */ - unsigned long oldmax = 0; - - if (flags & VIR_DOMAIN_AFFECT_LIVE) - oldmax = vm->def->mem.max_balloon; - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!oldmax || oldmax > persistentDef->mem.max_balloon) - oldmax = persistentDef->mem.max_balloon; + if (flags & VIR_DOMAIN_AFFECT_LIVE && + newmem > vm->def->mem.max_balloon) { + virReportError(VIR_ERR_INVALID_ARG, + _("new memory value '%lu' cannot be greater than " + "the maximum value '%llu' for the live instance"), + newmem, vm->def->mem.max_balloon); + goto endjob; }
- if (newmem > oldmax) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("cannot set memory higher than max memory")); + if (flags & VIR_DOMAIN_AFFECT_CONFIG && + newmem > persistentDef->mem.max_balloon) { + virReportError(VIR_ERR_INVALID_ARG, + _("new memory value '%lu' cannot be greater than " + "the maximum value '%llu' for the persistent " + "configuration"), + newmem, persistentDef->mem.max_balloon); goto endjob; }
-- 2.2.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

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. --- Notes: Version 2: - renamed virDomainDefGetMemoryCurrent to virDomainDefGetMemoryActual - changed to the initial memory accessor when starting VMs in LXC, phyp and libxl 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 | 14 ++++----- 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 | 23 +++++++------- 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, 164 insertions(+), 94 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d95dd3e..b41ae17 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3174,24 +3174,26 @@ virDomainDefPostParseInternal(virDomainDefPtr def, return -1; } - if (def->mem.cur_balloon > def->mem.max_balloon) { + if (def->mem.cur_balloon > virDomainDefGetMemoryActual(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(virDomainDefGetMemoryActual(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, + virDomainDefGetMemoryActual(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, + virDomainDefGetMemoryActual(def)); + def->mem.cur_balloon = virDomainDefGetMemoryActual(def); } } else if (def->mem.cur_balloon == 0) { - def->mem.cur_balloon = def->mem.max_balloon; + def->mem.cur_balloon = virDomainDefGetMemoryActual(def); } /* @@ -6891,6 +6893,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; +} + + +/** + * virDomainDefGetMemoryActual: + * @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 +virDomainDefGetMemoryActual(virDomainDefPtr def) +{ + return virDomainDefGetMemoryInitial(def); +} + + static int virDomainControllerModelTypeFromString(const virDomainControllerDef *def, const char *model) @@ -15974,10 +16021,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; } if (src->mem.cur_balloon != dst->mem.cur_balloon) { @@ -19679,7 +19727,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAsprintf(buf, " dumpCore='%s'", virTristateSwitchTypeToString(def->mem.dump_core)); virBufferAsprintf(buf, " unit='KiB'>%llu</memory>\n", - def->mem.max_balloon); + virDomainDefGetMemoryActual(def)); virBufferAsprintf(buf, "<currentMemory unit='KiB'>%llu</currentMemory>\n", def->mem.cur_balloon); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 02ddd93..06ce8e7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2199,6 +2199,10 @@ struct _virDomainDef { xmlNodePtr metadata; }; +unsigned long long virDomainDefGetMemoryInitial(virDomainDefPtr def); +void virDomainDefSetMemoryInitial(virDomainDefPtr def, unsigned long long size); +unsigned long long virDomainDefGetMemoryActual(virDomainDefPtr def); + typedef enum { VIR_DOMAIN_TAINT_CUSTOM_ARGV, /* Custom ARGV passthrough from XML */ VIR_DOMAIN_TAINT_CUSTOM_MONITOR, /* Custom monitor commands issued */ diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 906c603..00169c7 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -870,7 +870,7 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) if (VIR_STRDUP(def->description, virtualSystemSettingData->data->Notes) < 0) goto cleanup; - def->mem.max_balloon = memorySettingData->data->Limit * 1024; /* megabyte to kilobyte */ + virDomainDefSetMemoryInitial(def, memorySettingData->data->Limit * 1024); /* megabyte to kilobyte */ def->mem.cur_balloon = memorySettingData->data->VirtualQuantity * 1024; /* megabyte to kilobyte */ def->vcpus = processorSettingData->data->VirtualQuantity; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c156b40..d4d0faf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -198,6 +198,8 @@ virDomainDefFormatConvertXMLFlags; virDomainDefFormatInternal; virDomainDefFree; virDomainDefGetDefaultEmulator; +virDomainDefGetMemoryActual; +virDomainDefGetMemoryInitial; virDomainDefGetSecurityLabelDef; virDomainDefHasDeviceAddress; virDomainDefMaybeAddController; @@ -209,6 +211,7 @@ virDomainDefParseFile; virDomainDefParseNode; virDomainDefParseString; virDomainDefPostParse; +virDomainDefSetMemoryInitial; virDomainDeleteConfig; virDomainDeviceAddressIsValid; virDomainDeviceAddressTypeToString; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 2321660..997b73c 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -659,7 +659,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, } } b_info->sched_params.weight = 1000; - b_info->max_memkb = def->mem.max_balloon; + b_info->max_memkb = virDomainDefGetMemoryInitial(def); b_info->target_memkb = def->mem.cur_balloon; if (hvm) { char bootorder[VIR_DOMAIN_BOOT_LAST + 1]; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index ce3a99b..ada61c3 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1075,7 +1075,7 @@ libxlDomainGetMaxMemory(virDomainPtr dom) if (virDomainGetMaxMemoryEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - ret = vm->def->mem.max_balloon; + ret = virDomainDefGetMemoryActual(vm->def); cleanup: if (vm) @@ -1157,7 +1157,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, if (flags & VIR_DOMAIN_MEM_CONFIG) { /* Help clang 2.8 decipher the logic flow. */ sa_assert(persistentDef); - persistentDef->mem.max_balloon = newmem; + virDomainDefSetMemoryInitial(persistentDef, newmem); if (persistentDef->mem.cur_balloon > newmem) persistentDef->mem.cur_balloon = newmem; ret = virDomainSaveConfig(cfg->configDir, persistentDef); @@ -1167,7 +1167,7 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, } else { /* resize the current memory */ - if (newmem > vm->def->mem.max_balloon) { + if (newmem > virDomainDefGetMemoryActual(vm->def)) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("cannot set memory higher than max memory")); goto endjob; @@ -1241,7 +1241,7 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) if (!virDomainObjIsActive(vm)) { info->cpuTime = 0; info->memory = vm->def->mem.cur_balloon; - info->maxMem = vm->def->mem.max_balloon; + info->maxMem = virDomainDefGetMemoryActual(vm->def); } else { if (libxl_domain_info(priv->ctx, &d_info, vm->def->id) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 8e46a01..8c631d5 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -146,7 +146,7 @@ static int virLXCCgroupSetupMemTune(virDomainDefPtr def, { int ret = -1; - if (virCgroupSetMemory(cgroup, def->mem.max_balloon) < 0) + if (virCgroupSetMemory(cgroup, virDomainDefGetMemoryInitial(def)) < 0) goto cleanup; if (def->mem.hard_limit && diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index fea9599..2aa8809 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -617,7 +617,7 @@ static int lxcDomainGetInfo(virDomainPtr dom, } } - info->maxMem = vm->def->mem.max_balloon; + info->maxMem = virDomainDefGetMemoryActual(vm->def); info->nrVirtCpu = vm->def->vcpus; ret = 0; @@ -686,7 +686,7 @@ lxcDomainGetMaxMemory(virDomainPtr dom) if (virDomainGetMaxMemoryEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - ret = vm->def->mem.max_balloon; + ret = virDomainDefGetMemoryActual(vm->def); cleanup: if (vm) @@ -735,7 +735,7 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - persistentDef->mem.max_balloon = newmem; + virDomainDefSetMemoryInitial(persistentDef, newmem); if (persistentDef->mem.cur_balloon > newmem) persistentDef->mem.cur_balloon = newmem; if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0) @@ -743,21 +743,21 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, } } else { if (flags & VIR_DOMAIN_AFFECT_LIVE && - newmem > vm->def->mem.max_balloon) { + newmem > virDomainDefGetMemoryActual(vm->def)) { virReportError(VIR_ERR_INVALID_ARG, _("new memory value '%lu' cannot be greater than " "the maximum value '%llu' for the live instance"), - newmem, vm->def->mem.max_balloon); + newmem, virDomainDefGetMemoryActual(vm->def)); goto cleanup; } if (flags & VIR_DOMAIN_AFFECT_CONFIG && - newmem > persistentDef->mem.max_balloon) { + newmem > virDomainDefGetMemoryActual(persistentDef)) { virReportError(VIR_ERR_INVALID_ARG, _("new memory value '%lu' cannot be greater than " "the maximum value '%llu' for the persistent " "configuration"), - newmem, persistentDef->mem.max_balloon); + newmem, virDomainDefGetMemoryActual(persistentDef)); goto cleanup; } diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 5c18cff..d465c60 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -165,11 +165,11 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def, *ptr = '\0'; if (STREQ(line, "MemTotal") && - (def->mem.hard_limit || def->mem.max_balloon)) { + (def->mem.hard_limit || virDomainDefGetMemoryActual(def))) { virBufferAsprintf(new_meminfo, "MemTotal: %8llu kB\n", meminfo.memtotal); } else if (STREQ(line, "MemFree") && - (def->mem.hard_limit || def->mem.max_balloon)) { + (def->mem.hard_limit || virDomainDefGetMemoryActual(def))) { virBufferAsprintf(new_meminfo, "MemFree: %8llu kB\n", (meminfo.memtotal - meminfo.memusage)); } else if (STREQ(line, "Buffers")) { diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index abf07ce..b3b72bc 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -772,7 +772,7 @@ lxcSetMemTune(virDomainDefPtr def, virConfPtr properties) if (lxcConvertSize(value->str, &size) < 0) return -1; size = size / 1024; - def->mem.max_balloon = size; + virDomainDefSetMemoryInitial(def, size); def->mem.hard_limit = size; } @@ -1012,7 +1012,7 @@ lxcParseConfigString(const char *config) } vmdef->id = -1; - vmdef->mem.max_balloon = 64 * 1024; + virDomainDefSetMemoryInitial(vmdef, 64 * 1024); vmdef->onReboot = VIR_DOMAIN_LIFECYCLE_RESTART; vmdef->onCrash = VIR_DOMAIN_LIFECYCLE_DESTROY; diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 5a5cd8d..f03fedb 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -458,7 +458,7 @@ static int openvzDomainGetInfo(virDomainPtr dom, } } - info->maxMem = vm->def->mem.max_balloon; + info->maxMem = virDomainDefGetMemoryActual(vm->def); info->memory = vm->def->mem.cur_balloon; info->nrVirtCpu = vm->def->vcpus; ret = 0; diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index c9338b5..bb3928a 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -522,7 +522,7 @@ parallelsDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) info->state = virDomainObjGetState(privdom, NULL); info->memory = privdom->def->mem.cur_balloon; - info->maxMem = privdom->def->mem.max_balloon; + info->maxMem = virDomainDefGetMemoryActual(privdom->def); info->nrVirtCpu = privdom->def->vcpus; info->cpuTime = 0; ret = 0; diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index fb1f724..7f9f7ef 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -1213,9 +1213,9 @@ prlsdkLoadDomain(parallelsConnPtr privconn, /* get RAM parameters */ pret = PrlVmCfg_GetRamSize(sdkdom, &ram); prlsdkCheckRetGoto(pret, error); - def->mem.max_balloon = ram << 10; /* RAM size obtained in Mbytes, - convert to Kbytes */ - def->mem.cur_balloon = def->mem.max_balloon; + virDomainDefSetMemoryInitial(def, ram << 10); /* RAM size obtained in Mbytes, + convert to Kbytes */ + def->mem.cur_balloon = ram << 10; if (prlsdkConvertCpuInfo(sdkdom, def, pdom) < 0) goto error; @@ -1769,14 +1769,14 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr def) return -1; } - if (def->mem.max_balloon != def->mem.cur_balloon) { + if (virDomainDefGetMemoryActual(def) != def->mem.cur_balloon) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("changing balloon parameters is not supported " "by parallels driver")); return -1; } - if (def->mem.max_balloon % (1 << 10) != 0) { + if (virDomainDefGetMemoryActual(def) % (1 << 10) != 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Memory size should be multiple of 1Mb.")); return -1; @@ -2863,7 +2863,7 @@ prlsdkDoApplyConfig(PRL_HANDLE sdkdom, prlsdkCheckRetGoto(pret, error); } - pret = PrlVmCfg_SetRamSize(sdkdom, def->mem.max_balloon >> 10); + pret = PrlVmCfg_SetRamSize(sdkdom, virDomainDefGetMemoryActual(def) >> 10); prlsdkCheckRetGoto(pret, error); pret = PrlVmCfg_SetCpuCount(sdkdom, def->vcpus); diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index d69e29c..c1d6eb2 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3252,6 +3252,7 @@ phypDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) LIBSSH2_SESSION *session = phyp_driver->session; virDomainDef def; char *managed_system = phyp_driver->managed_system; + unsigned long long memory; /* Flags checked by virDomainDefFormat */ @@ -3273,12 +3274,13 @@ phypDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) goto err; } - if ((def.mem.max_balloon = - phypGetLparMem(dom->conn, managed_system, dom->id, 0)) == 0) { + if ((memory = phypGetLparMem(dom->conn, managed_system, dom->id, 0)) == 0) { VIR_ERROR(_("Unable to determine domain's max memory.")); goto err; } + virDomainDefSetMemoryInitial(&def, memory); + if ((def.mem.cur_balloon = phypGetLparMem(dom->conn, managed_system, dom->id, 1)) == 0) { VIR_ERROR(_("Unable to determine domain's memory.")); @@ -3491,7 +3493,7 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) goto cleanup; } - if (!def->mem.max_balloon) { + if (!virDomainDefGetMemoryInitial(def)) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Field <currentMemory> on the domain XML file is missing or " "has invalid value.")); @@ -3517,7 +3519,8 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) virBufferAsprintf(&buf, " -r lpar -p %s -i min_mem=%lld,desired_mem=%lld," "max_mem=%lld,desired_procs=%d,virtual_scsi_adapters=%s", def->name, def->mem.cur_balloon, - def->mem.cur_balloon, def->mem.max_balloon, + def->mem.cur_balloon, + virDomainDefGetMemoryInitial(def), (int) def->vcpus, virDomainDiskGetSource(def->disks[0])); ret = phypExecBuffer(session, &buf, &exit_status, conn, false); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fb3d5ab..13b54fe 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8319,8 +8319,9 @@ qemuBuildCommandLine(virConnectPtr conn, * XML to reflect our rounding. */ virCommandAddArg(cmd, "-m"); - def->mem.max_balloon = VIR_DIV_UP(def->mem.max_balloon, 1024) * 1024; - virCommandAddArgFormat(cmd, "%llu", def->mem.max_balloon / 1024); + virDomainDefSetMemoryInitial(def, VIR_ROUND_UP(virDomainDefGetMemoryInitial(def), 1024)); + virCommandAddArgFormat(cmd, "%llu", virDomainDefGetMemoryInitial(def) / 1024); + if (def->mem.nhugepages && !virDomainNumaGetNodeCount(def->numa)) { const long system_page_size = virGetSystemPageSizeKB(); char *mem_path = NULL; @@ -10337,8 +10338,11 @@ qemuBuildCommandLine(virConnectPtr conn, * space just to be safe (some finer tuning might be * nice, though). */ - memKB = def->mem.hard_limit ? - def->mem.hard_limit : def->mem.max_balloon + 1024 * 1024; + if (def->mem.hard_limit) + memKB = def->mem.hard_limit; + else + memKB = virDomainDefGetMemoryActual(def) + 1024 * 1024; + virCommandSetMaxMemLock(cmd, memKB * 1024); } @@ -11890,7 +11894,8 @@ qemuParseCommandLine(virCapsPtr qemuCaps, } def->id = -1; - def->mem.cur_balloon = def->mem.max_balloon = 64 * 1024; + def->mem.cur_balloon = 64 * 1024; + virDomainDefSetMemoryInitial(def, def->mem.cur_balloon); def->maxvcpus = 1; def->vcpus = 1; def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC; @@ -12098,7 +12103,8 @@ qemuParseCommandLine(virCapsPtr qemuCaps, _("cannot parse memory level '%s'"), val); goto error; } - def->mem.cur_balloon = def->mem.max_balloon = mem * 1024; + virDomainDefSetMemoryInitial(def, mem * 1024); + def->mem.cur_balloon = mem * 1024; } else if (STREQ(arg, "-smp")) { WANT_VALUE(); if (qemuParseCommandLineSmp(def, val) < 0) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bf19de8..903e6ff 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2255,7 +2255,7 @@ qemuDomainGetMaxMemory(virDomainPtr dom) if (virDomainGetMaxMemoryEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - ret = vm->def->mem.max_balloon; + ret = virDomainDefGetMemoryActual(vm->def); cleanup: qemuDomObjEndAPI(&vm); @@ -2317,7 +2317,8 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, goto endjob; } - persistentDef->mem.max_balloon = newmem; + virDomainDefSetMemoryInitial(persistentDef, newmem); + if (persistentDef->mem.cur_balloon > newmem) persistentDef->mem.cur_balloon = newmem; ret = virDomainSaveConfig(cfg->configDir, persistentDef); @@ -2327,21 +2328,21 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, } else { /* resize the current memory */ if (flags & VIR_DOMAIN_AFFECT_LIVE && - newmem > vm->def->mem.max_balloon) { + newmem > virDomainDefGetMemoryActual(vm->def)) { virReportError(VIR_ERR_INVALID_ARG, _("new memory value '%lu' cannot be greater than " "the maximum value '%llu' for the live instance"), - newmem, vm->def->mem.max_balloon); + newmem, virDomainDefGetMemoryActual(vm->def)); goto endjob; } if (flags & VIR_DOMAIN_AFFECT_CONFIG && - newmem > persistentDef->mem.max_balloon) { + newmem > virDomainDefGetMemoryActual(persistentDef)) { virReportError(VIR_ERR_INVALID_ARG, _("new memory value '%lu' cannot be greater than " "the maximum value '%llu' for the persistent " "configuration"), - newmem, persistentDef->mem.max_balloon); + newmem, virDomainDefGetMemoryActual(persistentDef)); goto endjob; } @@ -2598,14 +2599,14 @@ static int qemuDomainGetInfo(virDomainPtr dom, } } - info->maxMem = vm->def->mem.max_balloon; + info->maxMem = virDomainDefGetMemoryActual(vm->def); if (virDomainObjIsActive(vm)) { qemuDomainObjPrivatePtr priv = vm->privateData; if ((vm->def->memballoon != NULL) && (vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)) { - info->memory = vm->def->mem.max_balloon; + info->memory = virDomainDefGetMemoryActual(vm->def); } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BALLOON_EVENT)) { info->memory = vm->def->mem.cur_balloon; } else if (qemuDomainJobAllowed(priv, QEMU_JOB_QUERY)) { @@ -2631,7 +2632,7 @@ static int qemuDomainGetInfo(virDomainPtr dom, info->memory = vm->def->mem.cur_balloon; } else if (err == 0) { /* Balloon not supported, so maxmem is always the allocation */ - info->memory = vm->def->mem.max_balloon; + info->memory = virDomainDefGetMemoryActual(vm->def); } else { info->memory = balloon; } @@ -18431,7 +18432,7 @@ qemuDomainGetStatsBalloon(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, if (dom->def->memballoon && dom->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE) { - cur_balloon = dom->def->mem.max_balloon; + cur_balloon = virDomainDefGetMemoryActual(dom->def); } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BALLOON_EVENT)) { cur_balloon = dom->def->mem.cur_balloon; } else { @@ -18449,7 +18450,7 @@ qemuDomainGetStatsBalloon(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, &record->nparams, maxparams, "balloon.maximum", - dom->def->mem.max_balloon) < 0) + virDomainDefGetMemoryActual(dom->def)) < 0) return -1; return 0; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 8691c7e..98ac5ec 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1252,9 +1252,11 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, * Kibibytes, but virProcessSetMaxMemLock expects the value in * bytes. */ - memKB = vm->def->mem.hard_limit - ? vm->def->mem.hard_limit - : vm->def->mem.max_balloon + (1024 * 1024); + if (vm->def->mem.hard_limit) + memKB = vm->def->mem.hard_limit; + else + memKB = virDomainDefGetMemoryActual(vm->def) + (1024 * 1024); + virProcessSetMaxMemLock(vm->pid, memKB * 1024); break; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 515402e..3bdc10a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4632,7 +4632,7 @@ int qemuProcessStart(virConnectPtr conn, */ if (virDomainDefNeedsPlacementAdvice(vm->def)) { nodeset = virNumaGetAutoPlacementAdvice(vm->def->vcpus, - vm->def->mem.max_balloon); + virDomainDefGetMemoryActual(vm->def)); if (!nodeset) goto cleanup; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a386270..132c969 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2149,7 +2149,7 @@ static int testDomainGetInfo(virDomainPtr domain, info->state = virDomainObjGetState(privdom, NULL); info->memory = privdom->def->mem.cur_balloon; - info->maxMem = privdom->def->mem.max_balloon; + info->maxMem = virDomainDefGetMemoryActual(privdom->def); info->nrVirtCpu = privdom->def->vcpus; info->cpuTime = ((tv.tv_sec * 1000ll * 1000ll * 1000ll) + (tv.tv_usec * 1000ll)); ret = 0; @@ -2517,7 +2517,7 @@ testDomainGetMaxMemory(virDomainPtr domain) goto cleanup; } - ret = privdom->def->mem.max_balloon; + ret = virDomainDefGetMemoryActual(privdom->def); cleanup: if (privdom) @@ -2543,7 +2543,7 @@ static int testDomainSetMaxMemory(virDomainPtr domain, } /* XXX validate not over host memory wrt to other domains */ - privdom->def->mem.max_balloon = memory; + virDomainDefSetMemoryInitial(privdom->def, memory); ret = 0; cleanup: @@ -2569,7 +2569,7 @@ static int testDomainSetMemory(virDomainPtr domain, goto cleanup; } - if (memory > privdom->def->mem.max_balloon) { + if (memory > virDomainDefGetMemoryActual(privdom->def)) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); goto cleanup; } diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 68efd18..27731f2 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1802,7 +1802,7 @@ umlDomainGetMaxMemory(virDomainPtr dom) if (virDomainGetMaxMemoryEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - ret = vm->def->mem.max_balloon; + ret = virDomainDefGetMemoryActual(vm->def); cleanup: if (vm) @@ -1838,7 +1838,7 @@ static int umlDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) goto cleanup; } - vm->def->mem.max_balloon = newmax; + virDomainDefSetMemoryInitial(vm->def, newmax); ret = 0; cleanup: @@ -1875,7 +1875,7 @@ static int umlDomainSetMemory(virDomainPtr dom, unsigned long newmem) goto cleanup; } - if (newmem > vm->def->mem.max_balloon) { + if (newmem > virDomainDefGetMemoryActual(vm->def)) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("cannot set memory higher than max memory")); goto cleanup; @@ -1922,7 +1922,7 @@ static int umlDomainGetInfo(virDomainPtr dom, } } - info->maxMem = vm->def->mem.max_balloon; + info->maxMem = virDomainDefGetMemoryActual(vm->def); info->memory = vm->def->mem.cur_balloon; info->nrVirtCpu = vm->def->vcpus; ret = 0; diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 55d3624..6697ecc 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3894,7 +3894,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) * reading and while dumping xml */ /* def->mem.max_balloon = maxMemorySize * 1024; */ - def->mem.max_balloon = memorySize * 1024; + virDomainDefSetMemoryInitial(def, memorySize * 1024); gVBoxAPI.UIMachine.GetCPUCount(machine, &CPUCount); def->maxvcpus = def->vcpus = CPUCount; @@ -6055,7 +6055,7 @@ static char *vboxDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, * the notation here seems to be inconsistent while * reading and while dumping xml */ - def->dom->mem.max_balloon = memorySize * 1024; + virDomainDefSetMemoryInitial(def->dom, memorySize * 1024); if (VIR_STRDUP(def->dom->os.type, "hvm") < 0) goto cleanup; def->dom->os.arch = virArchFromHost(); diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index fb7fa5b..36f992b 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -1126,7 +1126,7 @@ vmwareDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) info->state = virDomainObjGetState(vm, NULL); info->cpuTime = 0; - info->maxMem = vm->def->mem.max_balloon; + info->maxMem = virDomainDefGetMemoryActual(vm->def); info->memory = vm->def->mem.cur_balloon; info->nrVirtCpu = vm->def->vcpus; ret = 0; diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index ac2542a..95c081b 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1375,7 +1375,7 @@ virVMXParseConfig(virVMXContext *ctx, goto cleanup; } - def->mem.max_balloon = memsize * 1024; /* Scale from megabytes to kilobytes */ + virDomainDefSetMemoryInitial(def, memsize * 1024); /* Scale from megabytes to kilobytes */ /* vmx:sched.mem.max -> def:mem.cur_balloon */ if (virVMXGetConfigLong(conf, "sched.mem.max", &sched_mem_max, memsize, @@ -1388,8 +1388,8 @@ virVMXParseConfig(virVMXContext *ctx, def->mem.cur_balloon = sched_mem_max * 1024; /* Scale from megabytes to kilobytes */ - if (def->mem.cur_balloon > def->mem.max_balloon) - def->mem.cur_balloon = def->mem.max_balloon; + if (def->mem.cur_balloon > virDomainDefGetMemoryActual(def)) + def->mem.cur_balloon = virDomainDefGetMemoryActual(def); /* vmx:sched.mem.minsize -> def:mem.min_guarantee */ if (virVMXGetConfigLong(conf, "sched.mem.minsize", &sched_mem_minsize, 0, @@ -1402,8 +1402,8 @@ virVMXParseConfig(virVMXContext *ctx, def->mem.min_guarantee = sched_mem_minsize * 1024; /* Scale from megabytes to kilobytes */ - if (def->mem.min_guarantee > def->mem.max_balloon) - def->mem.min_guarantee = def->mem.max_balloon; + if (def->mem.min_guarantee > virDomainDefGetMemoryActual(def)) + def->mem.min_guarantee = virDomainDefGetMemoryActual(def); /* vmx:numvcpus -> def:vcpus */ if (virVMXGetConfigLong(conf, "numvcpus", &numvcpus, 1, true) < 0) @@ -3083,7 +3083,7 @@ virVMXFormatConfig(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virDomainDe /* def:mem.max_balloon -> vmx:memsize */ /* max-memory must be a multiple of 4096 kilobyte */ - max_balloon = VIR_DIV_UP(def->mem.max_balloon, 4096) * 4096; + max_balloon = VIR_DIV_UP(virDomainDefGetMemoryActual(def), 4096) * 4096; virBufferAsprintf(&buffer, "memsize = \"%llu\"\n", max_balloon / 1024); /* Scale from kilobytes to megabytes */ diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 354ab3f..64752df 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -479,7 +479,7 @@ xenXMDomainGetInfo(virConnectPtr conn, goto error; memset(info, 0, sizeof(virDomainInfo)); - info->maxMem = entry->def->mem.max_balloon; + info->maxMem = virDomainDefGetMemoryActual(entry->def); info->memory = entry->def->mem.cur_balloon; info->nrVirtCpu = entry->def->vcpus; info->state = VIR_DOMAIN_SHUTOFF; @@ -557,8 +557,8 @@ xenXMDomainSetMemory(virConnectPtr conn, goto cleanup; entry->def->mem.cur_balloon = memory; - if (entry->def->mem.cur_balloon > entry->def->mem.max_balloon) - entry->def->mem.cur_balloon = entry->def->mem.max_balloon; + if (entry->def->mem.cur_balloon > virDomainDefGetMemoryActual(entry->def)) + entry->def->mem.cur_balloon = virDomainDefGetMemoryActual(entry->def); /* If this fails, should we try to undo our changes to the * in-memory representation of the config file. I say not! @@ -600,10 +600,10 @@ xenXMDomainSetMaxMemory(virConnectPtr conn, if (!(entry = virHashLookup(priv->configCache, filename))) goto cleanup; - entry->def->mem.max_balloon = memory; - if (entry->def->mem.cur_balloon > entry->def->mem.max_balloon) - entry->def->mem.cur_balloon = entry->def->mem.max_balloon; + if (entry->def->mem.cur_balloon > memory) + entry->def->mem.cur_balloon = memory; + virDomainDefSetMemoryInitial(entry->def, memory); /* If this fails, should we try to undo our changes to the * in-memory representation of the config file. I say not! */ @@ -636,7 +636,7 @@ xenXMDomainGetMaxMemory(virConnectPtr conn, if (!(entry = virHashLookup(priv->configCache, filename))) goto cleanup; - ret = entry->def->mem.max_balloon; + ret = virDomainDefGetMemoryActual(entry->def); cleanup: xenUnifiedUnlock(priv); diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index afb6d6c..b3dd852 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1490,7 +1490,7 @@ xenapiDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) VIR_FREE(val); } memory = xenapiDomainGetMaxMemory(dom); - defPtr->mem.max_balloon = memory; + virDomainDefSetMemoryInitial(defPtr, memory); if (xen_vm_get_memory_dynamic_max(session, &dynamic_mem, vm)) { defPtr->mem.cur_balloon = (unsigned long) (dynamic_mem / 1024); } else { diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c index 91bc649..6ec9375 100644 --- a/src/xenapi/xenapi_utils.c +++ b/src/xenapi/xenapi_utils.c @@ -495,8 +495,8 @@ createVMRecordFromXml(virConnectPtr conn, virDomainDefPtr def, if (def->mem.cur_balloon) (*record)->memory_static_max = (int64_t) (def->mem.cur_balloon * 1024); - if (def->mem.max_balloon) - (*record)->memory_dynamic_max = (int64_t) (def->mem.max_balloon * 1024); + if (virDomainDefGetMemoryActual(def)) + (*record)->memory_dynamic_max = (int64_t) (virDomainDefGetMemoryActual(def) * 1024); else (*record)->memory_dynamic_max = (*record)->memory_static_max; diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index a2a1474..2712732 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -305,16 +305,18 @@ xenConfigSetString(virConfPtr conf, const char *setting, const char *str) static int xenParseMem(virConfPtr conf, virDomainDefPtr def) { + unsigned long long memory; + if (xenConfigGetULongLong(conf, "memory", &def->mem.cur_balloon, MIN_XEN_GUEST_SIZE * 2) < 0) return -1; - if (xenConfigGetULongLong(conf, "maxmem", &def->mem.max_balloon, + if (xenConfigGetULongLong(conf, "maxmem", &memory, def->mem.cur_balloon) < 0) return -1; def->mem.cur_balloon *= 1024; - def->mem.max_balloon *= 1024; + virDomainDefSetMemoryInitial(def, memory * 1024); return 0; } @@ -1410,7 +1412,7 @@ static int xenFormatMem(virConfPtr conf, virDomainDefPtr def) { if (xenConfigSetInt(conf, "maxmem", - VIR_DIV_UP(def->mem.max_balloon, 1024)) < 0) + VIR_DIV_UP(virDomainDefGetMemoryActual(def), 1024)) < 0) return -1; if (xenConfigSetInt(conf, "memory", diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index 3e18a7e..5a170d3 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -1155,10 +1155,11 @@ xenParseSxpr(const struct sexpr *root, } } - def->mem.max_balloon = (sexpr_u64(root, "domain/maxmem") << 10); + virDomainDefSetMemoryInitial(def, (sexpr_u64(root, "domain/maxmem") << 10)); def->mem.cur_balloon = (sexpr_u64(root, "domain/memory") << 10); - if (def->mem.cur_balloon > def->mem.max_balloon) - def->mem.cur_balloon = def->mem.max_balloon; + + if (def->mem.cur_balloon > virDomainDefGetMemoryActual(def)) + def->mem.cur_balloon = virDomainDefGetMemoryActual(def); if (cpus != NULL) { if (virBitmapParse(cpus, 0, &def->cpumask, @@ -2213,7 +2214,7 @@ xenFormatSxpr(virConnectPtr conn, virBufferEscapeSexpr(&buf, "(name '%s')", def->name); virBufferAsprintf(&buf, "(memory %llu)(maxmem %llu)", VIR_DIV_UP(def->mem.cur_balloon, 1024), - VIR_DIV_UP(def->mem.max_balloon, 1024)); + VIR_DIV_UP(virDomainDefGetMemoryActual(def), 1024)); virBufferAsprintf(&buf, "(vcpus %u)", def->maxvcpus); /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is either 32, or 64 on a platform where long is big enough. */ -- 2.2.2

On Wed, Feb 25, 2015 at 11:22:02AM +0100, 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. ---
Notes: Version 2: - renamed virDomainDefGetMemoryCurrent to virDomainDefGetMemoryActual - changed to the initial memory accessor when starting VMs in LXC, phyp and libxl
When looking at these accessors' names, I'm thinking how not to screw ourselves by naming decisions when memory hot-plug will come. If we have maxMemory, memory and currentMemory, and then have APIs where setting maximum memory actually changes "memory" and so on, we have to go through the whole documentation and everything that mentions memory just to fix the naming and we still won't be able to change API functions and flags. We will be stuck with this for fair amount of time, I guess. So I'm thinking about an approach that I haven't thought through completely, but I want to have other opinions as well. What if we used our <memory/> that we have now and started treating it as the complete maximum after all slots are filled? So instead of: <maxMemory>2048</maxMemory> <memory>1024</memory> <currentMemory>512</currentMemory> we'd have something like: <memory>2048</memory> <actualMemory>1024</actualMemory> <currentMemory>512</currentMemory> where the main difference is the fact that memory will still be maximum memory, so we don't have to rename half of the enums, funcs and vars in the code. <actualMemory/> would be optional (the name doesn't actually matter at all) and if not specified it would just work like until now. Yet another approach comes to mind and that is that we'll still have only <memory/> and <currentMemory/>, <memory/> gains "dimmSlots" attribute and the number of slots that are filled (and with what memory modules) will be decided upon the existence of memory devices (that is <memory/> under the <devices/> attribute). Default behaviour is still kept until "dimmSlots" (or "slots" or whatever) is not used and then the memory has to be defined by memory devices. I'm just trying to make our (as well as mgmt app developers' and libvirt users') lives easier. Thanks in advance for any feedback, if you think that the current idea is OK and this mail is just about donkey balls, let me know as well. Martin

Adding Dan && Eric to cc On Thu, Feb 26, 2015 at 14:06:55 +0100, Martin Kletzander wrote:
On Wed, Feb 25, 2015 at 11:22:02AM +0100, 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. ---
Notes: Version 2: - renamed virDomainDefGetMemoryCurrent to virDomainDefGetMemoryActual - changed to the initial memory accessor when starting VMs in LXC, phyp and libxl
When looking at these accessors' names, I'm thinking how not to screw ourselves by naming decisions when memory hot-plug will come. If we have maxMemory, memory and currentMemory, and then have APIs where setting maximum memory actually changes "memory" and so on, we have to go through the whole documentation and everything that mentions memory just to fix the naming and we still won't be able to change API functions and flags. We will be stuck with this for fair amount of time, I guess.
So I'm thinking about an approach that I haven't thought through completely, but I want to have other opinions as well.
What if we used our <memory/> that we have now and started treating it as the complete maximum after all slots are filled? So instead of:
<maxMemory>2048</maxMemory> <memory>1024</memory> <currentMemory>512</currentMemory>
we'd have something like:
<memory>2048</memory> <actualMemory>1024</actualMemory> <currentMemory>512</currentMemory>
This depends on how you want to interpret the <memory> element. Currently it is stored in variable called "max_balloon" so in that case my implementation is closer to the existing meaning as the max_balloon memory is actually available in the address space of the guest until the guest decides to enable the memory balloon. As a fact, both of those approaches have to have NUMA enabled though so you can configure the size of the initial memory. If we wanted to change the way we are configuring this we might want to consider whether we ever want to enable memory hotplug for non-NUMA guests and thus have to introduce both <maxMemory> and <actualMemory> or similar.
where the main difference is the fact that memory will still be maximum memory, so we don't have to rename half of the enums, funcs and vars in the code. <actualMemory/> would be optional (the name doesn't actually matter at all) and if not specified it would just work like until now.
Since the field is internally called max_ballon, every change will probably trigger some refactors.
Yet another approach comes to mind and that is that we'll still have only <memory/> and <currentMemory/>, <memory/> gains "dimmSlots" attribute and the number of slots that are filled (and with what memory modules) will be decided upon the existence of memory devices (that is <memory/> under the <devices/> attribute). Default behaviour is still kept until "dimmSlots" (or "slots" or whatever) is not used and then the memory has to be defined by memory devices.
This looks definitely more complex ...
I'm just trying to make our (as well as mgmt app developers' and libvirt users') lives easier>
... since you want to achieve this ^^
Thanks in advance for any feedback, if you think that the current idea is OK and this mail is just about donkey balls, let me know as well.
Martin
Peter

On Wed, Mar 04, 2015 at 03:25:31PM +0100, Peter Krempa wrote:
Adding Dan && Eric to cc
I already did that in my mail, but since the subject didn't start with "Re:" and they have de-duplication turned on, you can't see them here (thank the mailman, I guess).
On Thu, Feb 26, 2015 at 14:06:55 +0100, Martin Kletzander wrote:
On Wed, Feb 25, 2015 at 11:22:02AM +0100, 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. ---
Notes: Version 2: - renamed virDomainDefGetMemoryCurrent to virDomainDefGetMemoryActual - changed to the initial memory accessor when starting VMs in LXC, phyp and libxl
When looking at these accessors' names, I'm thinking how not to screw ourselves by naming decisions when memory hot-plug will come. If we have maxMemory, memory and currentMemory, and then have APIs where setting maximum memory actually changes "memory" and so on, we have to go through the whole documentation and everything that mentions memory just to fix the naming and we still won't be able to change API functions and flags. We will be stuck with this for fair amount of time, I guess.
So I'm thinking about an approach that I haven't thought through completely, but I want to have other opinions as well.
What if we used our <memory/> that we have now and started treating it as the complete maximum after all slots are filled? So instead of:
<maxMemory>2048</maxMemory> <memory>1024</memory> <currentMemory>512</currentMemory>
we'd have something like:
<memory>2048</memory> <actualMemory>1024</actualMemory> <currentMemory>512</currentMemory>
This depends on how you want to interpret the <memory> element. Currently it is stored in variable called "max_balloon" so in that case my implementation is closer to the existing meaning as the max_balloon memory is actually available in the address space of the guest until the guest decides to enable the memory balloon.
Yes, it is closer, from a developers' point of view. I was just trying to make it look nicer and probably a bit more usable from the users' POV.
As a fact, both of those approaches have to have NUMA enabled though so you can configure the size of the initial memory. If we wanted to change the way we are configuring this we might want to consider whether we ever want to enable memory hotplug for non-NUMA guests and thus have to introduce both <maxMemory> and <actualMemory> or similar.
where the main difference is the fact that memory will still be maximum memory, so we don't have to rename half of the enums, funcs and vars in the code. <actualMemory/> would be optional (the name doesn't actually matter at all) and if not specified it would just work like until now.
Since the field is internally called max_ballon, every change will probably trigger some refactors.
Yet another approach comes to mind and that is that we'll still have only <memory/> and <currentMemory/>, <memory/> gains "dimmSlots" attribute and the number of slots that are filled (and with what memory modules) will be decided upon the existence of memory devices (that is <memory/> under the <devices/> attribute). Default behaviour is still kept until "dimmSlots" (or "slots" or whatever) is not used and then the memory has to be defined by memory devices.
This looks definitely more complex ...
I'm just trying to make our (as well as mgmt app developers' and libvirt users') lives easier>
... since you want to achieve this ^^
Yes, I didn't say it's an easier choice ;) But anyway, it looks like nobody expressed their opinions while I was offline for about a week, so let's just go with your approach, then (unless you've changed your mind :) ).
Thanks in advance for any feedback, if you think that the current idea is OK and this mail is just about donkey balls, let me know as well.
Martin
Peter

The messages for currentMemory and memory were swapped. --- Notes: Version 2: - new in series src/phyp/phyp_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index c1d6eb2..a1e390d 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3488,14 +3488,14 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) if (!def->mem.cur_balloon) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("Field <memory> on the domain XML file is missing or has " + _("Field <currentMemory> on the domain XML file is missing or has " "invalid value.")); goto cleanup; } if (!virDomainDefGetMemoryInitial(def)) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("Field <currentMemory> on the domain XML file is missing or " + _("Field <memory> on the domain XML file is missing or " "has invalid value.")); goto cleanup; } -- 2.2.2

On Wed, Feb 25, 2015 at 11:22:03AM +0100, Peter Krempa wrote:
The messages for currentMemory and memory were swapped. ---
Notes: Version 2: - new in series
src/phyp/phyp_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index c1d6eb2..a1e390d 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3488,14 +3488,14 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def)
if (!def->mem.cur_balloon) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("Field <memory> on the domain XML file is missing or has " + _("Field <currentMemory> on the domain XML file is missing or has " "invalid value."));
You could've wrapped the message to fit into 80 columns and remove the dot at the and as well. ACK with that changed.
goto cleanup; }
if (!virDomainDefGetMemoryInitial(def)) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("Field <currentMemory> on the domain XML file is missing or " + _("Field <memory> on the domain XML file is missing or "
ditto
"has invalid value.")); goto cleanup; } -- 2.2.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Feb 26, 2015 at 13:29:15 +0100, Martin Kletzander wrote:
On Wed, Feb 25, 2015 at 11:22:03AM +0100, Peter Krempa wrote:
The messages for currentMemory and memory were swapped. ---
Notes: Version 2: - new in series
src/phyp/phyp_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index c1d6eb2..a1e390d 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3488,14 +3488,14 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def)
if (!def->mem.cur_balloon) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("Field <memory> on the domain XML file is missing or has " + _("Field <currentMemory> on the domain XML file is missing or has " "invalid value."));
You could've wrapped the message to fit into 80 columns and remove the dot at the and as well. ACK with that changed.
goto cleanup; }
if (!virDomainDefGetMemoryInitial(def)) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("Field <currentMemory> on the domain XML file is missing or " + _("Field <memory> on the domain XML file is missing or "
ditto
"has invalid value.")); goto cleanup; }
I've tweaked the messages and pushed the patch. Peter

The memory sizes in qemu are aligned up to 1 MiB boundaries. There are two places where this was done once for the total size and then for individual NUMA cell sizes. Add a function that will align the sizes in one place so that it's clear where the sizes are aligned. --- Notes: Version 2: - Already ACKed but didn't make sense to push. src/qemu/qemu_command.c | 7 +++---- src/qemu/qemu_domain.c | 21 +++++++++++++++++++++ src/qemu/qemu_domain.h | 2 ++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 13b54fe..576c6b1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7177,9 +7177,6 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, /* using of -numa memdev= cannot be combined with -numa mem=, thus we * need to check which approach to use */ for (i = 0; i < ncells; i++) { - unsigned long long cellmem = virDomainNumaGetNodeMemorySize(def->numa, i); - virDomainNumaSetNodeMemorySize(def->numa, i, VIR_ROUND_UP(cellmem, 1024)); - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) || virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { if ((rc = qemuBuildMemoryCellBackendStr(def, qemuCaps, cfg, i, @@ -8313,13 +8310,15 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildDomainLoaderCommandLine(cmd, def, qemuCaps) < 0) goto error; + if (qemuDomainAlignMemorySizes(def) < 0) + goto error; + /* Set '-m MB' based on maxmem, because the lower 'memory' limit * is set post-startup using the balloon driver. If balloon driver * is not supported, then they're out of luck anyway. Update the * XML to reflect our rounding. */ virCommandAddArg(cmd, "-m"); - virDomainDefSetMemoryInitial(def, VIR_ROUND_UP(virDomainDefGetMemoryInitial(def), 1024)); virCommandAddArgFormat(cmd, "%llu", virDomainDefGetMemoryInitial(def) / 1024); if (def->mem.nhugepages && !virDomainNumaGetNodeCount(def->numa)) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ac5ca74..37804c5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2837,3 +2837,24 @@ qemuDomObjEndAPI(virDomainObjPtr *vm) virObjectUnref(*vm); *vm = NULL; } + + +int +qemuDomainAlignMemorySizes(virDomainDefPtr def) +{ + unsigned long long mem; + size_t ncells = virDomainNumaGetNodeCount(def->numa); + size_t i; + + /* align NUMA cell sizes if relevant */ + for (i = 0; i < ncells; i++) { + mem = virDomainNumaGetNodeMemorySize(def->numa, i); + virDomainNumaSetNodeMemorySize(def->numa, i, VIR_ROUND_UP(mem, 1024)); + } + + /* align initial memory size */ + mem = virDomainDefGetMemoryInitial(def); + virDomainDefSetMemoryInitial(def, VIR_ROUND_UP(mem, 1024)); + + return 0; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index b2c3881..faf4ee2 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -414,4 +414,6 @@ int qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo, void qemuDomObjEndAPI(virDomainObjPtr *vm); +int qemuDomainAlignMemorySizes(virDomainDefPtr def); + #endif /* __QEMU_DOMAIN_H__ */ -- 2.2.2

On Wed, Feb 25, 2015 at 11:22:04AM +0100, Peter Krempa wrote:
The memory sizes in qemu are aligned up to 1 MiB boundaries. There are two places where this was done once for the total size and then for individual NUMA cell sizes.
Add a function that will align the sizes in one place so that it's clear where the sizes are aligned. ---
Notes: Version 2: - Already ACKed but didn't make sense to push.
src/qemu/qemu_command.c | 7 +++---- src/qemu/qemu_domain.c | 21 +++++++++++++++++++++ src/qemu/qemu_domain.h | 2 ++ 3 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 13b54fe..576c6b1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7177,9 +7177,6 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg, /* using of -numa memdev= cannot be combined with -numa mem=, thus we * need to check which approach to use */ for (i = 0; i < ncells; i++) { - unsigned long long cellmem = virDomainNumaGetNodeMemorySize(def->numa, i); - virDomainNumaSetNodeMemorySize(def->numa, i, VIR_ROUND_UP(cellmem, 1024)); - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) || virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) { if ((rc = qemuBuildMemoryCellBackendStr(def, qemuCaps, cfg, i, @@ -8313,13 +8310,15 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildDomainLoaderCommandLine(cmd, def, qemuCaps) < 0) goto error;
+ if (qemuDomainAlignMemorySizes(def) < 0) + goto error; + /* Set '-m MB' based on maxmem, because the lower 'memory' limit * is set post-startup using the balloon driver. If balloon driver * is not supported, then they're out of luck anyway. Update the * XML to reflect our rounding. */ virCommandAddArg(cmd, "-m"); - virDomainDefSetMemoryInitial(def, VIR_ROUND_UP(virDomainDefGetMemoryInitial(def), 1024)); virCommandAddArgFormat(cmd, "%llu", virDomainDefGetMemoryInitial(def) / 1024);
if (def->mem.nhugepages && !virDomainNumaGetNodeCount(def->numa)) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ac5ca74..37804c5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2837,3 +2837,24 @@ qemuDomObjEndAPI(virDomainObjPtr *vm) virObjectUnref(*vm); *vm = NULL; } + + +int +qemuDomainAlignMemorySizes(virDomainDefPtr def) +{ + unsigned long long mem; + size_t ncells = virDomainNumaGetNodeCount(def->numa); + size_t i; + + /* align NUMA cell sizes if relevant */ + for (i = 0; i < ncells; i++) { + mem = virDomainNumaGetNodeMemorySize(def->numa, i); + virDomainNumaSetNodeMemorySize(def->numa, i, VIR_ROUND_UP(mem, 1024)); + } + + /* align initial memory size */ + mem = virDomainDefGetMemoryInitial(def); + virDomainDefSetMemoryInitial(def, VIR_ROUND_UP(mem, 1024)); +
ACK without these accessors (so it can go it before we decide on the naming).
+ return 0; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index b2c3881..faf4ee2 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -414,4 +414,6 @@ int qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
void qemuDomObjEndAPI(virDomainObjPtr *vm);
+int qemuDomainAlignMemorySizes(virDomainDefPtr def); + #endif /* __QEMU_DOMAIN_H__ */ -- 2.2.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

The total NUMA memory consists of the sum of individual NUMA node memory amounts. --- Notes: Version 2: - Already ACKed but didn't make sense to push. src/conf/numa_conf.c | 13 +++++++++++++ src/conf/numa_conf.h | 2 ++ src/libvirt_private.syms | 1 + 3 files changed, 16 insertions(+) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index 9caa655..517eaa1 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -894,3 +894,16 @@ virDomainNumaSetNodeMemorySize(virDomainNumaPtr numa, { numa->mem_nodes[node].mem = size; } + + +unsigned long long +virDomainNumaGetMemorySize(virDomainNumaPtr numa) +{ + size_t i; + unsigned long long ret = 0; + + for (i = 0; i < numa->nmem_nodes; i++) + ret += numa->mem_nodes[i].mem; + + return ret; +} diff --git a/src/conf/numa_conf.h b/src/conf/numa_conf.h index dc2ce58..ded6e01 100644 --- a/src/conf/numa_conf.h +++ b/src/conf/numa_conf.h @@ -95,6 +95,8 @@ virNumaMemAccess virDomainNumaGetNodeMemoryAccessMode(virDomainNumaPtr numa, unsigned long long virDomainNumaGetNodeMemorySize(virDomainNumaPtr numa, size_t node) ATTRIBUTE_NONNULL(1); +unsigned long long virDomainNumaGetMemorySize(virDomainNumaPtr numa) + ATTRIBUTE_NONNULL(1); /* * Formatters diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d4d0faf..9a32758 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -633,6 +633,7 @@ virNodeDeviceObjUnlock; virDomainNumaCheckABIStability; virDomainNumaEquals; virDomainNumaFree; +virDomainNumaGetMemorySize; virDomainNumaGetNodeCount; virDomainNumaGetNodeCpumask; virDomainNumaGetNodeMemoryAccessMode; -- 2.2.2

On Wed, Feb 25, 2015 at 11:22:05AM +0100, Peter Krempa wrote:
The total NUMA memory consists of the sum of individual NUMA node memory amounts. ---
Notes: Version 2: - Already ACKed but didn't make sense to push.
And ACK again...

On Thu, Feb 26, 2015 at 14:49:33 +0100, Martin Kletzander wrote:
On Wed, Feb 25, 2015 at 11:22:05AM +0100, Peter Krempa wrote:
The total NUMA memory consists of the sum of individual NUMA node memory amounts. ---
Notes: Version 2: - Already ACKed but didn't make sense to push.
And ACK again...
Pushed; Thanks. Peter

Use the NUMA total instead of the configured size both in XML and for uses in the code once NUMA is enabled for a domain. One test case change is necessary as the rounding of the individual cell sizes was not matching the rounding of the total size. --- Notes: Version 2: - Already ACKed but didn't make sense to push. src/conf/domain_conf.c | 6 ++++++ tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b41ae17..b2a9297 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6904,6 +6904,12 @@ virDomainParseMemory(const char *xpath, unsigned long long virDomainDefGetMemoryInitial(virDomainDefPtr def) { + unsigned long long ret; + + /* return NUMA memory size total in case numa is enabled */ + if ((ret = virDomainNumaGetMemorySize(def->numa)) > 0) + return ret; + return def->mem.max_balloon; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args index 513d657..5dd7fcd 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ -/usr/bin/kvm -S -M pc -m 24104 -smp 32 \ +/usr/bin/kvm -S -M pc -m 24105 -smp 32 \ -object memory-backend-ram,id=ram-node0,size=20971520,host-nodes=3,\ policy=preferred \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ -- 2.2.2

On Wed, Feb 25, 2015 at 11:22:06AM +0100, Peter Krempa wrote:
Use the NUMA total instead of the configured size both in XML and for uses in the code once NUMA is enabled for a domain.
One test case change is necessary as the rounding of the individual cell sizes was not matching the rounding of the total size. ---
Notes: Version 2: - Already ACKed but didn't make sense to push.
src/conf/domain_conf.c | 6 ++++++ tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b41ae17..b2a9297 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6904,6 +6904,12 @@ virDomainParseMemory(const char *xpath, unsigned long long virDomainDefGetMemoryInitial(virDomainDefPtr def) { + unsigned long long ret; + + /* return NUMA memory size total in case numa is enabled */ + if ((ret = virDomainNumaGetMemorySize(def->numa)) > 0) + return ret; + return def->mem.max_balloon; }
If this accessor is used everywhere, then we can make the memory element optional and count the memory from numa specification, right?
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args index 513d657..5dd7fcd 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ -/usr/bin/kvm -S -M pc -m 24104 -smp 32 \ +/usr/bin/kvm -S -M pc -m 24105 -smp 32 \ -object memory-backend-ram,id=ram-node0,size=20971520,host-nodes=3,\ policy=preferred \ -numa node,nodeid=0,cpus=0,memdev=ram-node0 \ -- 2.2.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Martin Kletzander
-
Peter Krempa