On 20-Mar-18 10:33 AM, Burakov, Anatoly wrote:
On 19-Mar-18 6:53 AM, Tan, Jianfeng wrote:
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; tho...@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.
You're correct. Maybe return 0/-1 in case of success/failure and put fd
into a pointer? i.e.
int func(int *vfio_group_fd) {
<...>
*vfio_group_fd = fd;
return 0;
}
Or rather return 1/0/-1 depending on whether we got SOCKET_OK,
SOCKET_NO_FD or SOCKET_ERR.
Thanks,
Jianfeng
--
Thanks,
Anatoly
