[libvirt] [Xen-devel] [PATCH] libxl: fix dom0 balloon logic
Stefano Stabellini
stefano.stabellini at eu.citrix.com
Tue Mar 24 17:50:19 UTC 2015
On Mon, 23 Mar 2015, Ian Campbell wrote:
> (just ccing the other tools maintainers, in particular Stefano who knows
> what this stuff is supposed to do...)
>
> On Fri, 2015-03-20 at 17:10 -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 at suse.com>
> > ---
> > src/libxl/libxl_domain.c | 57 ++++++++++++++++++++++++------------------------
> > 1 file changed, 28 insertions(+), 29 deletions(-)
> >
> > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> > index 407a9bd..ff78133 100644
> > --- a/src/libxl/libxl_domain.c
> > +++ b/src/libxl/libxl_domain.c
> > @@ -1121,38 +1121,41 @@ libxlDomainFreeMem(libxlDomainObjPrivatePtr priv, libxl_domain_config *d_config)
> > {
> > uint32_t needed_mem;
> > uint32_t free_mem;
> > - size_t i;
> > - int ret = -1;
> > + int ret;
> > int tries = 3;
> > int wait_secs = 10;
> >
> > - if ((ret = libxl_domain_need_memory(priv->ctx, &d_config->b_info,
> > - &needed_mem)) >= 0) {
> > - for (i = 0; i < tries; ++i) {
> > - if ((ret = libxl_get_free_memory(priv->ctx, &free_mem)) < 0)
> > - break;
> > + ret = libxl_domain_need_memory(priv->ctx, &d_config->b_info,
> > + &needed_mem);
> > + if (ret < 0)
> > + goto error;
> >
> > - if (free_mem >= needed_mem) {
> > - ret = 0;
> > - break;
> > - }
> > + do {
> > + ret = libxl_get_free_memory(priv->ctx, &free_mem);
> > + if (ret < 0)
> > + goto error;
> >
> > - if ((ret = libxl_set_memory_target(priv->ctx, 0,
> > - free_mem - needed_mem,
> > - /* relative */ 1, 0)) < 0)
> > - break;
> > + if (free_mem >= needed_mem)
> > + return 0;
> >
> > - ret = libxl_wait_for_free_memory(priv->ctx, 0, needed_mem,
> > - wait_secs);
> > - if (ret == 0 || ret != ERROR_NOMEM)
> > - break;
> > + ret = libxl_set_memory_target(priv->ctx, 0,
> > + free_mem - needed_mem,
> > + /* relative */ 1, 0);
> > + if (ret < 0)
> > + goto error;
> >
> > - if ((ret = libxl_wait_for_memory_target(priv->ctx, 0, 1)) < 0)
> > - break;
> > - }
> > - }
> > + ret = libxl_wait_for_free_memory(priv->ctx, 0, needed_mem,
> > + wait_secs);
> > + if (ret < 0)
> > + goto error;
Shouldn't this be a call to libxl_wait_for_memory_target instead?
> > - return ret;
> > + tries--;
> > + } while (tries > 0);
> > +
> > + error:
> > + virReportSystemError(ret, "%s",
> > + _("Failed to balloon domain0 memory"));
> > + return -1;
> > }
> >
> > static void
> > @@ -1271,12 +1274,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> > priv->ctx, &d_config) < 0)
> > goto endjob;
> >
> > - if (cfg->autoballoon && libxlDomainFreeMem(priv, &d_config) < 0) {
> > - virReportError(VIR_ERR_INTERNAL_ERROR,
> > - _("libxenlight failed to get free memory for domain '%s'"),
> > - d_config.c_info.name);
> > + if (cfg->autoballoon && libxlDomainFreeMem(priv, &d_config) < 0)
> > goto endjob;
> > - }
> >
> > if (virHostdevPrepareDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
> > vm->def, VIR_HOSTDEV_SP_PCI) < 0)
>
>
More information about the libvir-list
mailing list