Hi Jiawei, >From: 17826875952 <17826875...@163.com> >Sent: Friday, December 11, 2020 1:31 AM >To: Xia, Chenbo <chenbo....@intel.com> >Cc: maxime.coque...@redhat.com >Subject: Re:RE: [PATCH] net/virtio-user: fix error run close(0) > > >Hi Chenbo, >Thanks for you comment！ > >At 2020-12-09 19:31:19, "Xia, Chenbo" <mailto:chenbo....@intel.com> wrote: >>Hi Jiawei, >> >>Thanks for catching this! >>Comments inline. >> >>> -----Original Message----- >>> From: Jiawei Zhu <mailto:17826875...@163.com> >>> Sent: Saturday, November 28, 2020 10:50 PM >>> To: mailto:dev@dpdk.org >>> Cc: mailto:liweife...@huawei.com; mailto:zhujiawe...@huawei.com; >>> mailto:maxime.coque...@redhat.com; >>> Xia, Chenbo <mailto:chenbo....@intel.com> >>> Subject: [PATCH] net/virtio-user: fix error run close(0) >>> >>> From: Jiawei Zhu <mailto:zhujiawe...@huawei.com> >>> >>> When i < VIRTIO_MAX_VIRTQUEUES and j == i, >>> dev->callfds[i] and dev->kickfds[i] are default 0. >>> So it will close(0), close the standard input (stdin). >>> >>> Fixes: e6e7ad8b3024 ("net/virtio-user: move eventfd open/close into >>> init/uninit") >>> Cc: mailto:sta...@dpdk.org >>> >>> Signed-off-by: Jiawei Zhu <mailto:zhujiawe...@huawei.com> >>> --- >>> drivers/net/virtio/virtio_user/virtio_user_dev.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c >>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c >>> index 053f026..1bfd223 100644 >>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c >>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c >>> @@ -284,7 +284,7 @@ int virtio_user_stop_device(struct virtio_user_dev *dev) >>> } >>> >>> if (i < VIRTIO_MAX_VIRTQUEUES) { >>> - for (j = 0; j <= i; ++j) { >>> + for (j = 0; j < i; ++j) { >> >>With the help of your patch, I notice another defect that if the code fails >>in kickfd >>creation, we will leave one callfd not closed. Since you are here, could you >>help solve >>this too? A potential solution could be doing 'dev->callfds[i] = callfd' just >>after callfd >>creation, keeping 'j <= i' and adding checks before close(). What do you >>think? > >This solution is ok to solve this，but I think the checks ars superfluous for >'j < i'. >So I think adding ‘close(callfd)’ before break when fails in kickfd creation >and keeping 'j < i'. >What do you think?
Yes, that's also a viable solution. Please go ahead with this 😊. Btw, next time you reply to patch email, please: 1. Better use plain text rather than HTML. 2. cc to dev@dpdk.org to make our discussion open to community. And since you will send new version now, please add v2 as patch prefix. Otherwise maintainers will be confused. Thanks! Chenbo > >> >>Btw, I noticed that you have sent multiple patches that have same content. If >>you want to >>send new version. Please --in-reply-to this patch as this is the one that >>shows in patchwork. >>(http://patchwork.dpdk.org/patch/84626/) >> >>Thanks! >>Chenbo >> >>> close(dev->callfds[j]); >>> close(dev->kickfds[j]); >>> } >>> -- >>> 1.8.3.1 >> >Thanks! >Jiawei
