[Libvir] [PATCH] Add the message when a little memory is set with setmaxmem.
Hi The message which a cause of an error is hard to detect is displayed when "virsh setmaxmem" sets the maximum memory of an active domain less than Used Memory. -------------------------------------------------------------------------------- # virsh setmaxmem 0 4096 libvir: Xen error : failed Xen syscall ioctl 8518692 libvir: Xen Daemon error : POST operation failed: (xend.err "(22, 'Invalid argument')") libvir: error : library call virDomainSetMaxMemory failed, possibly not supported -------------------------------------------------------------------------------- This patch displays the message which is easier to detect the cause of an error to a user. -------------------------------------------------------------------------------- # virsh setmaxmem 0 4096 error: 4096 is less than current used memory. -------------------------------------------------------------------------------- Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com> Thanks, Masayuki Sunou. -------------------------------------------------------------------------------- Index: libvirt/src/virsh.c =================================================================== RCS file: /data/cvs/libvirt/src/virsh.c,v retrieving revision 1.67 diff -u -p -r1.67 virsh.c --- libvirt/src/virsh.c 20 Mar 2007 15:31:46 -0000 1.67 +++ libvirt/src/virsh.c 22 Mar 2007 02:27:16 -0000 @@ -1493,6 +1493,7 @@ static int cmdSetmaxmem(vshControl * ctl, vshCmd * cmd) { virDomainPtr dom; + virDomainInfo info; int bytes; int ret = TRUE; @@ -1509,6 +1510,18 @@ cmdSetmaxmem(vshControl * ctl, vshCmd * return FALSE; } + if (virDomainGetID(dom) != ((unsigned int)-1)) { + if (virDomainGetInfo(dom, &info) != 0) { + virDomainFree(dom); + return FALSE; + } + if (bytes < info.memory) { + vshError(ctl, FALSE, _("%d is less than current used memory."), bytes); + virDomainFree(dom); + return FALSE; + } + } + if (virDomainSetMaxMemory(dom, bytes) != 0) { ret = FALSE; } --------------------------------------------------------------------------------
On Thu, Mar 22, 2007 at 01:56:25PM +0900, Masayuki Sunou wrote:
Hi
The message which a cause of an error is hard to detect is displayed when "virsh setmaxmem" sets the maximum memory of an active domain less than Used Memory.
-------------------------------------------------------------------------------- # virsh setmaxmem 0 4096 libvir: Xen error : failed Xen syscall ioctl 8518692 libvir: Xen Daemon error : POST operation failed: (xend.err "(22, 'Invalid argument')") libvir: error : library call virDomainSetMaxMemory failed, possibly not supported --------------------------------------------------------------------------------
This patch displays the message which is easier to detect the cause of an error to a user.
-------------------------------------------------------------------------------- # virsh setmaxmem 0 4096 error: 4096 is less than current used memory. --------------------------------------------------------------------------------
Hum, this sounds useful from a user interraction perspective, but I have a doubt about it, first it's a bit racy, i.e. the memory usage could change between the GetInfo call and the SetMaxMem one, but my main problem is that I'm afraid this would prevent the possibility of automatic memory resize in some case. Assume a paravirtual guest kernel, supporting memory resizing, then it could in theory shrink its memory footprint if asked for (migrating dirty pages to swap dynamically for example). Such a mechanism sounds important to load balance usage between multiple guests sharing a host, and I'm afraid your patch would block that capacity just because Xen does not accept that operation in that version. I'm afraid that patch - even if limited to virsh - is unfortunately not a good idea, as usual the preferred way would be to get xend fixed to return a better error message, so I prefer to decline applying that patch as is. I'm also wondering if virDomainSetMemory() should not be called first to try to shrink and then only make the check of the current memory usage, the problem is that memory shrinking is likely to be a long operation in that case since it certainly gonna involve moving blocks to disk. I Cc'ed Rik who may have a more complete opinion on guest memory shrinking. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Hi Daniel I understood your suggestion. Therefore, I decline applying this patch. Thanks, Masayuki Sunou. In message <20070322101643.GA21706@redhat.com> "Re: [Libvir] [PATCH] Add the message when a little memory is set with setmaxmem." "Daniel Veillard <veillard@redhat.com>" wrote:
On Thu, Mar 22, 2007 at 01:56:25PM +0900, Masayuki Sunou wrote:
Hi
The message which a cause of an error is hard to detect is displayed when "virsh setmaxmem" sets the maximum memory of an active domain less than Used Memory.
-------------------------------------------------------------------------------- # virsh setmaxmem 0 4096 libvir: Xen error : failed Xen syscall ioctl 8518692 libvir: Xen Daemon error : POST operation failed: (xend.err "(22, 'Invalid argument')") libvir: error : library call virDomainSetMaxMemory failed, possibly not supported --------------------------------------------------------------------------------
This patch displays the message which is easier to detect the cause of an error to a user.
-------------------------------------------------------------------------------- # virsh setmaxmem 0 4096 error: 4096 is less than current used memory. --------------------------------------------------------------------------------
Hum, this sounds useful from a user interraction perspective, but I have a doubt about it, first it's a bit racy, i.e. the memory usage could change between the GetInfo call and the SetMaxMem one, but my main problem is that I'm afraid this would prevent the possibility of automatic memory resize in some case. Assume a paravirtual guest kernel, supporting memory resizing, then it could in theory shrink its memory footprint if asked for (migrating dirty pages to swap dynamically for example). Such a mechanism sounds important to load balance usage between multiple guests sharing a host, and I'm afraid your patch would block that capacity just because Xen does not accept that operation in that version. I'm afraid that patch - even if limited to virsh - is unfortunately not a good idea, as usual the preferred way would be to get xend fixed to return a better error message, so I prefer to decline applying that patch as is. I'm also wondering if virDomainSetMemory() should not be called first to try to shrink and then only make the check of the current memory usage, the problem is that memory shrinking is likely to be a long operation in that case since it certainly gonna involve moving blocks to disk. I Cc'ed Rik who may have a more complete opinion on guest memory shrinking.
Daniel
-- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Mon, Mar 26, 2007 at 11:34:10AM +0900, Masayuki Sunou wrote:
Hi Daniel
Hi Masayuki,
I understood your suggestion. Therefore, I decline applying this patch. [...]
I'm afraid that patch - even if limited to virsh - is unfortunately not a good idea, as usual the preferred way would be to get xend fixed to return a better error message, so I prefer to decline applying that patch as is. I'm also wondering if virDomainSetMemory() should not be called first to try to shrink and then only make the check of the current memory usage, the problem is that memory shrinking is likely to be a long operation in that case since it certainly gonna involve moving blocks to disk.
What do you think of the following sequence (in pseudo code): set_max(max) { cur = virDomainGetMemory() if (cur > max) { ret = virDomainSetMemory(max); if (ret == 0) { virDomainSetMaxMemory(max) } } } It gives the system one more chance to actually do the shrinking, but I'm afraid it won't solve the initial problem of your patch which is to avoid the Xen error message, it's unlikely virDomainSetMemory() error would be any better than virDomainSetMaxMemory() one, but that could be tried, if you think it's worth trying to chase this issue. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
participants (2)
-
Daniel Veillard -
Masayuki Sunou