1:09 p.m.
On 07/30/2014 11:41 PM, Chen Hanxiao wrote:
Currently, setmaxmem return success on an active
domain, but nothing happened.
Not quite true... Prior to these changes...
# virsh -c lxc:/// start vm1
# virsh -c lxc:/// dominfo vm1 | grep memory
Max memory: 500000 KiB
Used memory: 776 KiB
# virsh -c lxc:/// dumpxml vm1 | grep mem
<memory unit='KiB'>500000</memory>
# cat /sys/fs/cgroup/memory/machine.slice/*/memory.limit_in_bytes
512000000
# virsh -c lxc:/// setmaxmem vm1 525000
# virsh -c lxc:/// dominfo vm1 | grep memory
Max memory: 525000 KiB
Used memory: 776 KiB
# virsh -c lxc:/// dumpxml vm1 | grep mem
<memory unit='KiB'>525000</memory>
# cat /sys/fs/cgroup/memory/machine.slice/*/memory.limit_in_bytes
512000000
# virsh -c lxc:/// console vm1
Connected to domain vm1
Escape character is ^]
sh-4.2# grep Mem /proc/meminfo
MemTotal: 500000 kB
MemFree: 499076 kB
MemAvailable: 3629632 kB
#
Now I'll agree that technically what one expected to happen didn't since
it's not supported, but something did happen :-)... And of course after
a destroy the maxmem went back to 500000.
This patch will disable this behaviour,
also add support persistent config.
And it will be used in a later patch.
FWIW:
I agree with Peter's observation from your v1 - the two patches should
be one as it is clearer that way what you are doing and leaves no doubt
why the flag is used
I do see that other drivers like qemu & libxl use the MEM_MAXIMUM flag.
So perhaps said differently -
"This patch changes the LXC driver setmaxmem function to support the
'--live', '--config', and '--current' flags by revectoring the
code
through the setmem function using the VIR_DOMAIN_MEM_MAXIMUM flag. The
setmem code is refactored to handle both cases depending on the flag.
The changed maxmem code for the MEM_MAXIMUM path will not allow
modification to the memory values of an active guest unless the --config
switch is used."
Signed-off-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
---
v2: disable changing max memory on an active domain
drop useless as_assert
src/lxc/lxc_driver.c | 64 +++++++++++++++++++++++++++++++++-------------------
1 file changed, 41 insertions(+), 23 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 9e12ecc..d99ab3b 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -721,10 +721,10 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
virLXCDomainObjPrivatePtr priv;
virLXCDriverPtr driver = dom->conn->privateData;
virLXCDriverConfigPtr cfg = NULL;
- unsigned long oldmax = 0;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
- VIR_DOMAIN_AFFECT_CONFIG, -1);
+ VIR_DOMAIN_AFFECT_CONFIG |
+ VIR_DOMAIN_MEM_MAXIMUM, -1);
if (!(vm = lxcDomObjFromDomain(dom)))
goto cleanup;
@@ -743,32 +743,50 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
&persistentDef) < 0)
goto cleanup;
I see the next set of changes is essentially cut-n-pasted from qemu,
although the changes after the else are the same as the former setmem
function - it's just the git diff that obfuscates that.
The changes do work as I'd expect, so ACK from that point of view.
I can merge the two patches for you and commit, but I'll give it a bit
to make sure no one else has concerns
John
- 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_MEM_MAXIMUM) {
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("Cannot resize the max memory "
+ "on an active domain"));
+ goto cleanup;
+ }
- if (newmem > oldmax) {
- virReportError(VIR_ERR_INVALID_ARG,
- "%s", _("Cannot set memory higher than max
memory"));
- goto cleanup;
- }
+ if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ persistentDef->mem.max_balloon = newmem;
+ if (persistentDef->mem.cur_balloon > newmem)
+ persistentDef->mem.cur_balloon = newmem;
+ if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0)
+ goto cleanup;
+ }
+ } else {
+ unsigned long oldmax = 0;
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
- if (virCgroupSetMemory(priv->cgroup, newmem) < 0) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- "%s", _("Failed to set memory for
domain"));
- goto cleanup;
+ 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_CONFIG) {
- sa_assert(persistentDef);
- persistentDef->mem.cur_balloon = newmem;
- if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0)
+ if (newmem > oldmax) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ "%s", _("Cannot set memory higher than max
memory"));
goto cleanup;
+ }
+
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (virCgroupSetMemory(priv->cgroup, newmem) < 0) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ "%s", _("Failed to set memory for
domain"));
+ goto cleanup;
+ }
+ }
+
+ if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ persistentDef->mem.cur_balloon = newmem;
+ if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0)
+ goto cleanup;
+ }
}
ret = 0;