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 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.
Xen is booted with dom0_mem.
Dom0 has autoballooning disabled in the kernel.

> 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@xxxxxxxxxxxxx>
>
> 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.

Christoph

> +
>      if (target) {
>          *target_memkb = strtoul(target, &endptr, 10);
>          if (*endptr != '\0') {
> @@ -2799,38 +2806,43 @@ retry_transaction:
>              rc = ERROR_FAIL;
>              goto out;
>          }
> -        rc = 0;
> -        goto out;
>      }
>
>      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;
> -
> -    libxl__xs_write(gc, t, target_path, "%"PRIu32,
> -            (uint32_t) info.current_memkb);
> -    libxl__xs_write(gc, t, max_path, "%"PRIu32,
> -            (uint32_t) info.max_memkb);
> -
> -    free_mem_slack = (uint32_t) (PAGE_TO_MEMKB(physinfo.total_pages) -
> -            info.current_memkb);
> -    /* From empirical measurements the free_mem_slack shouldn't be more
> -     * than 15% of the total memory present on the system. */
> -    if (free_mem_slack > PAGE_TO_MEMKB(physinfo.total_pages) * 0.15)
> -        free_mem_slack = PAGE_TO_MEMKB(physinfo.total_pages) * 0.15;
> -    libxl__xs_write(gc, t, free_mem_slack_path, "%"PRIu32,
> free_mem_slack); -
> -    *target_memkb = (uint32_t) info.current_memkb;
> +        goto out;
> +
> +    if (target == NULL) {
> +        libxl__xs_write(gc, t, target_path, "%"PRIu32,
> +                (uint32_t) info.current_memkb);
> +        *target_memkb = (uint32_t) info.current_memkb;
> +    }
> +    if (staticmax == NULL)
> +        libxl__xs_write(gc, t, max_path, "%"PRIu32,
> +                (uint32_t) info.max_memkb);
> +
> +    if (freememslack == NULL) {
> +        free_mem_slack_kb = (uint32_t)
> (PAGE_TO_MEMKB(physinfo.total_pages) - +               
> info.current_memkb);
> +        /* From empirical measurements the free_mem_slack shouldn't be
> more +         * than 15% of the total memory present on the system. */ +  
>      if (free_mem_slack_kb > PAGE_TO_MEMKB(physinfo.total_pages) * 0.15) + 
>           free_mem_slack_kb = PAGE_TO_MEMKB(physinfo.total_pages) * 0.15; +
>        libxl__xs_write(gc, t, free_mem_slack_path, "%"PRIu32,
> free_mem_slack_kb); +    }
>      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;



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