[libvirt] [PATCH 2/3] lxc: Make SetMemory work for active domains only

[Jiri Denemark]

> > @@ -642,27 +642,30 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem) {
> >          goto cleanup;
> >      }
> >  
> > -    if (virDomainObjIsActive(vm)) {
> > -        if (driver->cgroup == NULL) {
> > -            lxcError(VIR_ERR_NO_SUPPORT,
> > -                     "%s", _("cgroups must be configured on the host"));
> > -            goto cleanup;
> > -        }
> > +    if (!virDomainObjIsActive(vm)) {
> > +        lxcError(VIR_ERR_OPERATION_INVALID,
> > +                 "%s", _("Domain is not running"));
> > +        goto cleanup;
> > +    }
> >  
> > -        if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) {
> > -            lxcError(VIR_ERR_INTERNAL_ERROR,
> > -                     _("Unable to get cgroup for %s"), vm->def->name);
> > -            goto cleanup;
> > -        }
> > +    if (driver->cgroup == NULL) {
> > +        lxcError(VIR_ERR_NO_SUPPORT,
> > +                 "%s", _("cgroups must be configured on the host"));
> > +        goto cleanup;
> > +    }
> >  
> > -        if (virCgroupSetMemory(cgroup, newmem) < 0) {
> > -            lxcError(VIR_ERR_OPERATION_FAILED,
> > -                     "%s", _("Failed to set memory for domain"));
> > -            goto cleanup;
> > -        }
> > -    } else {
> > -        vm->def->memory = newmem;
> > +    if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) {
> > +        lxcError(VIR_ERR_INTERNAL_ERROR,
> > +                 _("Unable to get cgroup for %s"), vm->def->name);
> > +        goto cleanup;
> > +    }
> > +
> > +    if (virCgroupSetMemory(cgroup, newmem) < 0) {
> > +        lxcError(VIR_ERR_OPERATION_FAILED,
> > +                 "%s", _("Failed to set memory for domain"));
> > +        goto cleanup;
> >      }
> > +
> >      ret = 0;
> >  
> >  cleanup:
> 
> I'm not 100% sure of the patch but the new sequence look more logical,
> I'm still concerned that the new code seems to not update vm->def->memory

Hmm, the patch generated by git is a bit confusing. In reality it's quite
simple... Before the patch, there was a whole bunch of code within "if
(virDomainObjIsActive(vm))" and "vm->def->memory = newmem;" if the VM wasn't
active. Now, the function works only for active VMs (which is it's correct
behavior as it was never supposed to work offline, one has to change domain
XML to make offline changes), that is "vm->def->memory = newmem;" is replaced
with VIR_ERR_OPERATION_INVALID error. There is no change in semantics for
active VMs.

Jirka