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

[Elena Ufimtseva <ufimtseva@xxxxxxxxx>]

On Tue, Jul 29, 2014 at 6:43 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> 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.
>

Thank Ian,
will address these as well.

-- 
Elena

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel