On Tue, May 04, 2010 at 11:56:56AM +0200, Jiri Denemark wrote:
> > > @@ -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.
Ah, okay, I didn't realized the vm->def->memory = newmem was only for
non-running domains,
ACK,
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list