[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw/virtio: fix vhost user fails to startup when
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH] hw/virtio: fix vhost user fails to startup when MQ |
Date: |
Wed, 03 May 2017 12:37:28 +0000 |
Hi
On Wed, May 3, 2017 at 7:09 AM Yang, Zhiyong <address@hidden> wrote:
> Hi,Maxime:
>
> > -----Original Message-----
> > From: Maxime Coquelin [mailto:address@hidden
> > Sent: Tuesday, May 2, 2017 8:23 PM
> > To: Yang, Zhiyong <address@hidden>; address@hidden
> > Cc: address@hidden
> > Subject: Re: [Qemu-devel] [PATCH] hw/virtio: fix vhost user fails to
> startup when
> > MQ
> >
> >
> >
> > On 04/28/2017 09:09 AM, Zhiyong Yang wrote:
> > > Qemu2.7~2.9 and vhost user for dpdk 17.02 release work together to
> > > cause failures of new connection when negotiating to set MQ.
> > > (one queue pair works well).
> > > Because there exist some bugs in qemu code when introducing
> > > VHOST_USER_PROTOCOL_F_REPLY_ACK to qemu. When
> > vhost_user_set_mem_table
> > > is invoked to deal with the vhost message VHOST_USER_SET_MEM_TABLE for
> > > the second time, qemu indeed doesn't send the messge (The message
> > > needs to be sent only once)but still will be waiting for dpdk's reply
> > > ack, then, qemu is always freezing, while DPDK is always waiting for
> > > next vhost message from qemu.
> > > The patch aims to fix the bug, MQ can work well.
> > > The same bug is found in function vhost_user_net_set_mtu, it is
> > > fixed at the same time.
> > > DPDK related patch is as following:
> > > http://www.dpdk.org/dev/patchwork/patch/23955/
> > >
> > > Signed-off-by: Zhiyong Yang <address@hidden>
> > > ---
> > > hw/virtio/vhost-user.c | 18 +++++++++++-------
> > > 1 file changed, 11 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index
> > > 9334a8a..c2c54ce 100644
> > > --- a/hw/virtio/vhost-user.c
> > > +++ b/hw/virtio/vhost-user.c
> > > @@ -205,10 +205,11 @@ static int vhost_user_write(struct vhost_dev
> *dev,
> > VhostUserMsg *msg,
> > > /*
> > > * For non-vring specific requests, like
> VHOST_USER_SET_MEM_TABLE,
> > > * we just need send it once in the first time. For later such
> > > - * request, we just ignore it.
> > > + * request, we just ignore it. In this case, return value is 1
> which is
> > > + * different from 0 that stands for message written successfully.
> > > */
> > > if (vhost_user_one_time_request(msg->request) && dev->vq_index
> != 0) {
> > > - return 0;
> > > + return 1;
> >
> > I personally prefer the fix I suggested in the DPDK mail thread, as
> returning a
> > random positive value does look like a workaround:
>
> I think that for vhost_user_write(), it's behaving in a different way for
> some specific vhost messages.
> So, it should not return the same returen value 0 which stands for success.
>
But you need to do the special handling for every caller.
> >
> > "
> > I think the problem must be fixed generally and not per request.
> > Maybe in vhost_user_write() if one-time request, just clear the
> > VHOST_USER_NEED_REPLY flag. Then, in process_message_reply(), return
> early
> > if this flag isn't set.
> > "
> It's another choise. Either this one nor that one, not a big deal. :)
> Fixing these existing bugs is the most important.
>
>
While the suggestion from Maxime would work transparently, similar to
one-time request are transparent to caller today. I also prefer that
solution.
thanks
--
Marc-André Lureau