Re: [PATCH v6 07/10] libxc: allocate domain memory for vnuma enabled

[Ian Campbell <Ian.Campbell@citrix.com>]

On Fri, 2014-07-18 at 01:50 -0400, Elena Ufimtseva wrote:
> diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> index 40d3408..23267ed 100644
> --- a/tools/libxc/xc_dom_x86.c
> +++ b/tools/libxc/xc_dom_x86.c
> @@ -756,26 +756,6 @@ static int x86_shadow(xc_interface *xch, domid_t domid)
>      return rc;
>  }
>  
> -int arch_boot_alloc(struct xc_dom_image *dom)
> -{
> -        int rc = 0;
> -        xen_pfn_t allocsz, i;
> -
> -        /* allocate guest memory */
> -        for ( i = rc = allocsz = 0;
> -              (i < dom->total_pages) && !rc;
> -              i += allocsz )
> -        {
> -            allocsz = dom->total_pages - i;
> -            if ( allocsz > 1024*1024 )
> -                allocsz = 1024*1024;
> -            rc = xc_domain_populate_physmap_exact(
> -                dom->xch, dom->guest_domid, allocsz,
> -                0, 0, &dom->p2m_host[i]);
> -        }
> -        return rc;
> -}

You only just moved this here in the last patch! Please move it to the
right place from the beginning.

> -
>  int arch_setup_meminit(struct xc_dom_image *dom)
>  {
>      int rc;
> @@ -832,6 +812,13 @@ int arch_setup_meminit(struct xc_dom_image *dom)
>          for ( pfn = 0; pfn < dom->total_pages; pfn++ )
>              dom->p2m_host[pfn] = pfn;
>  
> +        /* allocate guest memory */
> +        if ( dom->nr_nodes == 0 ) {
> +            xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
> +                         "%s: Cannot allocate domain memory for 0 vnodes\n",
> +                         __FUNCTION__);

Should this not indicate a system where vnuma is not enabled?

> +            return -EINVAL;
> +        }
>          rc = arch_boot_alloc(dom);
>          if ( rc )
>              return rc;

> diff --git a/tools/libxc/xg_private.h b/tools/libxc/xg_private.h
> index e593364..21e4a20 100644
> --- a/tools/libxc/xg_private.h
> +++ b/tools/libxc/xg_private.h
> @@ -123,6 +123,7 @@ typedef uint64_t l4_pgentry_64_t;
>  #define ROUNDUP(_x,_w) (((unsigned long)(_x)+(1UL<<(_w))-1) & ~((1UL<<(_w))-1))
>  #define NRPAGES(x) (ROUNDUP(x, PAGE_SHIFT) >> PAGE_SHIFT)
>  
> +#define VNUMA_NO_NODE ~((unsigned int)0)

This was defined in a previous patch too, in a libxl header, I think.

If this is not to be exposed to libxl users then you don't need the
libxl copy at all -- you should add this to a public libxc header, with
a suitable namespace prefix, and libxl can consume it from there.

Ian.