WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel][PATCH] xl create: endless loop

On Monday 08 November 2010 09:18:19 Zhang, Yang Z wrote:
> Did the patch apply? This issue still exist in xen-unstable(22364).

Yes.

>
> best regards
> yang
>
> > -----Original Message-----
> > From: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
> > [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Christoph
> > Egger Sent: Thursday, November 04, 2010 11:40 PM
> > To: xen-devel@xxxxxxxxxxxxxxxxxxx
> > Cc: Jackson; Stefano Stabellini
> > Subject: Re: [Xen-devel][PATCH] xl create: endless loop
> >
> > On Wednesday 03 November 2010 17:12:47 Christoph Egger wrote:
> > > On Thursday 21 October 2010 16:11:55 Stefano Stabellini wrote:
> > > > 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@xxxxxxx>
> > > >
> > > > 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;
> > >
> > > No, this patch has no effect for me.
> > > In libxl__fill_dom0_memory_info(), the code path goes that way:
> > >
> > >     t = xs_transaction_start(ctx->xsh);
> > >
> > >     target = libxl__xs_read(gc, t, target_path);
> > >     if (target) {    <-- target contains "5"
> > >         *target_memkb = strtoul(target, &endptr, 10);
> > >         if (*endptr != '\0') {     <-- *endptr contains '\0'
> > >             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> > >                     "invalid memory target %s from %s\n", target,
> > > target_path);
> > >             rc = ERROR_FAIL;
> > >             goto out;
> > >         }
> > >         rc = 0;
> > >         goto out;    <-- take this jump with rc being 0
> > >     }
> >
> > A slightly modified version works.
> >
> > Signed-off-by: Mark Langsdorf <Mark.Langsdorf@xxxxxxx>
> > Signed-off-by: Christoph Egger <Christoph.Egger@xxxxxxx>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel