[libvirt] [PATCH] LXC: introduce lxcDomainSetMemoryFlags

[chenhanxiao at cn.fujitsu.com]

> -----Original Message-----
> From: Ján Tomko [mailto:jtomko at redhat.com]
> Sent: Monday, July 07, 2014 6:54 PM
> To: Chen, Hanxiao/陈 晗霄; libvir-list at redhat.com
> Subject: Re: [libvirt] [PATCH] LXC: introduce lxcDomainSetMemoryFlags
> 
> On 07/04/2014 10:21 AM, Chen Hanxiao wrote:
> > In lxc, we could not use setmem command
> > with --config options.
> > This patch will add support for this.
> >
> > Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> > ---
> >  src/lxc/lxc_driver.c | 52
> ++++++++++++++++++++++++++++++++++++++++++----------
> >  1 file changed, 42 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> > index 79c3b4a..68795cb 100644
> > --- a/src/lxc/lxc_driver.c
> > +++ b/src/lxc/lxc_driver.c
> > @@ -706,11 +706,19 @@ static int lxcDomainSetMaxMemory(virDomainPtr dom,
> unsigned long newmax)
> >      return ret;
> >  }
> >
> > -static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem)
> > +static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
> > +                                    unsigned int flags)
> 
> Indentation looks off.
> 
> >  {
> >      virDomainObjPtr vm;
> > +    virDomainDefPtr persistentDef = NULL;
> > +    virCapsPtr caps = NULL;
> >      int ret = -1;
> >      virLXCDomainObjPrivatePtr priv;
> > +    virLXCDriverPtr driver = dom->conn->privateData;
> > +    virLXCDriverConfigPtr cfg = NULL;
> > +
> > +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> > +                  VIR_DOMAIN_AFFECT_CONFIG, -1);
> >
> >      if (!(vm = lxcDomObjFromDomain(dom)))
> >          goto cleanup;
> > @@ -720,22 +728,38 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned
> long newmem)
> >      if (virDomainSetMemoryEnsureACL(dom->conn, vm->def) < 0)
> >          goto cleanup;
> 
> 'make check' complains about a mismatched ACL check:
> ./lxc/lxc_driver.c:728 Mismatch check 'virDomainSetMemoryEnsureACL' for
> function 'lxcDomainSetMemoryFlags'
> 
> 
> >
> > +    if (!(caps = virLXCDriverGetCapabilities(driver, false)))
> > +        goto cleanup;
> > +
> > +    if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags,
> > +                                        &persistentDef) < 0)
> > +        goto cleanup;
> > +
> 
> >      if (newmem > vm->def->mem.max_balloon) {
> >          virReportError(VIR_ERR_INVALID_ARG,
> >                         "%s", _("Cannot set memory higher than max memory"));
> >          goto cleanup;
> >      }
> 
> This check is only valid for AFFECT_LIVE. For AFFECT_CONFIG, a separate check
> is needed.

It should be virDomainSetMemoryFlagsEnsureACL.

> 
> >
> > -    if (!virDomainObjIsActive(vm)) {
> > -        virReportError(VIR_ERR_OPERATION_INVALID,
> > -                       "%s", _("Domain is not running"));
> > -        goto cleanup;
> > -    }
> > +     if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> > +         cfg = virLXCDriverGetConfig(driver);
> > +         persistentDef->mem.cur_balloon = newmem;
> > +         if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0)
> > +             goto cleanup;
> > +     }
> >
> > -    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) {
> 
> > +        if (!virDomainObjIsActive(vm)) {
> > +            virReportError(VIR_ERR_OPERATION_INVALID,
> > +                           "%s", _("Domain is not running"));
> > +            goto cleanup;
> > +        }
> 
> virDomainLiveConfigHelperMethod already checked if the domain is running when
> AFFECT_LIVE is specified.
Yes, we should remove this block.

Thanks for your review, v2 will come soon.

- Chen

> 
> > +
> > +        if (virCgroupSetMemory(priv->cgroup, newmem) < 0) {
> > +            virReportError(VIR_ERR_OPERATION_FAILED,
> > +                           "%s", _("Failed to set memory for domain"));
> > +            goto cleanup;
> > +        }
> >      }
> >
> >      ret = 0;
> 
> 
> Jan
>