Re: [PATCH v2] vhsot-user: Do not wait for replay for not sent VHOST_USER_SET_LOG_BASE

[Michael S. Tsirkin]

On Thu, Aug 01, 2024 at 05:48:36PM +0800, BillXiang wrote:
> 
> > From: "Michael S. Tsirkin"<mst@redhat.com>
> > Date:  Thu, Aug 1, 2024, 16:26
> > Subject:  Re: [PATCH v2] vhsot-user: Do not wait for replay for not sent 
> > VHOST_USER_SET_LOG_BASE
> > To: "BillXiang"<xiangwencheng@dayudpu.com>
> > Cc: <qemu-devel@nongnu.org>
> > On Sun, Jul 21, 2024 at 11:20:56AM +0800, BillXiang wrote:
> > > 
> > > > From: "Michael S. Tsirkin"<mst@redhat.com>
> > > > Date:  Sun, Jul 21, 2024, 03:01
> > > > Subject:  Re: [PATCH v2] vhsot-user: Do not wait for replay for not 
> > > > sent VHOST_USER_SET_LOG_BASE
> > > > To: "BillXiang"<xiangwencheng@dayudpu.com>
> > > > Cc: <qemu-devel@nongnu.org>
> > > > typos in subject do not inspire confidence.
> > > 
> > > Sorry
> > > 
> > > > 
> > > > On Thu, Jul 18, 2024 at 04:31:03PM +0800, BillXiang wrote:
> > > > > From: BillXiang <xiangwencheng@dayudpu.com>
> > > > > 
> > > > > We have added VHOST_USER_SET_LOG_BASE to vhost_user_per_device_request
> > > > > in 
> > > > > https://lists.nongnu.org/archive/html/qemu-devel/2024-06/msg02559.html
> > > > 
> > > > Pls put commit here not ML link:
> > > > commit ABCDEFGHIKLM ("subject")
> > > > 
> > > > > and will send this message only for vq 0.
> > > > 
> > > > you mean "should send"?
> > > > 
> > > 
> > > What I mean is the commit 7c211eb078c4 ("vhost-user: Skip unnecessary 
> > > duplicated VHOST_USER_SET_LOG_BASE requests") 
> > > will cause VHOST_USER_SET_LOG_BASE to be sent only when 'vq_index == 0' 
> > > in vhost_user_write, 
> > > so that we can not use vhost_user_read to get reply when 'vq_index != 0'
> > 
> > 
> > do we try to do it now? in which configurations?
> > 
> 
> Sorry, I'm not certain about what you're instructing me to do.


I am asking that commit log is structured like this:

currently, abc happens, as a result, def happens
do hij, such that klm instead



> Maybe I should merge this patch whit commit 7c211eb078c4
> ("vhost-user: Skip unnecessary duplicated VHOST_USER_SET_LOG_BASE requests") 
>  and "[PATCH] vhsot-user: Remove redundant judgment" into one patch.
> 
> All of these three patches are aimed to do the same thing as 
> commit c98ac64cfb53 ("vhost-user: send set log base message only once")
> which is not a good solution because there is already a function 
> vhost_user_per_device_request
> for those messages that should be sent only once per device.
> 
> In commit 7c211eb078c4 ("vhost-user: Skip unnecessary duplicated 
> VHOST_USER_SET_LOG_BASE requests") 
> I added VHOST_USER_SET_LOG_BASE to vhost_user_per_device_request so that it 
> will be sent only once 
> per device when 'vq_index == 0'.
> After that we should remove the check add by commit c98ac64cfb53 
> ("vhost-user: send set log base message only once")
> in "[PATCH] vhsot-user: Remove redundant judgment".
> And last, in current patch "vhsot-user: Do not wait for replay for not sent 
> VHOST_USER_SET_LOG_BASE"
> we should only read reply when 'vq_index == 0'.
> 
> > 
> > > > 
> > > > > 
> > > > > Signed-off-by: BillXiang <xiangwencheng@dayudpu.com>
> > > > > ---
> > > > > V1[1] -> V2:
> > > > >  - Refrain from appending flags to messages that could 
> > > > > precipitate validation failures upon execution of the 
> > > > > `vhost_user_read_header` function. 
> > > > > 
> > > > > [1]https://lists.nongnu.org/archive/html/qemu-devel/2024-07/msg01923.html
> > > > > ---
> > > > >  hw/virtio/vhost-user.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > > > > index 00561daa06..fd12992d15 100644
> > > > > --- a/hw/virtio/vhost-user.c
> > > > > +++ b/hw/virtio/vhost-user.c
> > > > > @@ -460,7 +460,7 @@ static int vhost_user_set_log_base(struct 
> > > > > vhost_dev *dev, uint64_t base,
> > > > >          return ret;
> > > > >      }
> > > > >  
> > > > > -    if (shmfd) {
> > > > > +    if (shmfd && (dev->vq_index == 0)) {
> > > > 
> > > > extra () not needed here.
> > > > 
> > > > >          msg.hdr.size = 0;
> > > > >          ret = vhost_user_read(dev, &msg);
> > > > >          if (ret < 0) {
> > > > > -- 
> > > > > 2.30.0