Hi, Maxime, Marc-André Lureau: Thank you a lot for your suggestions and I’m agree with you. I will send V2 later according to Maxime’s suggested change.
Thanks Zhiyong From: Marc-André Lureau [mailto:marcandre.lur...@gmail.com] Sent: Wednesday, May 3, 2017 8:37 PM To: Yang, Zhiyong <zhiyong.y...@intel.com>; Maxime Coquelin <maxime.coque...@redhat.com>; qemu-devel@nongnu.org Cc: Liu, Yuanhan <yuanhan....@intel.com>; m...@redhat.com Subject: Re: [Qemu-devel] [PATCH] hw/virtio: fix vhost user fails to startup when MQ Hi On Wed, May 3, 2017 at 7:09 AM Yang, Zhiyong <zhiyong.y...@intel.com<mailto:zhiyong.y...@intel.com>> wrote: Hi,Maxime: > -----Original Message----- > From: Maxime Coquelin > [mailto:maxime.coque...@redhat.com<mailto:maxime.coque...@redhat.com>] > Sent: Tuesday, May 2, 2017 8:23 PM > To: Yang, Zhiyong <zhiyong.y...@intel.com<mailto:zhiyong.y...@intel.com>>; > qemu-devel@nongnu.org<mailto:qemu-devel@nongnu.org> > Cc: m...@redhat.com<mailto:m...@redhat.com> > 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 > > <zhiyong.y...@intel.com<mailto:zhiyong.y...@intel.com>> > > --- > > 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