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

> > @@ -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.