Re: [dpdk-dev] [PATCH v5] vfio: change to use generic multi-process channel

["Tan, Jianfeng" <jianfeng.tan@intel.com>]

Hi Anatoly,

Thank you for the review. All your comments will be addressed in next version, except for below concern which might be taken care of in another patch if it also concerns you.

> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Wednesday, March 14, 2018 9:27 PM
> To: Tan, Jianfeng; dev@dpdk.org
> Cc: Richardson, Bruce; Ananyev, Konstantin; thomas@monjalon.net
> Subject: Re: [PATCH v5] vfio: change to use generic multi-process channel
[...]
> 
> > +	mp_req.len_param = sizeof(*p);
> > +	mp_req.num_fds = 0;
> > +
> > +	vfio_group_fd = -1;
> > +	if (rte_mp_request(&mp_req, &mp_reply, &ts) == 0 &&
> > +	    mp_reply.nb_received == 1) {
> > +		mp_rep = &mp_reply.msgs[0];
> > +		p = (struct vfio_mp_param *)mp_rep->param;
> > +		if (p->result == SOCKET_OK && mp_rep->num_fds == 1) {
> > +			cur_grp->group_no = iommu_group_no;
> > +			vfio_group_fd = mp_rep->fds[0];
> > +			cur_grp->fd = vfio_group_fd;
> > +			vfio_cfg.vfio_active_groups++;
> >   		}
> > +		free(mp_reply.msgs);
> >   	}
> > -	return -1;
> > +
> > +	if (vfio_group_fd < 0)
> > +		RTE_LOG(ERR, EAL, "  cannot request group fd\n");
> > +	return vfio_group_fd;
> 
> p->result can be SOCKET_NO_FD, in which case returned value should be
> zero. I think this is missing from this code. There probably should be
> an "else if (p->result == SOCKET_NO_FD)" clause that sets return value to 0.
> 
> You should be able to test this by trying to set up a device for VFIO
> that isn't bound to VFIO driver, in a secondary process.

OK, I will fix this.

But really, "zero" could be ambiguous as a fd could, theoretically, be zero too.

Thanks,
Jianfeng