Re: [Qemu-devel] [PATCH WIP 0/4] vhost-scsi: new device supporting the tcm_vhost Linux kernel module

[Nicholas A. Bellinger]

Hi Paolo,

On Wed, 2013-01-30 at 17:41 +0100, Paolo Bonzini wrote:
> Ok, so here is my attempt at a vhost-scsi device.  I'm creating an
> entirely separate device, with the common parts of virtio-scsi and
> vhost-scsi (actually little more than the initialization) grouped into
> a VirtIOSCSICommon type.  The device is used simply like "-device
> vhost-scsi-pci,wwpn=WWPN", with all configuration done in configfs
> beforehand.
> 

Cool.  :)

> As expected, using a separate device finds a snag: vhost-scsi is passing
> force=false to vhost_dev_init, and the BIOS does not use MSI-X so it
> will actually use the non-vhost implementation which is wrong.  I fixed
> this by passing force=true; I'm not sure what that would break, but I
> figured "not much" since the BIOS polls and does not rely on interrupts.
> 
> That makes vhost start, but it still doesn't work for me with a 3.7.2
> kernel on the host.  Even Nick's patches hang the guest as soon as vhost
> starts, and I get the same behavior with mine.

After bisection this evening, the change that ended up breaking
vhost-scsi is vhost backend guest notifier masking support patch in
commit f56a12475.

After adding the two new notifiers in vhost-scsi/virtio-scsi following
in qemu.git/virtio-scsi-nab code, the tcm_vhost LUN scan is functioning
again..

>   (Of course with my
> patches the BIOS hangs and you never reach Linux; but try a BIOS without
> virtio-scsi support, and you'll see Linux hanging in the same way).
> 
> Here is my configuration:
> 
>   cd /sys/kernel/config/target
>   mkdir -p core/fileio_0/fileio
>   echo 'fd_dev_name=/home/pbonzini/test.img,fd_dev_size=5905580032' > 
> core/fileio_0/fileio/control 
>   echo 1 > core/fileio_0/fileio/enable
>   mkdir -p vhost/naa.600140554cf3a18e/tpgt_0/lun/lun_0
>   cd vhost/naa.600140554cf3a18e/tpgt_0
>   ln -sf ../../../../../core/fileio_0/fileio/ lun/lun_0/virtual_scsi_port
>   echo naa.60014053226f0388 > nexus
> 
> Nick's patches are run with "-vhost-scsi 
> id=vs,tpgt=0,wwpn=naa.600140554cf3a18e
> -device virtio-scsi-pci,vhost-scsi=vs".  Perhaps I'm doing something wrong.

So after adding the same vhost backend guest notifiers to the new
VirtIOSCSICommon vhost-scsi code, I'm now hitting an QEMU invalid
option:

./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -smp 4 -m 2048 -serial
file:/tmp/vhost-serial.txt
-hda /usr/src/qemu-vhost.git/debian_squeeze_amd64_standard-old.qcow2
-vhost-scsi id=vs,tpgt=0,wwpn=naa.600140579ad21088 -device
virtio-scsi-pci,vhost-scsi=vs
qemu-system-x86_64: -vhost-scsi: invalid option

Debugging this now..

> 
> Another small bug I found is an ordering problem between
> VHOST_SET_VRING_KICK and VHOST_SCSI_SET_ENDPOINT.  Starting the vq
> causes a "vhost_scsi_handle_vq endpoint not set" error in dmesg.
> Because of this I added the first two patches, which let me do
> VHOST_SCSI_SET_ENDPOINT before VHOST_SET_VRING_KICK but after setting
> up the vring.
> 
> Unfortunately, this is not enough to fix the hang.  And anyway, it's
> probably simpler to avoid the two patches and remove this test from the
> tcm_vhost.c vhost_scsi_set/clear_endpoint functions:
> 
>         mutex_lock(&vs->dev.mutex);
>         /* Verify that ring has been setup correctly. */
>         for (index = 0; index < vs->dev.nvqs; ++index) {
>                 /* Verify that ring has been setup correctly. */
>                 if (!vhost_vq_access_ok(&vs->vqs[index])) {
>                         mutex_unlock(&vs->dev.mutex);
>                         return -EFAULT;
>                 }
>         }
>         mutex_unlock(&vs->dev.mutex);
> 
> This way, VHOST_SCSI_SET_ENDPOINT can simply be invoked right after
> vhost_dev_init, and likewise VHOST_SCSI_CLEAR_ENDPOINT in vhost_scsi_exit.
> 

<nod>, Ok, I'll generate a patch for this soon.

> I placed both sets of patches on two branches (vhost-scsi-nab and
> vhost-scsi) of my github repo at git://github.com/bonzini/qemu.git.
> One thing I haven't done due to lack of time is applying Nick's patches
> to a tree from last September.  If it works, we can bisect.  But this is
> pretty much all the time I can devote to vhost-scsi.  Nick/Asias, if
> you want to pick it up please do.
> 

Thanks Paolo!

--nab

> Paolo
> 
> Paolo Bonzini (4):
>   vhost: do VHOST_SET_VRING_KICK after setting up all vrings
>   vhost: add set_vhost_endpoint and clear_vhost_endpoint callbacks
>   virtio-scsi: create VirtIOSCSICommon
>   vhost-scsi: new device supporting the tcm_vhost Linux kernel module
> 
>  hw/Makefile.objs     |   5 +-
>  hw/s390-virtio-bus.c |  35 +++++++++
>  hw/vhost-scsi.c      | 188 ++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/vhost-scsi.h      |  62 ++++++++++++++++
>  hw/vhost.c           |  75 ++++++++++++-------
>  hw/virtio-pci.c      |  59 +++++++++++++++
>  hw/virtio-scsi.c     | 199 
> +++++++++++++--------------------------------------
>  hw/virtio-scsi.h     | 129 +++++++++++++++++++++++++++++++++
>  hw/virtio.h          |   2 +
>  include/qemu/osdep.h |   4 ++
>  10 files changed, 583 insertions(+), 175 deletions(-)
>  create mode 100644 hw/vhost-scsi.c
>  create mode 100644 hw/vhost-scsi.h
>