Am 24.06.2020 um 05:36 hat Coiby Xu geschrieben:
> On Thu, Jun 18, 2020 at 12:43:47PM +0200, Kevin Wolf wrote:
> > Am 14.06.2020 um 20:39 hat Coiby Xu geschrieben:
> > > Allow vu_message_read to be replaced by one which will make use of the
> > > QIOChannel functions. Thus reading vhost-user message won't stall the
> > > guest.
> > >
> > > Signed-off-by: Coiby Xu <[email protected]>
> >
> > _vu_queue_notify() still has a direct call of vu_message_read() instead
> > of using the pointer. Is this intentional?
>
> This is a mistake. Thank you for reminding me!
>
> > Renaming the function would make sure that such semantic merge conflicts
> > don't stay unnoticed.
>
> Thank you for this tip! Do you suggest renaming the function only for
> the purpose of testing or should I adopt a name when submitting the
> patch? For the latter case, I will change it to vu_message_read_default
> then.
I think I would rename it permanently and keep the new name for the
actual submission. The reason is that if someone else is working on a
series adding new references, the compiler would catch it there, too.
vu_message_read_default() sounds good to me.
> > > @@ -1704,6 +1702,7 @@ vu_deinit(VuDev *dev)
> > > }
> > >
> > > if (vq->kick_fd != -1) {
> > > + dev->remove_watch(dev, vq->kick_fd);
> > > close(vq->kick_fd);
> > > vq->kick_fd = -1;
> > > }
> >
> > This hunk looks unrelated.
>
> In v4, I made the comment to explain why it's needed. But libvhost-user
> is supposed to be independent from QEMU, so Stefan suggested to remove it,
Yes, I saw the reason why you need it in later patches.
If you can remove it completely, that is even better, but otherwise I
would make the addition only later (either in the patch that actually
needs it or in a new separate patch) because it's not really part of
"allowing vu_message_read to be replaced", as the commit message says.
Kevin
