On 9/30/2017 4:16 PM, Yuanhan Liu wrote:
On Sat, Sep 30, 2017 at 12:03:33PM +0800, Tan, Jianfeng wrote:
+ if (rte_eal_mp_sendmsg("vhost pmd", params, sizeof(*params) + len,
To me, it's not a good idea to identify an object by a string. The common
practice is to use a handler, which could either be a struct or a nubmer.
My point is that if we choose the way you suggested, we need somewhere to
maintain them. For example, every time we need register a new action, we
need to update that file to add a new entry (number).
Not really. You could let the register function to return a struct, in
which there is a name field.
Anyway, I think it's not a big deal to turn it to struct, judging that
it may only contain one field only: the name. But at least, you should
not type the same string everywhere. Use macro instead.
Agreed. Will fix that.
In current way, the duplicated register with the same name will fail,
developers is responsible to check that.
+ fds, mem->nregions) < 0) {
+ RTE_LOG(ERR, PMD, "Failed to share mem table\n");
+ free(params);
+ return -1;
+ }
+
+ /* share callfd and kickfd */
+ params->type = VHOST_MSG_TYPE_SET_FDS;
+ vring_num = rte_vhost_get_vring_num(vid);
+ for (i = 0; i < vring_num; i++) {
+ if (rte_vhost_get_vhost_vring(vid, i, &vring) < 0) {
If you save the fds here, you don't have to get it every time when there
is a new secondary process attached. Then as I have suggested firstly,
you don't have to introduce callfd_pri in the vhost lib.
If we don't introduce callfd_pri, when we do virtqueue cleanup (or similar
operations) in vhost lib, it will wrongly close fds belonging to secondary
process.
You remind me that, instead of introduce callfd_pri, we can introduce
callfd_effective, to reduce the modification lines.
It's not about how many lines are modified. You were adding "effective"
fds, which is a semantic change. It makes the logic a bit more complex.
What's worse, it even doesn't resolve the issue completely.
Yes, it still has limitation. Just wonder if possible to move event fd
write out of vhost lib.
Thanks,
Jianfeng
--yliu
