Re: [PATCH] xl create: endless loop

[Christoph Egger <Christoph.Egger@amd.com>]

On Wednesday 10 November 2010 12:52:42 Stefano Stabellini wrote:
> On Wed, 10 Nov 2010, Christoph Egger wrote:
> > On Tuesday 09 November 2010 19:08:14 Stefano Stabellini wrote:
> > > On Wed, 3 Nov 2010, Christoph Egger wrote:
> > > > 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
> > > >     }
> > >
> > > The problem you are having is that somebody in your system is setting a
> > > target for dom0 without setting freemem-slack. Are you still running
> > > xend at boot?
> >
> > Yes, I do.
>
> running xend alongside xl is not recommended, it could cause bugs,
> especially if you don't disable autoballooning
>
>
> > > Currently libxl__fill_dom0_memory_info assumes that both values are set
> > > initially at the same time (by libxl__fill_dom0_memory_info).
> > > This patch should fix the issue, I would appreciate if you could test
> > > it.
> > >
> > > ---
> > >
> > >
> > > libxl: do not assume target and freemem-slack are written at the same
> > > time
> > >
> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > >
> > > diff -r 7188d1e4b0e1 tools/libxl/libxl.c
> > > --- a/tools/libxl/libxl.c	Tue Nov 09 12:00:05 2010 +0000
> > > +++ b/tools/libxl/libxl.c	Tue Nov 09 18:05:52 2010 +0000
> > > @@ -2779,18 +2779,25 @@ static int libxl__fill_dom0_memory_info(
> > >      int rc;
> > >      libxl_dominfo info;
> > >      libxl_physinfo physinfo;
> > > -    char *target = NULL, *endptr = NULL;
> > > +    char *target = NULL, *staticmax = NULL, *freememslack = NULL,
> > > *endptr = NULL; char *target_path = "/local/domain/0/memory/target";
> > >      char *max_path = "/local/domain/0/memory/static-max";
> > >      char *free_mem_slack_path =
> > > "/local/domain/0/memory/freemem-slack"; xs_transaction_t t;
> > >      libxl_ctx *ctx = libxl__gc_owner(gc);
> > > -    uint32_t free_mem_slack = 0;
> > > +    uint32_t free_mem_slack_kb = 0;
> > >
> > >  retry_transaction:
> > >      t = xs_transaction_start(ctx->xsh);
> > >
> > >      target = libxl__xs_read(gc, t, target_path);
> > > +    staticmax = libxl__xs_read(gc, t, target_path);
> > > +    freememslack = libxl__xs_read(gc, t, target_path);
> > > +    if (target && staticmax && freememslack) {
> > > +        rc = 0;
> > > +        goto out;
> > > +    }
> >
> > *target, *staticmax and *freememslack contain the value "5".
> > So with this patch, rc always returns 0 from there.
>
> that is correct: the values are there, so there is no need to write them
> to xenstore again, and the caller should be able to read the target
> correctly

> > Xen is booted with dom0_mem.
> > Dom0 has autoballooning disabled in the kernel.
>
> You still need to disable autoballooning in xl, setting autoballooning
> to 0 in /etc/xen/xl.conf

Yes, that makes the patch work for me. I have success in booting the
guest the first time with 'xl create'.

Please commit the patch.

Christoph


-- 
---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