Re: [PATCH] virtio: add the queue number check

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Jan 31, 2020 at 05:22:47PM +0100, Paolo Bonzini wrote:
> I have just found this email... sorry for the delay.
> 
> On 10/01/20 07:10, Yang Zhong wrote:
> >> No. If virtio-blk works, the bug is in vhost-user-blk; if virtio-blk needs
> >> no check in cpu count, vhost-user-blk also doesn't.
> >>
> >> You need to check first if the bug is in QEMU or the vhost-user-blk server.
> >
> >   (1). Seabios issue
> >   In init_virtio_blk() function, which set VIRTIO_CONFIG_S_DRIVER_OK
> >   status to qemu vhost-user-blk device.
> > 
> >   // the related code
> >   ......
> >   status |= VIRTIO_CONFIG_S_DRIVER_OK;
> >   vp_set_status(&vdrive->vp, status);
> >   ......
> > 
> >   I think there is no need for seabios to set VIRTIO_CONFIG_S_DRIVER_OK
> >   status to qemu vhost-user-blk device.
> 
> It does so because it cannot know how it will be used.  It could be used
> by the guest boot loader to load a kernel, for example.  SeaBIOS sets
> DRIVER_OK because it has loaded a driver for the disk; that's exactly
> what DRIVER_OK signals.

Right. More specifically DRIVER_OK means driver finished setup and is
going to add buffers and process used ones, so device should start
looking at queues.

> 
> >     In fact, this time vhost_user_blk_start almost do nothing because
> >     the real guest virtio-blk driver still not started yet. This time,
> >     there is only one vq can be used(this vq should be inited in seabios).
> > 
> >     When the guest virtio-blk driver really start and complet the
> >     probe(), the guest virtio-blk driver will set
> >     VIRTIO_CONFIG_S_DRIVER_OK to vhost-user-blk device again. This
> >     time, this driver will allocate RIGHT queue num according to
> >     MIN(vcpu, num_vqs).
> 
> Doesn't it first reset the status to 0?
> 
> >     (2). DPDK issue
> >      DPDK does not know the real queue number used by guest virtio-blk
> >      driver and it only know the queue number from vhost-user-blk
> >      commond line. Once the guest virtio-blk driver change the queue
> >      number according to MIN(vcpu, num_vqs), DPDK still use previous
> >      queue number and it think virtio is never ready by
> >      virtio_is_ready() function.
> 
> What is virtio_is_ready()?  The virtio device should not wait for all
> the queues to be set.  A device is ready when it sets DRIVER_OK, and
> that's it.

Or - if we want to support legacy guests, and due to a bunch of legacy
guest bugs - if a legacy guest kicked a queue before setting DRIVER_OK.

> >      or DPDK can get the real queue number by checking if the vring.desc
> >      is NON-NULL.
> 
> Note that there is no requirement that the driver initializes a
> consecutive number of virtqueues.  It is acceptable for it to initialize
> virtqueues 0, 1 and 57.  It seems like the bug is in DPDK, possibly more
> than one...
> 
> Paolo
> 
> >      By the way, vhost SCSI device has the same issue with
> >      vhost-user-blk device. 
> > 
> >      Yang
> > 
> >> Paolo
> >