Re: [libvirt] [PATCH] qemu: fix can use setmaxmem to change the maximum memory biger than max memory

On Tue, Mar 24, 2015 at 22:12:37 +0800, Luyao Huang wrote: > When call qemuDomainSetMemoryFlags() to change maximum memory size, we > do not check if the maximum memory is biger than the max memory, this will > make vm disappear after libvirtd restart if we set a big maximum memory. > > Add a check for this, also fix a typos issue. > > Signed-off-by: Luyao Huang <lhuang(a)redhat.com&gt; > --- > src/conf/domain_conf.c | 2 +- > src/qemu/qemu_driver.c | 7 +++++++ > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index d633f93..0d4b165 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -16646,7 +16646,7 @@ virDomainDefCheckABIStability(virDomainDefPtr src, > > if (src->mem.memory_slots != dst->mem.memory_slots) { > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > - _("Target domain memory slots count '%u' doesn't match source '%u"), > + _("Target domain memory slots count '%u' doesn't match source '%u'"), > dst->mem.memory_slots, src->mem.memory_slots); > goto error; > } The hunk above is not at all related to the code change below and should be separate. I'll split the patch for you.

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 3518dec..86b87af 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -2324,6 +2324,13 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, > "nodes cannot be modified with this API")); > goto endjob; > } > + if (persistentDef->mem.max_memory && > + persistentDef->mem.max_memory < newmem) { > + virReportError(VIR_ERR_OPERATION_INVALID, "%s", > + _("cannot set maximum memory size biger than " higher than or greather than. Bigger can't be used in this context. > + "the max memory size")); > + goto endjob; > + } Technically this code requires that the VM does not have numa configured but has the memory hotplug code enabled. While you can set such configuration and use the qemuDomainSetMemoryFlags() API on it you won't be able to actually use that as QEMU requires NUMA to be enabled.

Strictly speaking, this patch makes sense as it allows to induce a invalid configuration that would vapourize the guest. ACK. I'll split the patch and tweak the error message before pushing.

Peter