From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wei Liu <wei.liu2@citrix.com>
Subject: Re: [PATCH v5 08/24] libxl: introduce
	libxl__vnuma_config_check
Date: Sun, 22 Feb 2015 15:47:24 +0000
Message-ID: <20150222154724.GX2159@zion.uk.xensource.com>
References: <1423770294-9779-1-git-send-email-wei.liu2@citrix.com>
	<1423770294-9779-9-git-send-email-wei.liu2@citrix.com>
	<1424191079.4235.88.camel@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Content-Disposition: inline
In-Reply-To: <1424191079.4235.88.camel@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>, "JBeulich@suse.com" <JBeulich@suse.com>, Andrew Cooper <Andrew.Cooper3@citrix.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, "ufimtseva@gmail.com" <ufimtseva@gmail.com>, Ian Jackson <Ian.Jackson@citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>
List-Id: xen-devel@lists.xenproject.org

On Tue, Feb 17, 2015 at 04:38:02PM +0000, Dario Faggioli wrote:
[...]
> > +
> > +/* Check if vNUMA configuration is valid:
> > + *  1. all pnodes inside vnode_to_pnode array are valid
> > + *  2. one vcpu belongs to and only belongs to one vnode
> >
> something like "each vcpu belongs to one and only one vnode" sounds
> better here...
> 
> > + *  3. each vmemrange is valid and doesn't overlap with each other
> >
> "doesn't overlap with any other one" perhaps (of course, I'm not a
> native speaker, so I can be very wrong! :-D)
> 
> > + */
> > +int libxl__vnuma_config_check(libxl__gc *gc,
> > +                              const libxl_domain_build_info *b_info,
> > +                              const libxl__domain_build_state *state)
> > +{
> > +    int i, j, rc = ERROR_VNUMA_CONFIG_INVALID, nr_nodes;
> >
> You should init nr_nodes to = 0 (see below).
> 
> > +    libxl_numainfo *ninfo = NULL;
> > +    uint64_t total_memkb = 0;
> > +    libxl_bitmap cpumap;
> > +    libxl_vnode_info *p;
> > +
> *v, or *vnode, or *vinfo, all sounds better than *p.
> 
> > +    rc = 0;
> > +out:
> > +    if (ninfo) libxl_numainfo_dispose(ninfo);
> >
> What you want here, to free a libxl_numainfo, is
> libxl_numainfo_list_free(ninfo,nr_nodes), which, BTW, can be called
> without checking whether ninfo is NULL, _provided_ you initialize
> nr_nodes to 0.
> 

All fixed. Thanks for reviewing.

Wei.

> Regards,
> Dario