Re: [libvirt] [PATCH V3] libxl: fix dom0 balloon logic

On Wed, Apr 01, 2015 at 11:08:54AM -0600, Jim Fehlig wrote: > Recent testing on large memory systems revealed a bug in the Xen xl > tool's freemem() function. When autoballooning is enabled, freemem() > is used to ensure enough memory is available to start a domain, > ballooning dom0 if necessary. When ballooning large amounts of memory > from dom0, freemem() would exceed its self-imposed wait time and > return an error. Meanwhile, dom0 continued to balloon. Starting the > domain later, after sufficient memory was ballooned from dom0, would > succeed. The libvirt implementation in libxlDomainFreeMem() suffers > the same bug since it is modeled after freemem(). > > In the end, the best place to fix the bug on the Xen side was to > slightly change the behavior of libxl_wait_for_memory_target(). > Instead of failing after caller-provided wait_sec, the function now > blocks as long as dom0 memory ballooning is progressing. It will return > failure only when more memory is needed to reach the target and wait_sec > have expired with no progress being made. See xen.git commit fd3aa246. > There was a dicussion on how this would affect other libxl apps like > libvirt > > http://lists.xen.org/archives/html/xen-devel/2015-03/msg00739.html > > If libvirt containing this patch was build against a Xen containing > the old libxl_wait_for_memory_target() behavior, libxlDomainFreeMem() > will fail after 30 sec and domain creation will be terminated. > Without this patch and with old libxl_wait_for_memory_target() behavior, > libxlDomainFreeMem() does not succeed after 30 sec, but returns success > anyway. Domain creation continues resulting in all sorts of fun stuff > like cpu soft lockups in the guest OS. It was decided to properly fix > libxl_wait_for_memory_target(), and if anything improve the default > behavior of apps using the freemem reference impl in xl. > > xl was patched to accommodate the change in > libxl_wait_for_memory_target() > with xen.git commit 883b30a0. This patch does the same in the libxl > driver. While at it, I changed the logic to essentially match > freemem() in $xensrc/tools/libxl/xl_cmdimpl.c. It was a bit cleaner > IMO and will make it easier to spot future, potentially interesting > divergences. > > Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; > --- > > V3: > Remove unneeded local variable 'ret' in libxlDomainFreeMem. > ACK