Re: [Qemu-devel] [PATCH] dataplane: support viostor virtio-pci status bit setting

[Stefan Hajnoczi]

On Mon, Jan 21, 2013 at 12:05:16PM +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 21, 2013 at 10:36:18AM +0100, Stefan Hajnoczi wrote:
> > On Sat, Jan 19, 2013 at 09:59:57AM +0200, Vadim Rozenfeld wrote:
> > > On Friday, January 18, 2013 05:59:37 PM Stefan Hajnoczi wrote:
> > > > On Thu, Jan 17, 2013 at 04:46:54PM +0100, Stefan Hajnoczi wrote:
> > > > > The viostor virtio-blk driver for Windows does not use the
> > > > > VIRTIO_CONFIG_S_DRIVER bit.  It only sets the 
> > > > > VIRTIO_CONFIG_S_DRIVER_OK
> > > > > bit.
> > > Will be added in the next build.
> > 
> > Nice, thanks.  For compatibility with existing viostor drivers QEMU will
> > carry this patch.
> > 
> > Stefan
> 
> I still think it's wrong: when DRIVER_OK is cleared you should stop
> device I think even if DRIVER is set.
> This patch keeps dataplane running if DRIVER is set.

When virtio-blk-data-plane is started vring indices start from 0.

When the guest submits requests before DRIVER_OK the vring indices
become non-zero.  Now if it sets status DRIVER (but ~DRIVER_OK) we'd
stop the vring.  The guest driver doesn't know we've effectively reset
the vring.

So on the next kick guest and host are no longer in sync and we get one
of the "index moved from 0 to 50" type errors.

So I think the consistent solution is to treat DRIVER as the status
level where vring processing starts and therefore keep the vring alive
until the device is reset.

Your approach allows guests to reset virtqueues by clearing the
DRIVER_OK bit.  I haven't seen anything that depends on this and in
practice Linux guest drivers do not limit vring lifetime to DRIVER_OK,
so it's riskier IMO to stop when DRIVER_OK gets cleared.

Stefan