Re: [PATCH] xl create: endless loop

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

On Tue, 19 Oct 2010, Christoph Egger wrote:
> On Monday 18 October 2010 14:50:35 Christoph Egger wrote:
> > Hi!
> >
> > I cannot start a guest with 'xl create' due to an endless loop in libxl.c,
> > function libxl__get_free_memory_slack():
> >
> > There is this code snippet:
> >
> > retry:
> >     free_mem_slack_s = libxl__xs_read(gc, XBT_NULL, free_mem_slack_path);
> >     if (!free_mem_slack_s) {
> >         rc = libxl__fill_dom0_memory_info(gc, &target_memkb);
> >         if (rc < 0)
> >             return rc;
> >         goto retry;
> >     } else {
> >
> >
> > libxl__xs_read() returns 0 and libxl__fill_dom0_memory_info() also returns
> > 0. So there's a loop of retries.
> 
> Attached patch fixes the endless loop.
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

thanks for the patch but this cannot possibly be the right fix:


diff -r 9d8d6b93114e tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Oct 18 15:24:39 2010 +0200
+++ b/tools/libxl/libxl.c	Tue Oct 19 10:29:00 2010 +0200
@@ -2836,7 +2836,7 @@ retry:
     free_mem_slack_s = libxl__xs_read(gc, XBT_NULL, free_mem_slack_path);
     if (!free_mem_slack_s) {
         rc = libxl__fill_dom0_memory_info(gc, &target_memkb);
-        if (rc < 0)
+        if (rc <= 0)
             return rc;
         goto retry;
     } else {

the idea is that libxl__fill_dom0_memory_info should fill the missing
informations in xenstore so that we can go ahead and try to read them
again and the second time should be successful.
Libxl__fill_dom0_memory_info returns 0 on success, so it is correct to
goto retry in that case.

The bug must be in libxl__fill_dom0_memory_info that doesn't return
error when it should.
Does the appended patch works for you?

---

diff -r 00b92112b055 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Wed Oct 20 17:26:51 2010 +0100
+++ b/tools/libxl/libxl.c	Thu Oct 21 15:08:29 2010 +0100
@@ -2793,11 +2793,11 @@ retry_transaction:
 
     rc = libxl_domain_info(ctx, &info, 0);
     if (rc < 0)
-        return rc;
+        goto out;
 
     rc = libxl_get_physinfo(ctx, &physinfo);
     if (rc < 0)
-        return rc;
+        goto out;
 
     libxl__xs_write(gc, t, target_path, "%"PRIu32,
             (uint32_t) info.current_memkb);
@@ -2816,9 +2816,12 @@ retry_transaction:
     rc = 0;
 
 out:
-    if (!xs_transaction_end(ctx->xsh, t, 0))
+    if (!xs_transaction_end(ctx->xsh, t, 0)) {
         if (errno == EAGAIN)
             goto retry_transaction;
+        else
+            rc = ERROR_FAIL;
+    }
 
 
     return rc;