Re: [Qemu-devel] [RFC 25/29] vhu: enable = false on get_vring_base

[Dr. David Alan Gilbert]

* Michael S. Tsirkin (address@hidden) wrote:
> On Wed, Jun 28, 2017 at 08:00:43PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> > 
> > When we receive a GET_VRING_BASE message set enable = false
> > to stop any new received packets modifying the ring.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> 
> I think I already reviewed a similar patch.

Yes you replied to my off-list mail; I hadn't got
around to fixing it yet.

> Spec says:
> Client must only process each ring when it is started.

but in that reply you said the spec said 

  Client must only pass data between the ring and the
  backend, when the ring is enabled.

So does the spec say 'started' or 'enabled'
(Pointer to the spec?)

> IMHO the real fix is to fix client to check the started
> flag before processing the ring.

Yep I can do that.  I was curious however whether it was
specified as 'started' or 'enabled' or both.

Dave

> > ---
> >  contrib/libvhost-user/libvhost-user.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/contrib/libvhost-user/libvhost-user.c 
> > b/contrib/libvhost-user/libvhost-user.c
> > index ceddeac74f..d37052b7b0 100644
> > --- a/contrib/libvhost-user/libvhost-user.c
> > +++ b/contrib/libvhost-user/libvhost-user.c
> > @@ -652,6 +652,7 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)
> >      vmsg->size = sizeof(vmsg->payload.state);
> >  
> >      dev->vq[index].started = false;
> > +    dev->vq[index].enable = false;
> >      if (dev->iface->queue_set_started) {
> >          dev->iface->queue_set_started(dev, index, false);
> >      }
> > -- 
> > 2.13.0
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK