Re: [Xen-devel] [PATCH v5 08/24] libxl: introduce libxl__vnuma_config_check

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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



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