Re: [libvirt] virsh setmaxmem
On Wed, Mar 10, 2010 at 03:33:55PM -0500, Chris Lalancette wrote:
All,
My recent patch to remove qemudDomainSetMaxMem() revealed some surprising
(to me) behavior of "virsh setmaxmem". In particular, if you run setmaxmem,
and
the hypervisor doesn't support it, this fact will be reported to you. However,
if the amount you were setting for maxmem happens to be lower than
"currentMemory",
setmaxmem will silently balloon down the domain for you. This is a surprising
result exactly because virsh reports error, but did something anyway. What I
would expect is that if a hypervisor doesn't support virDomainSetMaxMem(), nothing
at all is done.
If we agree that this is behavior that needs to be fixed, I would suggest
that we move the code to do the auto-ballooning into each of the individual
drivers that *do* support virDomainSetMaxMem(). Currently that includes the
test driver, the LXC driver, and the Xen driver. This way, we maintain the
current behavior for the hypervisors that support this call, and make sure
not to do anything at all for the hypervisors that do not.
Opinions?
Hum ... looking at cmdSetmaxmem() in tools/virsh.c it does
-------------------------------------------------
if (kilobytes < info.memory) {
if (virDomainSetMemory(dom, kilobytes) != 0) {
virDomainFree(dom);
vshError(ctl, "%s", _("Unable to shrink current
MemorySize"));
return FALSE;
}
}
if (virDomainSetMaxMemory(dom, kilobytes) != 0) {
vshError(ctl, "%s", _("Unable to change MaxMemorySize"));
ret = FALSE;
}
-------------------------------------------------
maybe the two need to be reversed, i.e. after checking that
virDomainSetMaxMemory actually is supported then we try to shrink the
domain memory. It's a bit of a strange thing because it's somehow a
chicken and egg problem, but you can't tell from virsh if
virDomainSetMaxMemory() call is supported a priori.
But I'm not too fond of changing the behaviour of existing APIs
due to this. It's not that virDomainSetMaxMem() itself did the
shrinking, it's virsh, and IMHO only virsh need to be fixed.
More opinions ? :-)
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/