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

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

[Martin Kletzander]

[-- Attachment #1.1: Type: text/plain, Size: 2354 bytes --]

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@suse.com>
>---
>
>V3:
>Remove unneeded local variable 'ret' in libxlDomainFreeMem.
>

ACK

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Jim Fehlig]

Martin Kletzander wrote:
> 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@suse.com>
>> ---
>>
>> V3:
>> Remove unneeded local variable 'ret' in libxlDomainFreeMem.
>>
>
> ACK

Thanks!  I've pushed this now.

Regards,
Jim