[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 2/2] vhost: Fix last vq queue index of devices with no cvq
From: |
Eugenio Perez Martin |
Subject: |
Re: [PATCH v3 2/2] vhost: Fix last vq queue index of devices with no cvq |
Date: |
Thu, 4 Nov 2021 07:34:28 +0100 |
On Thu, Nov 4, 2021 at 3:47 AM Jason Wang <jasowang@redhat.com> wrote:
>
> On Wed, Nov 3, 2021 at 6:01 PM Eugenio Pérez <eperezma@redhat.com> wrote:
> >
> > The -1 assumes that last index counts all vhost device models as having
> > two queues, but they count only the ones that models the data queues.
> >
> > Because of that, the right change in last_index is to actually add the
> > cvq, not to remove the missing one.
> >
> > This is not a problem to vhost-net, but it is to vhost-vdpa, which
> > device model trust to reach the last index to finish starting the
> > device.
> >
> > Tested with vp_vdpa with host's vhost=on and vhost=off, with ctrl_vq=on
> > and ctrl_vq=off.
> >
> > Fixes: 049eb15b5fc9 ("vhost: record the last virtqueue index for the virtio
> > device")
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> > hw/net/vhost_net.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 081946dc93..fe2b8a2b83 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -329,8 +329,8 @@ int vhost_net_start(VirtIODevice *dev, NetClientState
> > *ncs,
> > int r, e, i, last_index = data_queue_pairs * 2;
> > NetClientState *peer;
> >
> > - if (!cvq) {
> > - last_index -= 1;
> > + if (cvq) {
> > + last_index += 1;
>
> Patch looks ok.
>
> Not a native speaker but I guess "last" should be inside the closed
> interval. If this is true, I still prefer to change the check in
> vhost_vdpa_dev_start().
>
You are right, it's the way we are using it in iova address and there
are some other examples. I'll rename to end, similar to other
containers.
Thanks!
> Thanks
>
> > }
> >
> > if (!k->set_guest_notifiers) {
> > --
> > 2.27.0
> >
>