[dpdk-dev] [PATCH v2 1/3] eal/vfio: add support for multiple container

[Wang, Xiao W]

Hi Thomas, Rivet,

> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet at 6wind.com]
> Sent: Thursday, March 22, 2018 5:38 AM
> To: Thomas Monjalon <thomas at monjalon.net>
> Cc: Wang, Xiao W <xiao.w.wang at intel.com>; Chen, Junjie J
> <junjie.j.chen at intel.com>; dev at dpdk.org; maxime.coquelin at redhat.com;
> yliu at fridaylinux.org; Wang, Zhihong <zhihong.wang at intel.com>; Bie, Tiwei
> <tiwei.bie at intel.com>; Xu, Rosen <rosen.xu at intel.com>; Daly, Dan
> <dan.daly at intel.com>; Liang, Cunming <cunming.liang at intel.com>; Burakov,
> Anatoly <anatoly.burakov at intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 1/3] eal/vfio: add support for multiple
> container
> 
> On Wed, Mar 21, 2018 at 09:32:18PM +0100, Thomas Monjalon wrote:
> > Hi,
> >
> > 21/03/2018 14:21, Xiao Wang:
> > > +#endif /* VFIO_PRESENT */
> > >  #endif /* _RTE_VFIO_H_ */
> >
> > Please keep the empty line which was present between endif.

OK.
> >
> > > +	rte_vfio_create_container;
> > > +	rte_vfio_destroy_container;
> > > +	rte_vfio_bind_group_no;
> > > +	rte_vfio_unbind_group_no;
> > > +	rte_vfio_dma_map;
> > > +	rte_vfio_dma_unmap;
> > > +	rte_vfio_get_group_fd;
> >
> > Please keep alphabetical order.

OK. Will do.
> >
> > About the naming, I see "no" and "idx" are used.
> > Other APIs in DPDK are using "num" and "id". Any strong opinion?
> 
> {bind,unbind}_group is sufficient as a name.
> _no is redundant as implicit from the parameter type.

{bind,unbind}_group looks very neat. Will remove "_no".
For the eal_vfio.c internal API with "_idx" postfix, the return value is an index of an array. I think "idx" is appropriate. Before this patch, we already have function get_vfio_group_idx.

Thanks for the comments.
-Xiao
> 
> --
> Gaëtan Rivet
> 6WIND