> -----Original Message----- > From: Michal Privoznik [mailto:mprivozn@xxxxxxxxxx] > Sent: Thursday, July 24, 2014 5:00 PM > To: Chen, Hanxiao/陈 晗霄; libvir-list@xxxxxxxxxx > Subject: Re: [PATCH 1/2] LXC: add support for --config in setmaxmem > command > > On 16.07.2014 11:51, Chen Hanxiao wrote: > > In lxc, we could not use setmaxmem command > > with --config options. > > This patch will add support for this. > > > > Signed-off-by: Chen Hanxiao <chenhanxiao@xxxxxxxxxxxxxx> > > --- > > src/lxc/lxc_driver.c | 69 > ++++++++++++++++++++++++++++++++++------------------ > > 1 file changed, 46 insertions(+), 23 deletions(-) > > > > diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c > > index b7b4b02..be6ee19 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,55 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, > unsigned long newmem, > > &persistentDef) < 0) > > 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_MEM_MAXIMUM) { > > + if (flags & VIR_DOMAIN_AFFECT_LIVE) { > > + if (newmem < vm->def->mem.cur_balloon) { > > + virReportError(VIR_ERR_OPERATION_INVALID, "%s", > > + _("Cannot resize the max memory less than current" > > + " memory for an active domain")); > > + goto cleanup; > > + } > > + vm->def->mem.max_balloon = newmem; > > Are things that easy? Don't we need to communicate this with the > lxc_controler somehow? Even though you allow only extending, I think > unless we are 100% sure guest will see the resize, we shouldn't allow this. > I focused on '--config', so I kept what the original lxcDomainSetMaxMemory did. It looks that guest could not see the resize, and we shouldn't allow this. > > + } > > > > - if (newmem > oldmax) { > > - virReportError(VIR_ERR_INVALID_ARG, > > - "%s", _("Cannot set memory higher than max memory")); > > - goto cleanup; > > - } > > + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { > > + sa_assert(persistentDef); > > Is this assert needed? Did clang complain or is this just a pure lefover > from copying from qemu_driver.c? > > > + 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); > > Well, since it has been here already, I think we can leave it in your > patch too. > > > - 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) { > > + sa_assert(persistentDef); > > + persistentDef->mem.cur_balloon = newmem; > > + if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0) > > + goto cleanup; > > + } > > } > > > > ret = 0; > > > > But what I miss in this patch is, what would: > > virDomainSetMaxMemoryFlags(dom, newmem, VIR_DOMAIN_AFFECT_CURRENT | > VIR_DOMAIN_MEM_MAXIMUM) > > do? I mean, the _CURRENT is not translated to _LIVE or _CONFIG anywhere > in the function. I put that section in 2/2 patch. I'll add a description in v2. Thanks, - Chen -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list