[dpdk-dev] [PATCH 1/4] vhost: move fdset functions from fd_man.c to fd_man.h

[Yang, Zhiyong]

> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin at redhat.com]
> Sent: Wednesday, February 28, 2018 1:52 AM
> To: Yang, Zhiyong <zhiyong.yang at intel.com>; dev at dpdk.org;
> yliu at fridaylinux.org; Tan, Jianfeng <jianfeng.tan at intel.com>; Bie, Tiwei
> <tiwei.bie at intel.com>; Wang, Zhihong <zhihong.wang at intel.com>
> Cc: Wang, Dong1 <dong1.wang at intel.com>
> Subject: Re: [PATCH 1/4] vhost: move fdset functions from fd_man.c to
> fd_man.h
> 
> Hi Zhiyong,
> 
> On 02/14/2018 03:53 PM, Zhiyong Yang wrote:
> > The patch moves fdset related funcitons from fd_man.c to fd_man.h in
> > order to reuse these funcitons from the perspective of PMDs.
> >
> > Signed-off-by: Zhiyong Yang <zhiyong.yang at intel.com>
> > ---
> >   lib/librte_vhost/Makefile |   3 +-
> >   lib/librte_vhost/fd_man.c | 274 ----------------------------------------------
> >   lib/librte_vhost/fd_man.h | 258
> +++++++++++++++++++++++++++++++++++++++++--
> >   3 files changed, 253 insertions(+), 282 deletions(-)
> >   delete mode 100644 lib/librte_vhost/fd_man.c
> 
> I disagree with the patch.
> It is a good thing to reuse the code, but to do it, you need to extend the
> vhost lib API.
> 
> New API need to be prefixed with rte_vhost_, and be declared in
> rte_vhost.h.
> 
> And no need to move the functions from the .c to the .h file, as it moreover
> makes you inline them, which is not necessary here.

Thanks for your reviewing the series firstly, Maxime. :)

I considered to do it as you said. However I still preferred this one at last.
Here are my reasons.
1) As far as I know, this set of functions are used privately in librte_vhost before this feature.
No strong request from the perspective of DPDK application. If I understand well,  It is enough to expose the functions to all PMDs
And it is better to keep internal use in DPDK.

2) These functions help to implement vhost user, but they are not strongly related to other APIs of vhost user which have already exposed.
if we want to expose them as APIs at lib layer, many functions and related data structure has to be exposed in rte_vhost.h. it looks messy.
Your opinion?

thanks
Zhiyong