[Qemu-devel] [RFC-v2 0/6] vhost-scsi: Add support for host virtualized target

[Qemu-devel] [RFC-v2 0/6] vhost-scsi: Add support for host virtualized target

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

Hi Paolo, Stefan, & QEMU folks,

The following is the second RFC series for vhost-scsi patches against mainline
QEMU v1.1.0.  The series is available from the following working branch:

  git://git.kernel.org/pub/scm/virt/kvm/nab/qemu-kvm.git vhost-scsi-merge

Apologies for the delayed follow-up on this series.  The changes detailed below
addresses Paolo's original comments on vhost-scsi code from the last weeks.

As of this evening the tcm_vhost driver has now been merged into the mainline
kernel for 3.6-rc2 here:

  tcm_vhost: Initial merge for vhost level target fabric driver    
  http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297877

Also, after following up on the qemu-kvm IRQ injection changes (from Jan) that
caused a performance regerssion with QEMU 1.1.0 code originally reported here:

  vhost-scsi port to v1.1.0 + MSI-X performance regression
  http://comments.gmane.org/gmane.linux.scsi.target.devel/2277

It turns out that setting explict virtio-queue IRQ affinity within guest appears
to bring small block random IOPs performance back up to the pre IRQ injection
conversion levels.  I'm not sure why this ended up making so much of a difference
post IRQ injection conversion, but setting virtio-queue affinity is now getting
us back to pre IQN injection conversion levels.

Changes from v1 -> v2:

 - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as
   starting point for v3.6-rc code (Stefan + ALiguori + nab)
 - Fix upstream qemu conflict in hw/qdev-properties.c
 - Make GET_ABI_VERSION use int (nab + mst)
 - Drop unnecessary event-notifier changes (nab)
 - Fix vhost-scsi case lables in configure (reported by paolo)
 - Convert qdev_prop_vhost_scsi to use ->get() + ->set() following
   qdev_prop_netdev (reported by paolo)
 - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo)
 - Squash virtio-scsi: use the vhost-scsi host device from stefan (nab)
 - Fix up virtio_scsi_properties[] conflict w/ upstream qemu (nab)
 - Drop usage of to_virtio_scsi() in virtio_scsi_set_status()
      (reported by paolo)
 - Use modern VirtIOSCSIConf define in virtio-scsi.h (reported by paolo)
 - Use s->conf->vhost_scsi instead of proxyconf->vhost_scsi in
   virtio_scsi_init() (reported by paolo)
 - Only register QEMU SCSI bus is vhost-scsi is not active (reported by paolo)
 - Fix incorrect VirtIOSCSI->cmd_vqs[0] definition (nab)

Please have another look, and let me know if anything else needs to be
addressed.

Thanks!

--nab

Nicholas Bellinger (3):
  msix: Work-around for vhost-scsi with KVM in-kernel MSI injection
  virtio-scsi: Set max_target=0 during vhost-scsi operation
  virtio-scsi: Fix incorrect VirtIOSCSI->cmd_vqs[0] definition

Stefan Hajnoczi (3):
  vhost: Pass device path to vhost_dev_init()
  vhost-scsi: add -vhost-scsi host device for use with tcm-vhost
  virtio-scsi: Add start/stop functionality for vhost-scsi

 configure            |   10 +++
 hw/Makefile.objs     |    1 +
 hw/msix.c            |    3 +
 hw/qdev-properties.c |   40 ++++++++++++
 hw/qdev.h            |    3 +
 hw/vhost-scsi.c      |  170 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/vhost-scsi.h      |   50 +++++++++++++++
 hw/vhost.c           |    5 +-
 hw/vhost.h           |    3 +-
 hw/vhost_net.c       |    2 +-
 hw/virtio-pci.c      |    1 +
 hw/virtio-scsi.c     |   56 ++++++++++++++++-
 hw/virtio-scsi.h     |    1 +
 qemu-common.h        |    1 +
 qemu-config.c        |   16 +++++
 qemu-options.hx      |    4 +
 vl.c                 |   18 +++++
 17 files changed, 378 insertions(+), 6 deletions(-)
 create mode 100644 hw/vhost-scsi.c
 create mode 100644 hw/vhost-scsi.h

-- 
1.7.2.5

[Qemu-devel] [RFC-v2 1/6] msix: Work-around for vhost-scsi with KVM in-kernel MSI injection

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This is required to get past the following assert with:

commit 1523ed9e1d46b0b54540049d491475ccac7e6421
Author: Jan Kiszka <jan.kiszka@siemens.com>
Date:   Thu May 17 10:32:39 2012 -0300

    virtio/vhost: Add support for KVM in-kernel MSI injection

Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 hw/msix.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index 800fc32..c1e6dc3 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -544,6 +544,9 @@ void msix_unset_vector_notifiers(PCIDevice *dev)
 {
     int vector;
 
+    if (!dev->msix_vector_use_notifier && !dev->msix_vector_release_notifier)
+        return;
+
     assert(dev->msix_vector_use_notifier &&
            dev->msix_vector_release_notifier);
 
-- 
1.7.2.5

[Qemu-devel] [RFC-v2 2/6] vhost: Pass device path to vhost_dev_init()

[Nicholas A. Bellinger]

From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

The path to /dev/vhost-net is currently hardcoded in vhost_dev_init().
This needs to be changed so that /dev/vhost-scsi can be used.  Pass in
the device path instead of hardcoding it.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 hw/vhost.c     |    5 +++--
 hw/vhost.h     |    3 ++-
 hw/vhost_net.c |    2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/vhost.c b/hw/vhost.c
index 0fd8da8..d0ce5aa 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -747,14 +747,15 @@ static void vhost_eventfd_del(MemoryListener *listener,
 {
 }
 
-int vhost_dev_init(struct vhost_dev *hdev, int devfd, bool force)
+int vhost_dev_init(struct vhost_dev *hdev, int devfd, const char *devpath,
+                   bool force)
 {
     uint64_t features;
     int r;
     if (devfd >= 0) {
         hdev->control = devfd;
     } else {
-        hdev->control = open("/dev/vhost-net", O_RDWR);
+        hdev->control = open(devpath, O_RDWR);
         if (hdev->control < 0) {
             return -errno;
         }
diff --git a/hw/vhost.h b/hw/vhost.h
index 80e64df..0c47229 100644
--- a/hw/vhost.h
+++ b/hw/vhost.h
@@ -44,7 +44,8 @@ struct vhost_dev {
     bool force;
 };
 
-int vhost_dev_init(struct vhost_dev *hdev, int devfd, bool force);
+int vhost_dev_init(struct vhost_dev *hdev, int devfd, const char *devpath,
+                   bool force);
 void vhost_dev_cleanup(struct vhost_dev *hdev);
 bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev);
 int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);
diff --git a/hw/vhost_net.c b/hw/vhost_net.c
index ecaa22d..df2c4a3 100644
--- a/hw/vhost_net.c
+++ b/hw/vhost_net.c
@@ -109,7 +109,7 @@ struct vhost_net *vhost_net_init(NetClientState *backend, int devfd,
         (1 << VHOST_NET_F_VIRTIO_NET_HDR);
     net->backend = r;
 
-    r = vhost_dev_init(&net->dev, devfd, force);
+    r = vhost_dev_init(&net->dev, devfd, "/dev/vhost-net", force);
     if (r < 0) {
         goto fail;
     }
-- 
1.7.2.5

[Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost

[Nicholas A. Bellinger]

From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

This patch adds a new type of host device that drives the vhost_scsi
device.  The syntax to add vhost-scsi is:

  qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123

The virtio-scsi emulated device will make use of vhost-scsi to process
virtio-scsi requests inside the kernel and hand them to the in-kernel
SCSI target stack using the tcm_vhost fabric driver.

The tcm_vhost driver was merged into the upstream linux kernel for 3.6-rc2,
and the commit can be found here:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297

Changelog v1 -> v2:

- Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as
  starting point for v3.6-rc code (Stefan + ALiguori + nab)
- Fix upstream qemu conflict in hw/qdev-properties.c
- Make GET_ABI_VERSION use int (nab + mst)
- Fix vhost-scsi case lables in configure (reported by paolo)
- Convert qdev_prop_vhost_scsi to use ->get() + ->set() following
  qdev_prop_netdev (reported by paolo)
- Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo)

Changelog v0 -> v1:

- Add VHOST_SCSI_SET_ENDPOINT call (stefan)
- Enable vhost notifiers for multiple queues (Zhi)
- clear vhost-scsi endpoint on stopped (Zhi)
- Add CONFIG_VHOST_SCSI for QEMU build configure (nab)
- Rename vhost_vring_target -> vhost_scsi_target (mst + nab)
- Add support for VHOST_SCSI_GET_ABI_VERSION ioctl (aliguori + nab)

Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 configure            |   10 +++
 hw/Makefile.objs     |    1 +
 hw/qdev-properties.c |   40 ++++++++++++
 hw/qdev.h            |    3 +
 hw/vhost-scsi.c      |  170 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/vhost-scsi.h      |   50 +++++++++++++++
 qemu-common.h        |    1 +
 qemu-config.c        |   16 +++++
 qemu-options.hx      |    4 +
 vl.c                 |   18 +++++
 10 files changed, 313 insertions(+), 0 deletions(-)
 create mode 100644 hw/vhost-scsi.c
 create mode 100644 hw/vhost-scsi.h

diff --git a/configure b/configure
index f0dbc03..1f03202 100755
--- a/configure
+++ b/configure
@@ -168,6 +168,7 @@ libattr=""
 xfs=""
 
 vhost_net="no"
+vhost_scsi="no"
 kvm="no"
 gprof="no"
 debug_tcg="no"
@@ -513,6 +514,7 @@ Haiku)
   usb="linux"
   kvm="yes"
   vhost_net="yes"
+  vhost_scsi="yes"
   if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
     audio_possible_drivers="$audio_possible_drivers fmod"
   fi
@@ -818,6 +820,10 @@ for opt do
   ;;
   --enable-vhost-net) vhost_net="yes"
   ;;
+  --disable-vhost-scsi) vhost_scsi="no"
+  ;;
+  --enable-vhost-scsi) vhost_scsi="yes"
+  ;;
   --disable-opengl) opengl="no"
   ;;
   --enable-opengl) opengl="yes"
@@ -3116,6 +3122,7 @@ echo "posix_madvise     $posix_madvise"
 echo "uuid support      $uuid"
 echo "libcap-ng support $cap_ng"
 echo "vhost-net support $vhost_net"
+echo "vhost-scsi support $vhost_scsi"
 echo "Trace backend     $trace_backend"
 echo "Trace output file $trace_file-<pid>"
 echo "spice support     $spice"
@@ -3828,6 +3835,9 @@ case "$target_arch2" in
       if test "$vhost_net" = "yes" ; then
         echo "CONFIG_VHOST_NET=y" >> $config_target_mak
       fi
+      if test "$vhost_scsi" = "yes" ; then
+        echo "CONFIG_VHOST_SCSI=y" >> $config_target_mak
+      fi
     fi
 esac
 case "$target_arch2" in
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 3ba5dd0..6ab75ec 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -169,6 +169,7 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o
 obj-$(CONFIG_VIRTIO) += virtio-serial-bus.o virtio-scsi.o
 obj-$(CONFIG_SOFTMMU) += vhost_net.o
 obj-$(CONFIG_VHOST_NET) += vhost.o
+obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o
 obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/
 obj-$(CONFIG_NO_PCI) += pci-stub.o
 obj-$(CONFIG_VGA) += vga.o
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 8aca0d4..0266266 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -4,6 +4,7 @@
 #include "blockdev.h"
 #include "hw/block-common.h"
 #include "net/hub.h"
+#include "vhost-scsi.h"
 
 void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
 {
@@ -696,6 +697,45 @@ PropertyInfo qdev_prop_vlan = {
     .set   = set_vlan,
 };
 
+/* --- vhost-scsi --- */
+
+static int parse_vhost_scsi_dev(DeviceState *dev, const char *str, void **ptr)
+{
+   VHostSCSI *p;
+
+   p = find_vhost_scsi(str);
+   if (p == NULL)
+       return -ENOENT;
+
+   *ptr = p;
+   return 0;
+}
+
+static const char *print_vhost_scsi_dev(void *ptr)
+{
+    VHostSCSI *p = ptr;
+
+    return (p) ? vhost_scsi_get_id(p) : "<null>";
+}
+
+static void get_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
+                       const char *name, Error **errp)
+{
+    get_pointer(obj, v, opaque, print_vhost_scsi_dev, name, errp);
+}
+
+static void set_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
+                               const char *name, Error **errp)
+{
+    set_pointer(obj, v, opaque, parse_vhost_scsi_dev, name, errp);
+}
+
+PropertyInfo qdev_prop_vhost_scsi = {
+     .name = "vhost-scsi",
+     .get  = get_vhost_scsi_dev,
+     .set  = set_vhost_scsi_dev,
+};
+
 /* --- pointer --- */
 
 /* Not a proper property, just for dirty hacks.  TODO Remove it!  */
diff --git a/hw/qdev.h b/hw/qdev.h
index d699194..d5873bb 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -238,6 +238,7 @@ extern PropertyInfo qdev_prop_vlan;
 extern PropertyInfo qdev_prop_pci_devfn;
 extern PropertyInfo qdev_prop_blocksize;
 extern PropertyInfo qdev_prop_pci_host_devaddr;
+extern PropertyInfo qdev_prop_vhost_scsi;
 
 #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
         .name      = (_name),                                    \
@@ -305,6 +306,8 @@ extern PropertyInfo qdev_prop_pci_host_devaddr;
     DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
 #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
     DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
+#define DEFINE_PROP_VHOST_SCSI(_n, _s, _f)       \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_vhost_scsi, VHostSCSI*)
 
 #define DEFINE_PROP_END_OF_LIST()               \
     {}
diff --git a/hw/vhost-scsi.c b/hw/vhost-scsi.c
new file mode 100644
index 0000000..7145b2d
--- /dev/null
+++ b/hw/vhost-scsi.c
@@ -0,0 +1,170 @@
+/*
+ * vhost_scsi host device
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include <sys/ioctl.h>
+#include "config.h"
+#include "qemu-queue.h"
+#include "vhost-scsi.h"
+#include "vhost.h"
+
+struct VHostSCSI {
+    const char *id;
+    const char *wwpn;
+    uint16_t tpgt;
+    struct vhost_dev dev;
+    struct vhost_virtqueue vqs[3];
+    QLIST_ENTRY(VHostSCSI) list;
+};
+
+static QLIST_HEAD(, VHostSCSI) vhost_scsi_list =
+    QLIST_HEAD_INITIALIZER(vhost_scsi_list);
+
+VHostSCSI *find_vhost_scsi(const char *id)
+{
+    VHostSCSI *vs;
+
+    QLIST_FOREACH(vs, &vhost_scsi_list, list) {
+        if (strcmp(id, vs->id) == 0) {
+            return vs;
+        }
+    }
+    return NULL;
+}
+
+const char *vhost_scsi_get_id(VHostSCSI *vs)
+{
+    return vs->id;
+}
+
+int vhost_scsi_start(VHostSCSI *vs, VirtIODevice *vdev)
+{
+    int ret, abi_version;
+    struct vhost_scsi_target backend;
+
+    if (!vhost_dev_query(&vs->dev, vdev)) {
+        return -ENOTSUP;
+    }
+
+    vs->dev.nvqs = 3;
+    vs->dev.vqs = vs->vqs;
+
+    ret = vhost_dev_enable_notifiers(&vs->dev, vdev);
+    if (ret < 0) {
+        return ret;
+    }
+
+    ret = vhost_dev_start(&vs->dev, vdev);
+    if (ret < 0) {
+        return ret;
+    }
+
+    memset(&backend, 0, sizeof(backend));
+    ret = ioctl(vs->dev.control, VHOST_SCSI_GET_ABI_VERSION, &abi_version);
+    if (ret < 0) {
+        ret = -errno;
+        vhost_dev_stop(&vs->dev, vdev);
+        return ret;
+    }
+    if (abi_version > VHOST_SCSI_ABI_VERSION) {
+        fprintf(stderr, "The running tcm_vhost kernel abi_version: %d is greater"
+		" than vhost_scsi userspace supports: %d\n", abi_version,
+		VHOST_SCSI_ABI_VERSION);
+        ret = -ENOSYS;
+        vhost_dev_stop(&vs->dev, vdev);
+        return ret;
+    }
+    fprintf(stdout, "TCM_vHost ABI version: %d\n", abi_version);
+
+    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
+    backend.vhost_tpgt = vs->tpgt;
+    ret = ioctl(vs->dev.control, VHOST_SCSI_SET_ENDPOINT, &backend);
+    if (ret < 0) {
+        ret = -errno;
+        vhost_dev_stop(&vs->dev, vdev);
+        return ret;
+    }
+
+    return 0;
+}
+
+void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev)
+{
+    int ret;
+    struct vhost_scsi_target backend;
+
+    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
+    backend.vhost_tpgt = vs->tpgt;
+    ret = ioctl(vs->dev.control, VHOST_SCSI_CLEAR_ENDPOINT, &backend);
+    if (ret < 0) {
+        fprintf(stderr, "Failed to clear endpoint\n");
+    }
+
+    vhost_dev_stop(&vs->dev, vdev);
+}
+
+static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
+                                 uint16_t tpgt)
+{
+    VHostSCSI *vs = g_malloc0(sizeof(*vs));
+    int ret;
+
+    /* TODO set up vhost-scsi device and bind to tcm_vhost/$wwpm/tpgt_$tpgt */
+    fprintf(stderr, "wwpn = \"%s\" tpgt = \"%u\"\n", id, tpgt);
+
+    ret = vhost_dev_init(&vs->dev, -1, "/dev/vhost-scsi", false);
+    if (ret < 0) {
+        fprintf(stderr, "vhost-scsi: vhost initialization failed: %s\n",
+                strerror(-ret));
+        return NULL;
+    }
+    vs->dev.backend_features = 0;
+    vs->dev.acked_features = 0;
+
+    vs->id = g_strdup(id);
+    vs->wwpn = g_strdup(wwpn);
+    vs->tpgt = tpgt;
+    QLIST_INSERT_HEAD(&vhost_scsi_list, vs, list);
+
+    return vs;
+}
+
+VHostSCSI *vhost_scsi_add_opts(QemuOpts *opts)
+{
+    const char *id;
+    const char *wwpn;
+    uint64_t tpgt;
+
+    id = qemu_opts_id(opts);
+    if (!id) {
+        fprintf(stderr, "vhost-scsi: no id specified\n");
+        return NULL;
+    }
+    if (find_vhost_scsi(id)) {
+        fprintf(stderr, "duplicate vhost-scsi: \"%s\"\n", id);
+        return NULL;
+    }
+
+    wwpn = qemu_opt_get(opts, "wwpn");
+    if (!wwpn) {
+        fprintf(stderr, "vhost-scsi: \"%s\" missing wwpn\n", id);
+        return NULL;
+    }
+
+    tpgt = qemu_opt_get_number(opts, "tpgt", UINT64_MAX);
+    if (tpgt > UINT16_MAX) {
+        fprintf(stderr, "vhost-scsi: \"%s\" needs a 16-bit tpgt\n", id);
+        return NULL;
+    }
+
+    return vhost_scsi_add(id, wwpn, tpgt);
+}
diff --git a/hw/vhost-scsi.h b/hw/vhost-scsi.h
new file mode 100644
index 0000000..f3096dc
--- /dev/null
+++ b/hw/vhost-scsi.h
@@ -0,0 +1,50 @@
+/*
+ * vhost_scsi host device
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef VHOST_SCSI_H
+#define VHOST_SCSI_H
+
+#include "qemu-common.h"
+#include "qemu-option.h"
+
+/*
+ * Used by QEMU userspace to ensure a consistent vhost-scsi ABI.
+ *
+ * ABI Rev 0: July 2012 version starting point for v3.6-rc merge candidate +
+ *            RFC-v2 vhost-scsi userspace.  Add GET_ABI_VERSION ioctl usage
+ */
+
+#define VHOST_SCSI_ABI_VERSION	0
+
+/* TODO #include <linux/vhost.h> properly */
+/* For VHOST_SCSI_SET_ENDPOINT/VHOST_SCSI_CLEAR_ENDPOINT ioctl */
+struct vhost_scsi_target {
+    int abi_version;
+    unsigned char vhost_wwpn[224];
+    unsigned short vhost_tpgt;
+};
+
+#define VHOST_VIRTIO 0xAF
+#define VHOST_SCSI_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x40, struct vhost_scsi_target)
+#define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct vhost_scsi_target)
+#define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, struct vhost_scsi_target)
+
+VHostSCSI *find_vhost_scsi(const char *id);
+const char *vhost_scsi_get_id(VHostSCSI *vs);
+
+VHostSCSI *vhost_scsi_add_opts(QemuOpts *opts);
+
+int vhost_scsi_start(VHostSCSI *vs, VirtIODevice *vdev);
+void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev);
+
+#endif
diff --git a/qemu-common.h b/qemu-common.h
index f9deca6..ec36002 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -280,6 +280,7 @@ typedef struct EventNotifier EventNotifier;
 typedef struct VirtIODevice VirtIODevice;
 typedef struct QEMUSGList QEMUSGList;
 typedef struct SHPCDevice SHPCDevice;
+typedef struct VHostSCSI VHostSCSI;
 
 typedef uint64_t pcibus_t;
 
diff --git a/qemu-config.c b/qemu-config.c
index 5c3296b..33399ea 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -626,6 +626,21 @@ QemuOptsList qemu_boot_opts = {
     },
 };
 
+QemuOptsList qemu_vhost_scsi_opts = {
+    .name = "vhost-scsi",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_vhost_scsi_opts.head),
+    .desc = {
+        {
+            .name = "wwpn",
+            .type = QEMU_OPT_STRING,
+        }, {
+            .name = "tpgt",
+            .type = QEMU_OPT_NUMBER,
+        },
+        { /* end of list */ }
+    },
+};
+
 static QemuOptsList *vm_config_groups[32] = {
     &qemu_drive_opts,
     &qemu_chardev_opts,
@@ -641,6 +656,7 @@ static QemuOptsList *vm_config_groups[32] = {
     &qemu_machine_opts,
     &qemu_boot_opts,
     &qemu_iscsi_opts,
+    &qemu_vhost_scsi_opts,
     NULL,
 };
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 47cb5bd..4e7a03c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -565,6 +565,10 @@ possible drivers and properties, use @code{-device ?} and
 ETEXI
 
 DEFHEADING()
+DEF("vhost-scsi", HAS_ARG, QEMU_OPTION_vhost_scsi,
+    "-vhost-scsi wwpn=string0,tpgt=number0\n"
+    "                add vhost-scsi device\n",
+    QEMU_ARCH_ALL)
 
 DEFHEADING(File system options:)
 
diff --git a/vl.c b/vl.c
index 91076f0..61c8284 100644
--- a/vl.c
+++ b/vl.c
@@ -144,6 +144,7 @@ int main(int argc, char **argv)
 #include "qemu-options.h"
 #include "qmp-commands.h"
 #include "main-loop.h"
+#include "hw/vhost-scsi.h"
 #ifdef CONFIG_VIRTFS
 #include "fsdev/qemu-fsdev.h"
 #endif
@@ -1858,6 +1859,14 @@ static int fsdev_init_func(QemuOpts *opts, void *opaque)
 }
 #endif
 
+static int vhost_scsi_init_func(QemuOpts *opts, void *opaque)
+{
+    if (!vhost_scsi_add_opts(opts)) {
+        return -1;
+    }
+    return 0;
+}
+
 static int mon_init_func(QemuOpts *opts, void *opaque)
 {
     CharDriverState *chr;
@@ -2617,6 +2626,11 @@ int main(int argc, char **argv, char **envp)
                 }
                 break;
 #endif
+            case QEMU_OPTION_vhost_scsi:
+                if (!qemu_opts_parse(qemu_find_opts("vhost-scsi"), optarg, 0)) {
+                    exit(1);
+                }
+                break;
 #ifdef CONFIG_SLIRP
             case QEMU_OPTION_tftp:
                 legacy_tftp_prefix = optarg;
@@ -3337,6 +3351,10 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 #endif
+    if (qemu_opts_foreach(qemu_find_opts("vhost-scsi"),
+                          vhost_scsi_init_func, NULL, 1)) {
+        exit(1);
+    }
 
     os_daemonize();
 
-- 
1.7.2.5

[Qemu-devel] [RFC-v2 4/6] virtio-scsi: Add start/stop functionality for vhost-scsi

[Nicholas A. Bellinger]

From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

This patch starts and stops vhost as the virtio device transitions
through its status phases.  Vhost can only be started once the guest
reports its driver has successfully initialized, which means the
virtqueues have been set up by the guest.

v2: - Squash virtio-scsi: use the vhost-scsi host device from stefan (nab)
    - Fix up virtio_scsi_properties[] conflict w/ upstream qemu (nab)
    - Drop usage of to_virtio_scsi() in virtio_scsi_set_status()
      (reported by paolo)
    - Use modern VirtIOSCSIConf define in virtio-scsi.h (reported by paolo)
    - Use s->conf->vhost_scsi instead of proxyconf->vhost_scsi in
      virtio_scsi_init() (reported by paolo)
    - Only register QEMU SCSI bus is vhost-scsi is not active (reported
      by paolo)

Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 hw/virtio-pci.c  |    1 +
 hw/virtio-scsi.c |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 hw/virtio-scsi.h |    1 +
 3 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 125eded..b29fc3b 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -1036,6 +1036,7 @@ static void virtio_scsi_exit_pci(PCIDevice *pci_dev)
 }
 
 static Property virtio_scsi_properties[] = {
+    DEFINE_PROP_VHOST_SCSI("vhost-scsi", VirtIOPCIProxy, scsi.vhost_scsi),
     DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, DEV_NVECTORS_UNSPECIFIED),
     DEFINE_VIRTIO_SCSI_PROPERTIES(VirtIOPCIProxy, host_features, scsi),
diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index 5f737ac..8130956 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -13,9 +13,13 @@
  *
  */
 
+#include "qemu-common.h"
+#include "qemu-error.h"
+#include "vhost-scsi.h"
 #include "virtio-scsi.h"
 #include <hw/scsi.h>
 #include <hw/scsi-defs.h>
+#include "vhost.h"
 
 #define VIRTIO_SCSI_VQ_SIZE     128
 #define VIRTIO_SCSI_CDB_SIZE    32
@@ -147,6 +151,9 @@ typedef struct {
     VirtQueue *ctrl_vq;
     VirtQueue *event_vq;
     VirtQueue *cmd_vqs[0];
+
+    bool vhost_started;
+    VHostSCSI *vhost_scsi;
 } VirtIOSCSI;
 
 typedef struct VirtIOSCSIReq {
@@ -699,6 +706,38 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
     .load_request = virtio_scsi_load_request,
 };
 
+static bool virtio_scsi_started(VirtIOSCSI *s, uint8_t val)
+{
+    return (val & VIRTIO_CONFIG_S_DRIVER_OK) && s->vdev.vm_running;
+}
+
+static void virtio_scsi_set_status(VirtIODevice *vdev, uint8_t val)
+{
+    VirtIOSCSI *s = (VirtIOSCSI *)vdev;
+    bool start = virtio_scsi_started(s, val);
+
+    if (s->vhost_started == start) {
+        return;
+    }
+
+    if (start) {
+        int ret;
+
+        ret = vhost_scsi_start(s->vhost_scsi, vdev);
+        if (ret < 0) {
+            error_report("virtio-scsi: unable to start vhost: %s\n",
+                         strerror(-ret));
+
+            /* There is no userspace virtio-scsi fallback so exit */
+            exit(1);
+        }
+    } else {
+        vhost_scsi_stop(s->vhost_scsi, vdev);
+    }
+
+    s->vhost_started = start;
+}
+
 VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf)
 {
     VirtIOSCSI *s;
@@ -712,12 +751,17 @@ VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf)
 
     s->qdev = dev;
     s->conf = proxyconf;
+    s->vhost_started = false;
+    s->vhost_scsi = s->conf->vhost_scsi;
 
     /* TODO set up vdev function pointers */
     s->vdev.get_config = virtio_scsi_get_config;
     s->vdev.set_config = virtio_scsi_set_config;
     s->vdev.get_features = virtio_scsi_get_features;
     s->vdev.reset = virtio_scsi_reset;
+    if (s->vhost_scsi) {
+        s->vdev.set_status = virtio_scsi_set_status;
+    }
 
     s->ctrl_vq = virtio_add_queue(&s->vdev, VIRTIO_SCSI_VQ_SIZE,
                                    virtio_scsi_handle_ctrl);
@@ -743,5 +787,9 @@ void virtio_scsi_exit(VirtIODevice *vdev)
 {
     VirtIOSCSI *s = (VirtIOSCSI *)vdev;
     unregister_savevm(s->qdev, "virtio-scsi", s);
+
+    /* This will stop vhost backend if appropriate. */
+    virtio_scsi_set_status(vdev, 0);
+
     virtio_cleanup(vdev);
 }
diff --git a/hw/virtio-scsi.h b/hw/virtio-scsi.h
index 4bc889d..74e9422 100644
--- a/hw/virtio-scsi.h
+++ b/hw/virtio-scsi.h
@@ -22,6 +22,7 @@
 #define VIRTIO_ID_SCSI  8
 
 struct VirtIOSCSIConf {
+    VHostSCSI *vhost_scsi;
     uint32_t num_queues;
     uint32_t max_sectors;
     uint32_t cmd_per_lun;
-- 
1.7.2.5

[Qemu-devel] [RFC-v2 5/6] virtio-scsi: Set max_target=0 during vhost-scsi operation

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This QEMU patch sets VirtIOSCSIConfig->max_target=0 for vhost-scsi operation
to restrict virtio-scsi LLD guest scanning to max_id=0 (a single target ID
instance) when connected to individual tcm_vhost endpoints.

This ensures that virtio-scsi LLD only attempts to scan target IDs up to
VIRTIO_SCSI_MAX_TARGET when connected via virtio-scsi-raw.

Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 hw/virtio-scsi.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index 8130956..5e2ff6b 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -545,7 +545,11 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
     stl_raw(&scsiconf->sense_size, s->sense_size);
     stl_raw(&scsiconf->cdb_size, s->cdb_size);
     stl_raw(&scsiconf->max_channel, VIRTIO_SCSI_MAX_CHANNEL);
-    stl_raw(&scsiconf->max_target, VIRTIO_SCSI_MAX_TARGET);
+    if (s->vhost_scsi) {
+        stl_raw(&scsiconf->max_target, 0);
+    } else {
+        stl_raw(&scsiconf->max_target, VIRTIO_SCSI_MAX_TARGET);
+    }
     stl_raw(&scsiconf->max_lun, VIRTIO_SCSI_MAX_LUN);
 }
 
-- 
1.7.2.5

[Qemu-devel] [RFC-v2 6/6] virtio-scsi: Fix incorrect VirtIOSCSI->cmd_vqs[0] definition

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch fixes bug in the definition of VirtIOSCSI->cmd_vqs[0],
where the return of virtio_add_queue() in virtio_scsi_init() ends up
overwriting past the end of ->cmd_vqs[0].

Since virtio_scsi currently assumes a single vqs for data, this patch
simply changes ->cmd_vqs[1] to handle the single VirtQueue.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 hw/virtio-scsi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index 5e2ff6b..2c70f89 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -150,7 +150,7 @@ typedef struct {
     bool events_dropped;
     VirtQueue *ctrl_vq;
     VirtQueue *event_vq;
-    VirtQueue *cmd_vqs[0];
+    VirtQueue *cmd_vqs[1];
 
     bool vhost_started;
     VHostSCSI *vhost_scsi;
-- 
1.7.2.5

Re: [Qemu-devel] [RFC-v2 1/6] msix: Work-around for vhost-scsi with KVM in-kernel MSI injection

[Michael S. Tsirkin]

On Mon, Aug 13, 2012 at 08:35:12AM +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> This is required to get past the following assert with:
> 
> commit 1523ed9e1d46b0b54540049d491475ccac7e6421
> Author: Jan Kiszka <jan.kiszka@siemens.com>
> Date:   Thu May 17 10:32:39 2012 -0300
> 
>     virtio/vhost: Add support for KVM in-kernel MSI injection
> 
> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>

Could you please add a bit more explanation why
this happens with virtio scsi and why this is valid?

> ---
>  hw/msix.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/msix.c b/hw/msix.c
> index 800fc32..c1e6dc3 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -544,6 +544,9 @@ void msix_unset_vector_notifiers(PCIDevice *dev)
>  {
>      int vector;
>  
> +    if (!dev->msix_vector_use_notifier && !dev->msix_vector_release_notifier)
> +        return;
> +
>      assert(dev->msix_vector_use_notifier &&
>             dev->msix_vector_release_notifier);
>  
> -- 
> 1.7.2.5

Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost

[Michael S. Tsirkin]

On Mon, Aug 13, 2012 at 08:35:14AM +0000, Nicholas A. Bellinger wrote:
> From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> 
> This patch adds a new type of host device that drives the vhost_scsi
> device.  The syntax to add vhost-scsi is:
> 
>   qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123
> 
> The virtio-scsi emulated device will make use of vhost-scsi to process
> virtio-scsi requests inside the kernel and hand them to the in-kernel
> SCSI target stack using the tcm_vhost fabric driver.
> 
> The tcm_vhost driver was merged into the upstream linux kernel for 3.6-rc2,
> and the commit can be found here:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297
> 
> Changelog v1 -> v2:
> 
> - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as
>   starting point for v3.6-rc code (Stefan + ALiguori + nab)
> - Fix upstream qemu conflict in hw/qdev-properties.c
> - Make GET_ABI_VERSION use int (nab + mst)
> - Fix vhost-scsi case lables in configure (reported by paolo)
> - Convert qdev_prop_vhost_scsi to use ->get() + ->set() following
>   qdev_prop_netdev (reported by paolo)
> - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo)
> 
> Changelog v0 -> v1:
> 
> - Add VHOST_SCSI_SET_ENDPOINT call (stefan)
> - Enable vhost notifiers for multiple queues (Zhi)
> - clear vhost-scsi endpoint on stopped (Zhi)
> - Add CONFIG_VHOST_SCSI for QEMU build configure (nab)
> - Rename vhost_vring_target -> vhost_scsi_target (mst + nab)
> - Add support for VHOST_SCSI_GET_ABI_VERSION ioctl (aliguori + nab)
> 
> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>  configure            |   10 +++
>  hw/Makefile.objs     |    1 +
>  hw/qdev-properties.c |   40 ++++++++++++
>  hw/qdev.h            |    3 +
>  hw/vhost-scsi.c      |  170 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/vhost-scsi.h      |   50 +++++++++++++++
>  qemu-common.h        |    1 +
>  qemu-config.c        |   16 +++++
>  qemu-options.hx      |    4 +
>  vl.c                 |   18 +++++
>  10 files changed, 313 insertions(+), 0 deletions(-)
>  create mode 100644 hw/vhost-scsi.c
>  create mode 100644 hw/vhost-scsi.h
> 
> diff --git a/configure b/configure
> index f0dbc03..1f03202 100755
> --- a/configure
> +++ b/configure
> @@ -168,6 +168,7 @@ libattr=""
>  xfs=""
>  
>  vhost_net="no"
> +vhost_scsi="no"
>  kvm="no"
>  gprof="no"
>  debug_tcg="no"
> @@ -513,6 +514,7 @@ Haiku)
>    usb="linux"
>    kvm="yes"
>    vhost_net="yes"
> +  vhost_scsi="yes"
>    if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
>      audio_possible_drivers="$audio_possible_drivers fmod"
>    fi
> @@ -818,6 +820,10 @@ for opt do
>    ;;
>    --enable-vhost-net) vhost_net="yes"
>    ;;
> +  --disable-vhost-scsi) vhost_scsi="no"
> +  ;;
> +  --enable-vhost-scsi) vhost_scsi="yes"
> +  ;;
>    --disable-opengl) opengl="no"
>    ;;
>    --enable-opengl) opengl="yes"
> @@ -3116,6 +3122,7 @@ echo "posix_madvise     $posix_madvise"
>  echo "uuid support      $uuid"
>  echo "libcap-ng support $cap_ng"
>  echo "vhost-net support $vhost_net"
> +echo "vhost-scsi support $vhost_scsi"
>  echo "Trace backend     $trace_backend"
>  echo "Trace output file $trace_file-<pid>"
>  echo "spice support     $spice"
> @@ -3828,6 +3835,9 @@ case "$target_arch2" in
>        if test "$vhost_net" = "yes" ; then
>          echo "CONFIG_VHOST_NET=y" >> $config_target_mak
>        fi
> +      if test "$vhost_scsi" = "yes" ; then
> +        echo "CONFIG_VHOST_SCSI=y" >> $config_target_mak
> +      fi
>      fi
>  esac
>  case "$target_arch2" in
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 3ba5dd0..6ab75ec 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -169,6 +169,7 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o
>  obj-$(CONFIG_VIRTIO) += virtio-serial-bus.o virtio-scsi.o
>  obj-$(CONFIG_SOFTMMU) += vhost_net.o
>  obj-$(CONFIG_VHOST_NET) += vhost.o
> +obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o
>  obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/
>  obj-$(CONFIG_NO_PCI) += pci-stub.o
>  obj-$(CONFIG_VGA) += vga.o
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 8aca0d4..0266266 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -4,6 +4,7 @@
>  #include "blockdev.h"
>  #include "hw/block-common.h"
>  #include "net/hub.h"
> +#include "vhost-scsi.h"
>  
>  void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
>  {
> @@ -696,6 +697,45 @@ PropertyInfo qdev_prop_vlan = {
>      .set   = set_vlan,
>  };
>  
> +/* --- vhost-scsi --- */
> +
> +static int parse_vhost_scsi_dev(DeviceState *dev, const char *str, void **ptr)
> +{
> +   VHostSCSI *p;
> +
> +   p = find_vhost_scsi(str);
> +   if (p == NULL)
> +       return -ENOENT;
> +
> +   *ptr = p;
> +   return 0;
> +}
> +
> +static const char *print_vhost_scsi_dev(void *ptr)
> +{
> +    VHostSCSI *p = ptr;
> +
> +    return (p) ? vhost_scsi_get_id(p) : "<null>";
> +}
> +
> +static void get_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> +                       const char *name, Error **errp)
> +{
> +    get_pointer(obj, v, opaque, print_vhost_scsi_dev, name, errp);
> +}
> +
> +static void set_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> +                               const char *name, Error **errp)
> +{
> +    set_pointer(obj, v, opaque, parse_vhost_scsi_dev, name, errp);
> +}
> +
> +PropertyInfo qdev_prop_vhost_scsi = {
> +     .name = "vhost-scsi",
> +     .get  = get_vhost_scsi_dev,
> +     .set  = set_vhost_scsi_dev,
> +};
> +
>  /* --- pointer --- */
>  
>  /* Not a proper property, just for dirty hacks.  TODO Remove it!  */
> diff --git a/hw/qdev.h b/hw/qdev.h
> index d699194..d5873bb 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -238,6 +238,7 @@ extern PropertyInfo qdev_prop_vlan;
>  extern PropertyInfo qdev_prop_pci_devfn;
>  extern PropertyInfo qdev_prop_blocksize;
>  extern PropertyInfo qdev_prop_pci_host_devaddr;
> +extern PropertyInfo qdev_prop_vhost_scsi;
>  
>  #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
>          .name      = (_name),                                    \
> @@ -305,6 +306,8 @@ extern PropertyInfo qdev_prop_pci_host_devaddr;
>      DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
>  #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
>      DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
> +#define DEFINE_PROP_VHOST_SCSI(_n, _s, _f)       \
> +    DEFINE_PROP(_n, _s, _f, qdev_prop_vhost_scsi, VHostSCSI*)
>  
>  #define DEFINE_PROP_END_OF_LIST()               \
>      {}
> diff --git a/hw/vhost-scsi.c b/hw/vhost-scsi.c
> new file mode 100644
> index 0000000..7145b2d
> --- /dev/null
> +++ b/hw/vhost-scsi.c
> @@ -0,0 +1,170 @@
> +/*
> + * vhost_scsi host device
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include <sys/ioctl.h>
> +#include "config.h"
> +#include "qemu-queue.h"
> +#include "vhost-scsi.h"
> +#include "vhost.h"
> +
> +struct VHostSCSI {
> +    const char *id;
> +    const char *wwpn;
> +    uint16_t tpgt;
> +    struct vhost_dev dev;
> +    struct vhost_virtqueue vqs[3];
> +    QLIST_ENTRY(VHostSCSI) list;
> +};
> +
> +static QLIST_HEAD(, VHostSCSI) vhost_scsi_list =
> +    QLIST_HEAD_INITIALIZER(vhost_scsi_list);
> +
> +VHostSCSI *find_vhost_scsi(const char *id)
> +{
> +    VHostSCSI *vs;
> +
> +    QLIST_FOREACH(vs, &vhost_scsi_list, list) {
> +        if (strcmp(id, vs->id) == 0) {
> +            return vs;
> +        }
> +    }
> +    return NULL;
> +}
> +
> +const char *vhost_scsi_get_id(VHostSCSI *vs)
> +{
> +    return vs->id;
> +}
> +
> +int vhost_scsi_start(VHostSCSI *vs, VirtIODevice *vdev)
> +{
> +    int ret, abi_version;
> +    struct vhost_scsi_target backend;
> +
> +    if (!vhost_dev_query(&vs->dev, vdev)) {
> +        return -ENOTSUP;
> +    }
> +
> +    vs->dev.nvqs = 3;
> +    vs->dev.vqs = vs->vqs;
> +
> +    ret = vhost_dev_enable_notifiers(&vs->dev, vdev);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    ret = vhost_dev_start(&vs->dev, vdev);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    memset(&backend, 0, sizeof(backend));
> +    ret = ioctl(vs->dev.control, VHOST_SCSI_GET_ABI_VERSION, &abi_version);
> +    if (ret < 0) {
> +        ret = -errno;
> +        vhost_dev_stop(&vs->dev, vdev);
> +        return ret;
> +    }
> +    if (abi_version > VHOST_SCSI_ABI_VERSION) {
> +        fprintf(stderr, "The running tcm_vhost kernel abi_version: %d is greater"
> +		" than vhost_scsi userspace supports: %d\n", abi_version,
> +		VHOST_SCSI_ABI_VERSION);
> +        ret = -ENOSYS;
> +        vhost_dev_stop(&vs->dev, vdev);
> +        return ret;
> +    }
> +    fprintf(stdout, "TCM_vHost ABI version: %d\n", abi_version);
> +
> +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
> +    backend.vhost_tpgt = vs->tpgt;
> +    ret = ioctl(vs->dev.control, VHOST_SCSI_SET_ENDPOINT, &backend);
> +    if (ret < 0) {
> +        ret = -errno;
> +        vhost_dev_stop(&vs->dev, vdev);
> +        return ret;
> +    }
> +
> +    return 0;
> +}
> +
> +void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev)
> +{
> +    int ret;
> +    struct vhost_scsi_target backend;
> +
> +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
> +    backend.vhost_tpgt = vs->tpgt;
> +    ret = ioctl(vs->dev.control, VHOST_SCSI_CLEAR_ENDPOINT, &backend);
> +    if (ret < 0) {
> +        fprintf(stderr, "Failed to clear endpoint\n");
> +    }
> +
> +    vhost_dev_stop(&vs->dev, vdev);
> +}
> +
> +static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
> +                                 uint16_t tpgt)
> +{
> +    VHostSCSI *vs = g_malloc0(sizeof(*vs));
> +    int ret;
> +
> +    /* TODO set up vhost-scsi device and bind to tcm_vhost/$wwpm/tpgt_$tpgt */
> +    fprintf(stderr, "wwpn = \"%s\" tpgt = \"%u\"\n", id, tpgt);
> +
> +    ret = vhost_dev_init(&vs->dev, -1, "/dev/vhost-scsi", false);

This -1 is a hack. You need to support passing in fd from
the monitor, and pass it here.

> +    if (ret < 0) {
> +        fprintf(stderr, "vhost-scsi: vhost initialization failed: %s\n",
> +                strerror(-ret));
> +        return NULL;
> +    }
> +    vs->dev.backend_features = 0;
> +    vs->dev.acked_features = 0;
> +
> +    vs->id = g_strdup(id);
> +    vs->wwpn = g_strdup(wwpn);
> +    vs->tpgt = tpgt;
> +    QLIST_INSERT_HEAD(&vhost_scsi_list, vs, list);
> +
> +    return vs;
> +}
> +
> +VHostSCSI *vhost_scsi_add_opts(QemuOpts *opts)
> +{
> +    const char *id;
> +    const char *wwpn;
> +    uint64_t tpgt;
> +
> +    id = qemu_opts_id(opts);
> +    if (!id) {
> +        fprintf(stderr, "vhost-scsi: no id specified\n");
> +        return NULL;
> +    }
> +    if (find_vhost_scsi(id)) {
> +        fprintf(stderr, "duplicate vhost-scsi: \"%s\"\n", id);
> +        return NULL;
> +    }
> +
> +    wwpn = qemu_opt_get(opts, "wwpn");
> +    if (!wwpn) {
> +        fprintf(stderr, "vhost-scsi: \"%s\" missing wwpn\n", id);
> +        return NULL;
> +    }
> +
> +    tpgt = qemu_opt_get_number(opts, "tpgt", UINT64_MAX);
> +    if (tpgt > UINT16_MAX) {
> +        fprintf(stderr, "vhost-scsi: \"%s\" needs a 16-bit tpgt\n", id);
> +        return NULL;
> +    }
> +
> +    return vhost_scsi_add(id, wwpn, tpgt);
> +}
> diff --git a/hw/vhost-scsi.h b/hw/vhost-scsi.h
> new file mode 100644
> index 0000000..f3096dc
> --- /dev/null
> +++ b/hw/vhost-scsi.h
> @@ -0,0 +1,50 @@
> +/*
> + * vhost_scsi host device
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef VHOST_SCSI_H
> +#define VHOST_SCSI_H
> +
> +#include "qemu-common.h"
> +#include "qemu-option.h"
> +
> +/*
> + * Used by QEMU userspace to ensure a consistent vhost-scsi ABI.
> + *
> + * ABI Rev 0: July 2012 version starting point for v3.6-rc merge candidate +
> + *            RFC-v2 vhost-scsi userspace.  Add GET_ABI_VERSION ioctl usage
> + */
> +
> +#define VHOST_SCSI_ABI_VERSION	0
> +
> +/* TODO #include <linux/vhost.h> properly */
> +/* For VHOST_SCSI_SET_ENDPOINT/VHOST_SCSI_CLEAR_ENDPOINT ioctl */
> +struct vhost_scsi_target {
> +    int abi_version;
> +    unsigned char vhost_wwpn[224];
> +    unsigned short vhost_tpgt;
> +};
> +
> +#define VHOST_VIRTIO 0xAF
> +#define VHOST_SCSI_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x40, struct vhost_scsi_target)
> +#define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct vhost_scsi_target)
> +#define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, struct vhost_scsi_target)
> +
> +VHostSCSI *find_vhost_scsi(const char *id);
> +const char *vhost_scsi_get_id(VHostSCSI *vs);
> +
> +VHostSCSI *vhost_scsi_add_opts(QemuOpts *opts);
> +
> +int vhost_scsi_start(VHostSCSI *vs, VirtIODevice *vdev);
> +void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev);
> +
> +#endif
> diff --git a/qemu-common.h b/qemu-common.h
> index f9deca6..ec36002 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -280,6 +280,7 @@ typedef struct EventNotifier EventNotifier;
>  typedef struct VirtIODevice VirtIODevice;
>  typedef struct QEMUSGList QEMUSGList;
>  typedef struct SHPCDevice SHPCDevice;
> +typedef struct VHostSCSI VHostSCSI;
>  
>  typedef uint64_t pcibus_t;
>  
> diff --git a/qemu-config.c b/qemu-config.c
> index 5c3296b..33399ea 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -626,6 +626,21 @@ QemuOptsList qemu_boot_opts = {
>      },
>  };
>  
> +QemuOptsList qemu_vhost_scsi_opts = {
> +    .name = "vhost-scsi",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_vhost_scsi_opts.head),
> +    .desc = {
> +        {
> +            .name = "wwpn",
> +            .type = QEMU_OPT_STRING,
> +        }, {
> +            .name = "tpgt",
> +            .type = QEMU_OPT_NUMBER,
> +        },
> +        { /* end of list */ }
> +    },
> +};
> +
>  static QemuOptsList *vm_config_groups[32] = {
>      &qemu_drive_opts,
>      &qemu_chardev_opts,
> @@ -641,6 +656,7 @@ static QemuOptsList *vm_config_groups[32] = {
>      &qemu_machine_opts,
>      &qemu_boot_opts,
>      &qemu_iscsi_opts,
> +    &qemu_vhost_scsi_opts,
>      NULL,
>  };
>  
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 47cb5bd..4e7a03c 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -565,6 +565,10 @@ possible drivers and properties, use @code{-device ?} and
>  ETEXI
>  
>  DEFHEADING()
> +DEF("vhost-scsi", HAS_ARG, QEMU_OPTION_vhost_scsi,
> +    "-vhost-scsi wwpn=string0,tpgt=number0\n"
> +    "                add vhost-scsi device\n",
> +    QEMU_ARCH_ALL)
>  
>  DEFHEADING(File system options:)
>  
> diff --git a/vl.c b/vl.c
> index 91076f0..61c8284 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -144,6 +144,7 @@ int main(int argc, char **argv)
>  #include "qemu-options.h"
>  #include "qmp-commands.h"
>  #include "main-loop.h"
> +#include "hw/vhost-scsi.h"
>  #ifdef CONFIG_VIRTFS
>  #include "fsdev/qemu-fsdev.h"
>  #endif
> @@ -1858,6 +1859,14 @@ static int fsdev_init_func(QemuOpts *opts, void *opaque)
>  }
>  #endif
>  
> +static int vhost_scsi_init_func(QemuOpts *opts, void *opaque)
> +{
> +    if (!vhost_scsi_add_opts(opts)) {
> +        return -1;
> +    }
> +    return 0;
> +}
> +
>  static int mon_init_func(QemuOpts *opts, void *opaque)
>  {
>      CharDriverState *chr;
> @@ -2617,6 +2626,11 @@ int main(int argc, char **argv, char **envp)
>                  }
>                  break;
>  #endif
> +            case QEMU_OPTION_vhost_scsi:
> +                if (!qemu_opts_parse(qemu_find_opts("vhost-scsi"), optarg, 0)) {
> +                    exit(1);
> +                }
> +                break;
>  #ifdef CONFIG_SLIRP
>              case QEMU_OPTION_tftp:
>                  legacy_tftp_prefix = optarg;
> @@ -3337,6 +3351,10 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  #endif
> +    if (qemu_opts_foreach(qemu_find_opts("vhost-scsi"),
> +                          vhost_scsi_init_func, NULL, 1)) {
> +        exit(1);
> +    }
>  
>      os_daemonize();
>  
> -- 
> 1.7.2.5

Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost

[Michael S. Tsirkin]

On Mon, Aug 13, 2012 at 08:35:14AM +0000, Nicholas A. Bellinger wrote:
> From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> 
> This patch adds a new type of host device that drives the vhost_scsi
> device.  The syntax to add vhost-scsi is:
> 
>   qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123
> 
> The virtio-scsi emulated device will make use of vhost-scsi to process
> virtio-scsi requests inside the kernel and hand them to the in-kernel
> SCSI target stack using the tcm_vhost fabric driver.
> 
> The tcm_vhost driver was merged into the upstream linux kernel for 3.6-rc2,
> and the commit can be found here:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297
> 
> Changelog v1 -> v2:
> 
> - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as
>   starting point for v3.6-rc code (Stefan + ALiguori + nab)
> - Fix upstream qemu conflict in hw/qdev-properties.c
> - Make GET_ABI_VERSION use int (nab + mst)
> - Fix vhost-scsi case lables in configure (reported by paolo)
> - Convert qdev_prop_vhost_scsi to use ->get() + ->set() following
>   qdev_prop_netdev (reported by paolo)
> - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo)
> 
> Changelog v0 -> v1:
> 
> - Add VHOST_SCSI_SET_ENDPOINT call (stefan)
> - Enable vhost notifiers for multiple queues (Zhi)
> - clear vhost-scsi endpoint on stopped (Zhi)
> - Add CONFIG_VHOST_SCSI for QEMU build configure (nab)
> - Rename vhost_vring_target -> vhost_scsi_target (mst + nab)
> - Add support for VHOST_SCSI_GET_ABI_VERSION ioctl (aliguori + nab)
> 
> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>


Sent mail too fast, sorry. More comments below.

> ---
>  configure            |   10 +++
>  hw/Makefile.objs     |    1 +
>  hw/qdev-properties.c |   40 ++++++++++++
>  hw/qdev.h            |    3 +
>  hw/vhost-scsi.c      |  170 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/vhost-scsi.h      |   50 +++++++++++++++
>  qemu-common.h        |    1 +
>  qemu-config.c        |   16 +++++
>  qemu-options.hx      |    4 +
>  vl.c                 |   18 +++++
>  10 files changed, 313 insertions(+), 0 deletions(-)
>  create mode 100644 hw/vhost-scsi.c
>  create mode 100644 hw/vhost-scsi.h
> 
> diff --git a/configure b/configure
> index f0dbc03..1f03202 100755
> --- a/configure
> +++ b/configure
> @@ -168,6 +168,7 @@ libattr=""
>  xfs=""
>  
>  vhost_net="no"
> +vhost_scsi="no"
>  kvm="no"
>  gprof="no"
>  debug_tcg="no"
> @@ -513,6 +514,7 @@ Haiku)
>    usb="linux"
>    kvm="yes"
>    vhost_net="yes"
> +  vhost_scsi="yes"
>    if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
>      audio_possible_drivers="$audio_possible_drivers fmod"
>    fi
> @@ -818,6 +820,10 @@ for opt do
>    ;;
>    --enable-vhost-net) vhost_net="yes"
>    ;;
> +  --disable-vhost-scsi) vhost_scsi="no"
> +  ;;
> +  --enable-vhost-scsi) vhost_scsi="yes"
> +  ;;
>    --disable-opengl) opengl="no"
>    ;;
>    --enable-opengl) opengl="yes"
> @@ -3116,6 +3122,7 @@ echo "posix_madvise     $posix_madvise"
>  echo "uuid support      $uuid"
>  echo "libcap-ng support $cap_ng"
>  echo "vhost-net support $vhost_net"
> +echo "vhost-scsi support $vhost_scsi"
>  echo "Trace backend     $trace_backend"
>  echo "Trace output file $trace_file-<pid>"
>  echo "spice support     $spice"
> @@ -3828,6 +3835,9 @@ case "$target_arch2" in
>        if test "$vhost_net" = "yes" ; then
>          echo "CONFIG_VHOST_NET=y" >> $config_target_mak
>        fi
> +      if test "$vhost_scsi" = "yes" ; then
> +        echo "CONFIG_VHOST_SCSI=y" >> $config_target_mak
> +      fi
>      fi
>  esac
>  case "$target_arch2" in
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 3ba5dd0..6ab75ec 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -169,6 +169,7 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o
>  obj-$(CONFIG_VIRTIO) += virtio-serial-bus.o virtio-scsi.o
>  obj-$(CONFIG_SOFTMMU) += vhost_net.o
>  obj-$(CONFIG_VHOST_NET) += vhost.o
> +obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o
>  obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/
>  obj-$(CONFIG_NO_PCI) += pci-stub.o
>  obj-$(CONFIG_VGA) += vga.o
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 8aca0d4..0266266 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -4,6 +4,7 @@
>  #include "blockdev.h"
>  #include "hw/block-common.h"
>  #include "net/hub.h"
> +#include "vhost-scsi.h"
>  
>  void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
>  {
> @@ -696,6 +697,45 @@ PropertyInfo qdev_prop_vlan = {
>      .set   = set_vlan,
>  };
>  
> +/* --- vhost-scsi --- */
> +
> +static int parse_vhost_scsi_dev(DeviceState *dev, const char *str, void **ptr)
> +{
> +   VHostSCSI *p;
> +
> +   p = find_vhost_scsi(str);
> +   if (p == NULL)
> +       return -ENOENT;
> +
> +   *ptr = p;
> +   return 0;
> +}
> +
> +static const char *print_vhost_scsi_dev(void *ptr)
> +{
> +    VHostSCSI *p = ptr;
> +
> +    return (p) ? vhost_scsi_get_id(p) : "<null>";
> +}
> +
> +static void get_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> +                       const char *name, Error **errp)
> +{
> +    get_pointer(obj, v, opaque, print_vhost_scsi_dev, name, errp);
> +}
> +
> +static void set_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> +                               const char *name, Error **errp)
> +{
> +    set_pointer(obj, v, opaque, parse_vhost_scsi_dev, name, errp);
> +}
> +
> +PropertyInfo qdev_prop_vhost_scsi = {
> +     .name = "vhost-scsi",
> +     .get  = get_vhost_scsi_dev,
> +     .set  = set_vhost_scsi_dev,
> +};
> +
>  /* --- pointer --- */
>  
>  /* Not a proper property, just for dirty hacks.  TODO Remove it!  */

Why does this make sense in the generic qdev-properties?
There's exactly one device that can use this, no?

> diff --git a/hw/qdev.h b/hw/qdev.h
> index d699194..d5873bb 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -238,6 +238,7 @@ extern PropertyInfo qdev_prop_vlan;
>  extern PropertyInfo qdev_prop_pci_devfn;
>  extern PropertyInfo qdev_prop_blocksize;
>  extern PropertyInfo qdev_prop_pci_host_devaddr;
> +extern PropertyInfo qdev_prop_vhost_scsi;
>  
>  #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
>          .name      = (_name),                                    \
> @@ -305,6 +306,8 @@ extern PropertyInfo qdev_prop_pci_host_devaddr;
>      DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
>  #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
>      DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
> +#define DEFINE_PROP_VHOST_SCSI(_n, _s, _f)       \
> +    DEFINE_PROP(_n, _s, _f, qdev_prop_vhost_scsi, VHostSCSI*)
>

Can this move to vhost-scsi.c?
  
>  #define DEFINE_PROP_END_OF_LIST()               \
>      {}
> diff --git a/hw/vhost-scsi.c b/hw/vhost-scsi.c
> new file mode 100644
> index 0000000..7145b2d
> --- /dev/null
> +++ b/hw/vhost-scsi.c
> @@ -0,0 +1,170 @@
> +/*
> + * vhost_scsi host device
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include <sys/ioctl.h>
> +#include "config.h"
> +#include "qemu-queue.h"
> +#include "vhost-scsi.h"
> +#include "vhost.h"
> +
> +struct VHostSCSI {
> +    const char *id;
> +    const char *wwpn;
> +    uint16_t tpgt;
> +    struct vhost_dev dev;
> +    struct vhost_virtqueue vqs[3];

Could you add enum for vq numbers pls?

> +    QLIST_ENTRY(VHostSCSI) list;
> +};
> +
> +static QLIST_HEAD(, VHostSCSI) vhost_scsi_list =
> +    QLIST_HEAD_INITIALIZER(vhost_scsi_list);
> +
> +VHostSCSI *find_vhost_scsi(const char *id)
> +{
> +    VHostSCSI *vs;
> +
> +    QLIST_FOREACH(vs, &vhost_scsi_list, list) {
> +        if (strcmp(id, vs->id) == 0) {

!strcmp

> +            return vs;
> +        }
> +    }
> +    return NULL;
> +}
> +
> +const char *vhost_scsi_get_id(VHostSCSI *vs)
> +{
> +    return vs->id;
> +}
> +
> +int vhost_scsi_start(VHostSCSI *vs, VirtIODevice *vdev)
> +{
> +    int ret, abi_version;
> +    struct vhost_scsi_target backend;
> +
> +    if (!vhost_dev_query(&vs->dev, vdev)) {
> +        return -ENOTSUP;
> +    }
> +
> +    vs->dev.nvqs = 3;
> +    vs->dev.vqs = vs->vqs;
> +
> +    ret = vhost_dev_enable_notifiers(&vs->dev, vdev);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    ret = vhost_dev_start(&vs->dev, vdev);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    memset(&backend, 0, sizeof(backend));
> +    ret = ioctl(vs->dev.control, VHOST_SCSI_GET_ABI_VERSION, &abi_version);
> +    if (ret < 0) {
> +        ret = -errno;
> +        vhost_dev_stop(&vs->dev, vdev);
> +        return ret;
> +    }
> +    if (abi_version > VHOST_SCSI_ABI_VERSION) {
> +        fprintf(stderr, "The running tcm_vhost kernel abi_version: %d is greater"
> +		" than vhost_scsi userspace supports: %d\n", abi_version,
> +		VHOST_SCSI_ABI_VERSION);
> +        ret = -ENOSYS;
> +        vhost_dev_stop(&vs->dev, vdev);
> +        return ret;
> +    }
> +    fprintf(stdout, "TCM_vHost ABI version: %d\n", abi_version);
> +
> +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
> +    backend.vhost_tpgt = vs->tpgt;
> +    ret = ioctl(vs->dev.control, VHOST_SCSI_SET_ENDPOINT, &backend);
> +    if (ret < 0) {
> +        ret = -errno;
> +        vhost_dev_stop(&vs->dev, vdev);
> +        return ret;
> +    }
> +
> +    return 0;
> +}
> +
> +void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev)
> +{
> +    int ret;
> +    struct vhost_scsi_target backend;
> +
> +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
> +    backend.vhost_tpgt = vs->tpgt;
> +    ret = ioctl(vs->dev.control, VHOST_SCSI_CLEAR_ENDPOINT, &backend);
> +    if (ret < 0) {
> +        fprintf(stderr, "Failed to clear endpoint\n");
> +    }
> +
> +    vhost_dev_stop(&vs->dev, vdev);
> +}
> +
> +static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
> +                                 uint16_t tpgt)
> +{
> +    VHostSCSI *vs = g_malloc0(sizeof(*vs));
> +    int ret;
> +
> +    /* TODO set up vhost-scsi device and bind to tcm_vhost/$wwpm/tpgt_$tpgt */
> +    fprintf(stderr, "wwpn = \"%s\" tpgt = \"%u\"\n", id, tpgt);
> +

Please do not keep debugging fprintfs around.

> +    ret = vhost_dev_init(&vs->dev, -1, "/dev/vhost-scsi", false);

commented on this separately

> +    if (ret < 0) {
> +        fprintf(stderr, "vhost-scsi: vhost initialization failed: %s\n",
> +                strerror(-ret));

errors should go to monitor, here and elsewhere.

> +        return NULL;
> +    }
> +    vs->dev.backend_features = 0;
> +    vs->dev.acked_features = 0;
> +
> +    vs->id = g_strdup(id);
> +    vs->wwpn = g_strdup(wwpn);
> +    vs->tpgt = tpgt;
> +    QLIST_INSERT_HEAD(&vhost_scsi_list, vs, list);
> +
> +    return vs;
> +}
> +
> +VHostSCSI *vhost_scsi_add_opts(QemuOpts *opts)
> +{
> +    const char *id;
> +    const char *wwpn;
> +    uint64_t tpgt;
> +
> +    id = qemu_opts_id(opts);
> +    if (!id) {
> +        fprintf(stderr, "vhost-scsi: no id specified\n");
> +        return NULL;
> +    }
> +    if (find_vhost_scsi(id)) {
> +        fprintf(stderr, "duplicate vhost-scsi: \"%s\"\n", id);
> +        return NULL;
> +    }
> +
> +    wwpn = qemu_opt_get(opts, "wwpn");
> +    if (!wwpn) {
> +        fprintf(stderr, "vhost-scsi: \"%s\" missing wwpn\n", id);
> +        return NULL;
> +    }
> +
> +    tpgt = qemu_opt_get_number(opts, "tpgt", UINT64_MAX);
> +    if (tpgt > UINT16_MAX) {
> +        fprintf(stderr, "vhost-scsi: \"%s\" needs a 16-bit tpgt\n", id);
> +        return NULL;
> +    }
> +
> +    return vhost_scsi_add(id, wwpn, tpgt);
> +}
> diff --git a/hw/vhost-scsi.h b/hw/vhost-scsi.h
> new file mode 100644
> index 0000000..f3096dc
> --- /dev/null
> +++ b/hw/vhost-scsi.h
> @@ -0,0 +1,50 @@
> +/*
> + * vhost_scsi host device
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef VHOST_SCSI_H
> +#define VHOST_SCSI_H
> +
> +#include "qemu-common.h"
> +#include "qemu-option.h"
> +
> +/*
> + * Used by QEMU userspace to ensure a consistent vhost-scsi ABI.
> + *
> + * ABI Rev 0: July 2012 version starting point for v3.6-rc merge candidate +
> + *            RFC-v2 vhost-scsi userspace.  Add GET_ABI_VERSION ioctl usage
> + */
> +
> +#define VHOST_SCSI_ABI_VERSION	0
> +
> +/* TODO #include <linux/vhost.h> properly */
> +/* For VHOST_SCSI_SET_ENDPOINT/VHOST_SCSI_CLEAR_ENDPOINT ioctl */
> +struct vhost_scsi_target {
> +    int abi_version;
> +    unsigned char vhost_wwpn[224];
> +    unsigned short vhost_tpgt;
> +};
> +
> +#define VHOST_VIRTIO 0xAF
> +#define VHOST_SCSI_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x40, struct vhost_scsi_target)
> +#define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct vhost_scsi_target)
> +#define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, struct vhost_scsi_target)
> +
> +VHostSCSI *find_vhost_scsi(const char *id);
> +const char *vhost_scsi_get_id(VHostSCSI *vs);
> +
> +VHostSCSI *vhost_scsi_add_opts(QemuOpts *opts);
> +
> +int vhost_scsi_start(VHostSCSI *vs, VirtIODevice *vdev);
> +void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev);
> +
> +#endif
> diff --git a/qemu-common.h b/qemu-common.h
> index f9deca6..ec36002 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -280,6 +280,7 @@ typedef struct EventNotifier EventNotifier;
>  typedef struct VirtIODevice VirtIODevice;
>  typedef struct QEMUSGList QEMUSGList;
>  typedef struct SHPCDevice SHPCDevice;
> +typedef struct VHostSCSI VHostSCSI;
>  
>  typedef uint64_t pcibus_t;
>  
> diff --git a/qemu-config.c b/qemu-config.c
> index 5c3296b..33399ea 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -626,6 +626,21 @@ QemuOptsList qemu_boot_opts = {
>      },
>  };
>  
> +QemuOptsList qemu_vhost_scsi_opts = {
> +    .name = "vhost-scsi",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_vhost_scsi_opts.head),
> +    .desc = {
> +        {
> +            .name = "wwpn",
> +            .type = QEMU_OPT_STRING,
> +        }, {
> +            .name = "tpgt",
> +            .type = QEMU_OPT_NUMBER,
> +        },
> +        { /* end of list */ }
> +    },
> +};
> +
>  static QemuOptsList *vm_config_groups[32] = {
>      &qemu_drive_opts,
>      &qemu_chardev_opts,
> @@ -641,6 +656,7 @@ static QemuOptsList *vm_config_groups[32] = {
>      &qemu_machine_opts,
>      &qemu_boot_opts,
>      &qemu_iscsi_opts,
> +    &qemu_vhost_scsi_opts,
>      NULL,
>  };
>  
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 47cb5bd..4e7a03c 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -565,6 +565,10 @@ possible drivers and properties, use @code{-device ?} and
>  ETEXI
>  
>  DEFHEADING()
> +DEF("vhost-scsi", HAS_ARG, QEMU_OPTION_vhost_scsi,
> +    "-vhost-scsi wwpn=string0,tpgt=number0\n"
> +    "                add vhost-scsi device\n",
> +    QEMU_ARCH_ALL)
>  
>  DEFHEADING(File system options:)
>  
> diff --git a/vl.c b/vl.c
> index 91076f0..61c8284 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -144,6 +144,7 @@ int main(int argc, char **argv)
>  #include "qemu-options.h"
>  #include "qmp-commands.h"
>  #include "main-loop.h"
> +#include "hw/vhost-scsi.h"
>  #ifdef CONFIG_VIRTFS
>  #include "fsdev/qemu-fsdev.h"
>  #endif
> @@ -1858,6 +1859,14 @@ static int fsdev_init_func(QemuOpts *opts, void *opaque)
>  }
>  #endif
>  
> +static int vhost_scsi_init_func(QemuOpts *opts, void *opaque)
> +{
> +    if (!vhost_scsi_add_opts(opts)) {
> +        return -1;
> +    }
> +    return 0;
> +}
> +
>  static int mon_init_func(QemuOpts *opts, void *opaque)
>  {
>      CharDriverState *chr;
> @@ -2617,6 +2626,11 @@ int main(int argc, char **argv, char **envp)
>                  }
>                  break;
>  #endif
> +            case QEMU_OPTION_vhost_scsi:
> +                if (!qemu_opts_parse(qemu_find_opts("vhost-scsi"), optarg, 0)) {
> +                    exit(1);
> +                }
> +                break;
>  #ifdef CONFIG_SLIRP
>              case QEMU_OPTION_tftp:
>                  legacy_tftp_prefix = optarg;
> @@ -3337,6 +3351,10 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  #endif
> +    if (qemu_opts_foreach(qemu_find_opts("vhost-scsi"),
> +                          vhost_scsi_init_func, NULL, 1)) {
> +        exit(1);
> +    }
>  
>      os_daemonize();
>  
> -- 
> 1.7.2.5

Re: [Qemu-devel] [RFC-v2 6/6] virtio-scsi: Fix incorrect VirtIOSCSI->cmd_vqs[0] definition

[Michael S. Tsirkin]

On Mon, Aug 13, 2012 at 08:35:17AM +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> This patch fixes bug in the definition of VirtIOSCSI->cmd_vqs[0],
> where the return of virtio_add_queue() in virtio_scsi_init() ends up
> overwriting past the end of ->cmd_vqs[0].
> 
> Since virtio_scsi currently assumes a single vqs for data, this patch
> simply changes ->cmd_vqs[1] to handle the single VirtQueue.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>

This is a bugfix we need even without vhost, right?

> ---
>  hw/virtio-scsi.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
> index 5e2ff6b..2c70f89 100644
> --- a/hw/virtio-scsi.c
> +++ b/hw/virtio-scsi.c
> @@ -150,7 +150,7 @@ typedef struct {
>      bool events_dropped;
>      VirtQueue *ctrl_vq;
>      VirtQueue *event_vq;
> -    VirtQueue *cmd_vqs[0];
> +    VirtQueue *cmd_vqs[1];
>  
>      bool vhost_started;
>      VHostSCSI *vhost_scsi;
> -- 
> 1.7.2.5

Re: [Qemu-devel] [RFC-v2 0/6] vhost-scsi: Add support for host virtualized target

[Michael S. Tsirkin]

On Mon, Aug 13, 2012 at 08:35:11AM +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> Hi Paolo, Stefan, & QEMU folks,
> 
> The following is the second RFC series for vhost-scsi patches against mainline
> QEMU v1.1.0.  The series is available from the following working branch:
> 
>   git://git.kernel.org/pub/scm/virt/kvm/nab/qemu-kvm.git vhost-scsi-merge
> 
> Apologies for the delayed follow-up on this series.  The changes detailed below
> addresses Paolo's original comments on vhost-scsi code from the last weeks.

Looks good overall. I sent some comments on list.
Thanks!

> As of this evening the tcm_vhost driver has now been merged into the mainline
> kernel for 3.6-rc2 here:
> 
>   tcm_vhost: Initial merge for vhost level target fabric driver    
>   http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297877
> 
> Also, after following up on the qemu-kvm IRQ injection changes (from Jan) that
> caused a performance regerssion with QEMU 1.1.0 code originally reported here:
> 
>   vhost-scsi port to v1.1.0 + MSI-X performance regression
>   http://comments.gmane.org/gmane.linux.scsi.target.devel/2277
> 
> It turns out that setting explict virtio-queue IRQ affinity within guest appears
> to bring small block random IOPs performance back up to the pre IRQ injection
> conversion levels.  I'm not sure why this ended up making so much of a difference
> post IRQ injection conversion, but setting virtio-queue affinity is now getting
> us back to pre IQN injection conversion levels.
> 
> Changes from v1 -> v2:
> 
>  - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as
>    starting point for v3.6-rc code (Stefan + ALiguori + nab)
>  - Fix upstream qemu conflict in hw/qdev-properties.c
>  - Make GET_ABI_VERSION use int (nab + mst)
>  - Drop unnecessary event-notifier changes (nab)
>  - Fix vhost-scsi case lables in configure (reported by paolo)
>  - Convert qdev_prop_vhost_scsi to use ->get() + ->set() following
>    qdev_prop_netdev (reported by paolo)
>  - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo)
>  - Squash virtio-scsi: use the vhost-scsi host device from stefan (nab)
>  - Fix up virtio_scsi_properties[] conflict w/ upstream qemu (nab)
>  - Drop usage of to_virtio_scsi() in virtio_scsi_set_status()
>       (reported by paolo)
>  - Use modern VirtIOSCSIConf define in virtio-scsi.h (reported by paolo)
>  - Use s->conf->vhost_scsi instead of proxyconf->vhost_scsi in
>    virtio_scsi_init() (reported by paolo)
>  - Only register QEMU SCSI bus is vhost-scsi is not active (reported by paolo)
>  - Fix incorrect VirtIOSCSI->cmd_vqs[0] definition (nab)
> 
> Please have another look, and let me know if anything else needs to be
> addressed.
> 
> Thanks!
> 
> --nab
> 
> Nicholas Bellinger (3):
>   msix: Work-around for vhost-scsi with KVM in-kernel MSI injection
>   virtio-scsi: Set max_target=0 during vhost-scsi operation
>   virtio-scsi: Fix incorrect VirtIOSCSI->cmd_vqs[0] definition
> 
> Stefan Hajnoczi (3):
>   vhost: Pass device path to vhost_dev_init()
>   vhost-scsi: add -vhost-scsi host device for use with tcm-vhost
>   virtio-scsi: Add start/stop functionality for vhost-scsi
> 
>  configure            |   10 +++
>  hw/Makefile.objs     |    1 +
>  hw/msix.c            |    3 +
>  hw/qdev-properties.c |   40 ++++++++++++
>  hw/qdev.h            |    3 +
>  hw/vhost-scsi.c      |  170 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/vhost-scsi.h      |   50 +++++++++++++++
>  hw/vhost.c           |    5 +-
>  hw/vhost.h           |    3 +-
>  hw/vhost_net.c       |    2 +-
>  hw/virtio-pci.c      |    1 +
>  hw/virtio-scsi.c     |   56 ++++++++++++++++-
>  hw/virtio-scsi.h     |    1 +
>  qemu-common.h        |    1 +
>  qemu-config.c        |   16 +++++
>  qemu-options.hx      |    4 +
>  vl.c                 |   18 +++++
>  17 files changed, 378 insertions(+), 6 deletions(-)
>  create mode 100644 hw/vhost-scsi.c
>  create mode 100644 hw/vhost-scsi.h
> 
> -- 
> 1.7.2.5

Re: [Qemu-devel] [RFC-v2 1/6] msix: Work-around for vhost-scsi with KVM in-kernel MSI injection

[Jan Kiszka]

On 2012-08-13 10:35, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> This is required to get past the following assert with:
> 
> commit 1523ed9e1d46b0b54540049d491475ccac7e6421
> Author: Jan Kiszka <jan.kiszka@siemens.com>
> Date:   Thu May 17 10:32:39 2012 -0300
> 
>     virtio/vhost: Add support for KVM in-kernel MSI injection
> 
> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>  hw/msix.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/msix.c b/hw/msix.c
> index 800fc32..c1e6dc3 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -544,6 +544,9 @@ void msix_unset_vector_notifiers(PCIDevice *dev)
>  {
>      int vector;
>  
> +    if (!dev->msix_vector_use_notifier && !dev->msix_vector_release_notifier)
> +        return;
> +
>      assert(dev->msix_vector_use_notifier &&
>             dev->msix_vector_release_notifier);
>  
> 

I think to remember pointing out that there is a bug somewhere in the
reset code which deactivates a non-active vhost instance, no?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC-v2 1/6] msix: Work-around for vhost-scsi with KVM in-kernel MSI injection

[Michael S. Tsirkin]

On Mon, Aug 13, 2012 at 02:06:10PM +0200, Jan Kiszka wrote:
> On 2012-08-13 10:35, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > 
> > This is required to get past the following assert with:
> > 
> > commit 1523ed9e1d46b0b54540049d491475ccac7e6421
> > Author: Jan Kiszka <jan.kiszka@siemens.com>
> > Date:   Thu May 17 10:32:39 2012 -0300
> > 
> >     virtio/vhost: Add support for KVM in-kernel MSI injection
> > 
> > Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > Cc: Jan Kiszka <jan.kiszka@siemens.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Anthony Liguori <aliguori@us.ibm.com>
> > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > ---
> >  hw/msix.c |    3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)
> > 
> > diff --git a/hw/msix.c b/hw/msix.c
> > index 800fc32..c1e6dc3 100644
> > --- a/hw/msix.c
> > +++ b/hw/msix.c
> > @@ -544,6 +544,9 @@ void msix_unset_vector_notifiers(PCIDevice *dev)
> >  {
> >      int vector;
> >  
> > +    if (!dev->msix_vector_use_notifier && !dev->msix_vector_release_notifier)
> > +        return;
> > +
> >      assert(dev->msix_vector_use_notifier &&
> >             dev->msix_vector_release_notifier);
> >  
> > 
> 
> I think to remember pointing out that there is a bug somewhere in the
> reset code which deactivates a non-active vhost instance, no?
> 
> Jan

Could not find it. Could you dig it up pls?

> -- 
> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC-v2 1/6] msix: Work-around for vhost-scsi with KVM in-kernel MSI injection

[Jan Kiszka]

On 2012-08-13 20:03, Michael S. Tsirkin wrote:
> On Mon, Aug 13, 2012 at 02:06:10PM +0200, Jan Kiszka wrote:
>> On 2012-08-13 10:35, Nicholas A. Bellinger wrote:
>>> From: Nicholas Bellinger <nab@linux-iscsi.org>
>>>
>>> This is required to get past the following assert with:
>>>
>>> commit 1523ed9e1d46b0b54540049d491475ccac7e6421
>>> Author: Jan Kiszka <jan.kiszka@siemens.com>
>>> Date:   Thu May 17 10:32:39 2012 -0300
>>>
>>>     virtio/vhost: Add support for KVM in-kernel MSI injection
>>>
>>> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>>> Cc: Jan Kiszka <jan.kiszka@siemens.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Anthony Liguori <aliguori@us.ibm.com>
>>> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
>>> ---
>>>  hw/msix.c |    3 +++
>>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/hw/msix.c b/hw/msix.c
>>> index 800fc32..c1e6dc3 100644
>>> --- a/hw/msix.c
>>> +++ b/hw/msix.c
>>> @@ -544,6 +544,9 @@ void msix_unset_vector_notifiers(PCIDevice *dev)
>>>  {
>>>      int vector;
>>>  
>>> +    if (!dev->msix_vector_use_notifier && !dev->msix_vector_release_notifier)
>>> +        return;
>>> +
>>>      assert(dev->msix_vector_use_notifier &&
>>>             dev->msix_vector_release_notifier);
>>>  
>>>
>>
>> I think to remember pointing out that there is a bug somewhere in the
>> reset code which deactivates a non-active vhost instance, no?
>>
>> Jan
> 
> Could not find it. Could you dig it up pls?

http://thread.gmane.org/gmane.linux.scsi.target.devel/2277/focus=2309

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC-v2 1/6] msix: Work-around for vhost-scsi with KVM in-kernel MSI injection

[Michael S. Tsirkin]

On Mon, Aug 13, 2012 at 08:06:17PM +0200, Jan Kiszka wrote:
> On 2012-08-13 20:03, Michael S. Tsirkin wrote:
> > On Mon, Aug 13, 2012 at 02:06:10PM +0200, Jan Kiszka wrote:
> >> On 2012-08-13 10:35, Nicholas A. Bellinger wrote:
> >>> From: Nicholas Bellinger <nab@linux-iscsi.org>
> >>>
> >>> This is required to get past the following assert with:
> >>>
> >>> commit 1523ed9e1d46b0b54540049d491475ccac7e6421
> >>> Author: Jan Kiszka <jan.kiszka@siemens.com>
> >>> Date:   Thu May 17 10:32:39 2012 -0300
> >>>
> >>>     virtio/vhost: Add support for KVM in-kernel MSI injection
> >>>
> >>> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> >>> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> >>> Cc: Paolo Bonzini <pbonzini@redhat.com>
> >>> Cc: Anthony Liguori <aliguori@us.ibm.com>
> >>> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> >>> ---
> >>>  hw/msix.c |    3 +++
> >>>  1 files changed, 3 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/hw/msix.c b/hw/msix.c
> >>> index 800fc32..c1e6dc3 100644
> >>> --- a/hw/msix.c
> >>> +++ b/hw/msix.c
> >>> @@ -544,6 +544,9 @@ void msix_unset_vector_notifiers(PCIDevice *dev)
> >>>  {
> >>>      int vector;
> >>>  
> >>> +    if (!dev->msix_vector_use_notifier && !dev->msix_vector_release_notifier)
> >>> +        return;
> >>> +
> >>>      assert(dev->msix_vector_use_notifier &&
> >>>             dev->msix_vector_release_notifier);
> >>>  
> >>>
> >>
> >> I think to remember pointing out that there is a bug somewhere in the
> >> reset code which deactivates a non-active vhost instance, no?
> >>
> >> Jan
> > 
> > Could not find it. Could you dig it up pls?
> 
> http://thread.gmane.org/gmane.linux.scsi.target.devel/2277/focus=2309
> 
> Jan

Ah yes. So let's not work around, need to get to the bottom of that.

> -- 
> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC-v2 1/6] msix: Work-around for vhost-scsi with KVM in-kernel MSI injection

[Blue Swirl]

On Mon, Aug 13, 2012 at 8:35 AM, Nicholas A. Bellinger
<nab@linux-iscsi.org> wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
>
> This is required to get past the following assert with:
>
> commit 1523ed9e1d46b0b54540049d491475ccac7e6421
> Author: Jan Kiszka <jan.kiszka@siemens.com>
> Date:   Thu May 17 10:32:39 2012 -0300
>
>     virtio/vhost: Add support for KVM in-kernel MSI injection
>
> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>  hw/msix.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/hw/msix.c b/hw/msix.c
> index 800fc32..c1e6dc3 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -544,6 +544,9 @@ void msix_unset_vector_notifiers(PCIDevice *dev)
>  {
>      int vector;
>
> +    if (!dev->msix_vector_use_notifier && !dev->msix_vector_release_notifier)
> +        return;

Missing braces, please read CODING_STYLE.

> +
>      assert(dev->msix_vector_use_notifier &&
>             dev->msix_vector_release_notifier);
>
> --
> 1.7.2.5
>
>

Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost

[Blue Swirl]

On Mon, Aug 13, 2012 at 8:35 AM, Nicholas A. Bellinger
<nab@linux-iscsi.org> wrote:
> From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>
> This patch adds a new type of host device that drives the vhost_scsi
> device.  The syntax to add vhost-scsi is:
>
>   qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123
>
> The virtio-scsi emulated device will make use of vhost-scsi to process
> virtio-scsi requests inside the kernel and hand them to the in-kernel
> SCSI target stack using the tcm_vhost fabric driver.
>
> The tcm_vhost driver was merged into the upstream linux kernel for 3.6-rc2,
> and the commit can be found here:
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297
>
> Changelog v1 -> v2:
>
> - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as
>   starting point for v3.6-rc code (Stefan + ALiguori + nab)
> - Fix upstream qemu conflict in hw/qdev-properties.c
> - Make GET_ABI_VERSION use int (nab + mst)
> - Fix vhost-scsi case lables in configure (reported by paolo)
> - Convert qdev_prop_vhost_scsi to use ->get() + ->set() following
>   qdev_prop_netdev (reported by paolo)
> - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo)
>
> Changelog v0 -> v1:
>
> - Add VHOST_SCSI_SET_ENDPOINT call (stefan)
> - Enable vhost notifiers for multiple queues (Zhi)
> - clear vhost-scsi endpoint on stopped (Zhi)
> - Add CONFIG_VHOST_SCSI for QEMU build configure (nab)
> - Rename vhost_vring_target -> vhost_scsi_target (mst + nab)
> - Add support for VHOST_SCSI_GET_ABI_VERSION ioctl (aliguori + nab)
>
> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>  configure            |   10 +++
>  hw/Makefile.objs     |    1 +
>  hw/qdev-properties.c |   40 ++++++++++++
>  hw/qdev.h            |    3 +
>  hw/vhost-scsi.c      |  170 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/vhost-scsi.h      |   50 +++++++++++++++
>  qemu-common.h        |    1 +
>  qemu-config.c        |   16 +++++
>  qemu-options.hx      |    4 +
>  vl.c                 |   18 +++++
>  10 files changed, 313 insertions(+), 0 deletions(-)
>  create mode 100644 hw/vhost-scsi.c
>  create mode 100644 hw/vhost-scsi.h
>
> diff --git a/configure b/configure
> index f0dbc03..1f03202 100755
> --- a/configure
> +++ b/configure
> @@ -168,6 +168,7 @@ libattr=""
>  xfs=""
>
>  vhost_net="no"
> +vhost_scsi="no"
>  kvm="no"
>  gprof="no"
>  debug_tcg="no"
> @@ -513,6 +514,7 @@ Haiku)
>    usb="linux"
>    kvm="yes"
>    vhost_net="yes"
> +  vhost_scsi="yes"
>    if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
>      audio_possible_drivers="$audio_possible_drivers fmod"
>    fi
> @@ -818,6 +820,10 @@ for opt do
>    ;;
>    --enable-vhost-net) vhost_net="yes"
>    ;;
> +  --disable-vhost-scsi) vhost_scsi="no"
> +  ;;
> +  --enable-vhost-scsi) vhost_scsi="yes"
> +  ;;
>    --disable-opengl) opengl="no"
>    ;;
>    --enable-opengl) opengl="yes"
> @@ -3116,6 +3122,7 @@ echo "posix_madvise     $posix_madvise"
>  echo "uuid support      $uuid"
>  echo "libcap-ng support $cap_ng"
>  echo "vhost-net support $vhost_net"
> +echo "vhost-scsi support $vhost_scsi"
>  echo "Trace backend     $trace_backend"
>  echo "Trace output file $trace_file-<pid>"
>  echo "spice support     $spice"
> @@ -3828,6 +3835,9 @@ case "$target_arch2" in
>        if test "$vhost_net" = "yes" ; then
>          echo "CONFIG_VHOST_NET=y" >> $config_target_mak
>        fi
> +      if test "$vhost_scsi" = "yes" ; then
> +        echo "CONFIG_VHOST_SCSI=y" >> $config_target_mak
> +      fi
>      fi
>  esac
>  case "$target_arch2" in
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 3ba5dd0..6ab75ec 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -169,6 +169,7 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o
>  obj-$(CONFIG_VIRTIO) += virtio-serial-bus.o virtio-scsi.o
>  obj-$(CONFIG_SOFTMMU) += vhost_net.o
>  obj-$(CONFIG_VHOST_NET) += vhost.o
> +obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o
>  obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/
>  obj-$(CONFIG_NO_PCI) += pci-stub.o
>  obj-$(CONFIG_VGA) += vga.o
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 8aca0d4..0266266 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -4,6 +4,7 @@
>  #include "blockdev.h"
>  #include "hw/block-common.h"
>  #include "net/hub.h"
> +#include "vhost-scsi.h"
>
>  void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
>  {
> @@ -696,6 +697,45 @@ PropertyInfo qdev_prop_vlan = {
>      .set   = set_vlan,
>  };
>
> +/* --- vhost-scsi --- */
> +
> +static int parse_vhost_scsi_dev(DeviceState *dev, const char *str, void **ptr)
> +{
> +   VHostSCSI *p;
> +
> +   p = find_vhost_scsi(str);
> +   if (p == NULL)
> +       return -ENOENT;

Braces, please.

> +
> +   *ptr = p;
> +   return 0;
> +}
> +
> +static const char *print_vhost_scsi_dev(void *ptr)
> +{
> +    VHostSCSI *p = ptr;
> +
> +    return (p) ? vhost_scsi_get_id(p) : "<null>";
> +}
> +
> +static void get_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> +                       const char *name, Error **errp)
> +{
> +    get_pointer(obj, v, opaque, print_vhost_scsi_dev, name, errp);
> +}
> +
> +static void set_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> +                               const char *name, Error **errp)
> +{
> +    set_pointer(obj, v, opaque, parse_vhost_scsi_dev, name, errp);
> +}
> +
> +PropertyInfo qdev_prop_vhost_scsi = {
> +     .name = "vhost-scsi",
> +     .get  = get_vhost_scsi_dev,
> +     .set  = set_vhost_scsi_dev,
> +};
> +
>  /* --- pointer --- */
>
>  /* Not a proper property, just for dirty hacks.  TODO Remove it!  */
> diff --git a/hw/qdev.h b/hw/qdev.h
> index d699194..d5873bb 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -238,6 +238,7 @@ extern PropertyInfo qdev_prop_vlan;
>  extern PropertyInfo qdev_prop_pci_devfn;
>  extern PropertyInfo qdev_prop_blocksize;
>  extern PropertyInfo qdev_prop_pci_host_devaddr;
> +extern PropertyInfo qdev_prop_vhost_scsi;
>
>  #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
>          .name      = (_name),                                    \
> @@ -305,6 +306,8 @@ extern PropertyInfo qdev_prop_pci_host_devaddr;
>      DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
>  #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
>      DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
> +#define DEFINE_PROP_VHOST_SCSI(_n, _s, _f)       \
> +    DEFINE_PROP(_n, _s, _f, qdev_prop_vhost_scsi, VHostSCSI*)
>
>  #define DEFINE_PROP_END_OF_LIST()               \
>      {}
> diff --git a/hw/vhost-scsi.c b/hw/vhost-scsi.c
> new file mode 100644
> index 0000000..7145b2d
> --- /dev/null
> +++ b/hw/vhost-scsi.c
> @@ -0,0 +1,170 @@
> +/*
> + * vhost_scsi host device
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include <sys/ioctl.h>
> +#include "config.h"
> +#include "qemu-queue.h"
> +#include "vhost-scsi.h"
> +#include "vhost.h"
> +
> +struct VHostSCSI {
> +    const char *id;
> +    const char *wwpn;
> +    uint16_t tpgt;
> +    struct vhost_dev dev;
> +    struct vhost_virtqueue vqs[3];
> +    QLIST_ENTRY(VHostSCSI) list;
> +};
> +
> +static QLIST_HEAD(, VHostSCSI) vhost_scsi_list =
> +    QLIST_HEAD_INITIALIZER(vhost_scsi_list);
> +
> +VHostSCSI *find_vhost_scsi(const char *id)
> +{
> +    VHostSCSI *vs;
> +
> +    QLIST_FOREACH(vs, &vhost_scsi_list, list) {
> +        if (strcmp(id, vs->id) == 0) {
> +            return vs;
> +        }
> +    }
> +    return NULL;
> +}
> +
> +const char *vhost_scsi_get_id(VHostSCSI *vs)
> +{
> +    return vs->id;
> +}
> +
> +int vhost_scsi_start(VHostSCSI *vs, VirtIODevice *vdev)
> +{
> +    int ret, abi_version;
> +    struct vhost_scsi_target backend;
> +
> +    if (!vhost_dev_query(&vs->dev, vdev)) {
> +        return -ENOTSUP;
> +    }
> +
> +    vs->dev.nvqs = 3;
> +    vs->dev.vqs = vs->vqs;
> +
> +    ret = vhost_dev_enable_notifiers(&vs->dev, vdev);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    ret = vhost_dev_start(&vs->dev, vdev);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    memset(&backend, 0, sizeof(backend));
> +    ret = ioctl(vs->dev.control, VHOST_SCSI_GET_ABI_VERSION, &abi_version);
> +    if (ret < 0) {
> +        ret = -errno;
> +        vhost_dev_stop(&vs->dev, vdev);
> +        return ret;
> +    }
> +    if (abi_version > VHOST_SCSI_ABI_VERSION) {
> +        fprintf(stderr, "The running tcm_vhost kernel abi_version: %d is greater"
> +               " than vhost_scsi userspace supports: %d\n", abi_version,
> +               VHOST_SCSI_ABI_VERSION);
> +        ret = -ENOSYS;
> +        vhost_dev_stop(&vs->dev, vdev);
> +        return ret;
> +    }
> +    fprintf(stdout, "TCM_vHost ABI version: %d\n", abi_version);
> +
> +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);

Please change vhost_wwpn to plain char *, then the cast can be removed.

> +    backend.vhost_tpgt = vs->tpgt;
> +    ret = ioctl(vs->dev.control, VHOST_SCSI_SET_ENDPOINT, &backend);
> +    if (ret < 0) {
> +        ret = -errno;
> +        vhost_dev_stop(&vs->dev, vdev);
> +        return ret;
> +    }
> +
> +    return 0;
> +}
> +
> +void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev)
> +{
> +    int ret;
> +    struct vhost_scsi_target backend;
> +
> +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);

Also here.

> +    backend.vhost_tpgt = vs->tpgt;
> +    ret = ioctl(vs->dev.control, VHOST_SCSI_CLEAR_ENDPOINT, &backend);
> +    if (ret < 0) {
> +        fprintf(stderr, "Failed to clear endpoint\n");
> +    }
> +
> +    vhost_dev_stop(&vs->dev, vdev);
> +}
> +
> +static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
> +                                 uint16_t tpgt)
> +{
> +    VHostSCSI *vs = g_malloc0(sizeof(*vs));
> +    int ret;
> +
> +    /* TODO set up vhost-scsi device and bind to tcm_vhost/$wwpm/tpgt_$tpgt */
> +    fprintf(stderr, "wwpn = \"%s\" tpgt = \"%u\"\n", id, tpgt);
> +
> +    ret = vhost_dev_init(&vs->dev, -1, "/dev/vhost-scsi", false);
> +    if (ret < 0) {
> +        fprintf(stderr, "vhost-scsi: vhost initialization failed: %s\n",
> +                strerror(-ret));
> +        return NULL;
> +    }
> +    vs->dev.backend_features = 0;
> +    vs->dev.acked_features = 0;
> +
> +    vs->id = g_strdup(id);
> +    vs->wwpn = g_strdup(wwpn);
> +    vs->tpgt = tpgt;
> +    QLIST_INSERT_HEAD(&vhost_scsi_list, vs, list);
> +
> +    return vs;
> +}
> +
> +VHostSCSI *vhost_scsi_add_opts(QemuOpts *opts)
> +{
> +    const char *id;
> +    const char *wwpn;
> +    uint64_t tpgt;
> +
> +    id = qemu_opts_id(opts);
> +    if (!id) {
> +        fprintf(stderr, "vhost-scsi: no id specified\n");
> +        return NULL;
> +    }
> +    if (find_vhost_scsi(id)) {
> +        fprintf(stderr, "duplicate vhost-scsi: \"%s\"\n", id);
> +        return NULL;
> +    }
> +
> +    wwpn = qemu_opt_get(opts, "wwpn");
> +    if (!wwpn) {
> +        fprintf(stderr, "vhost-scsi: \"%s\" missing wwpn\n", id);
> +        return NULL;
> +    }
> +
> +    tpgt = qemu_opt_get_number(opts, "tpgt", UINT64_MAX);
> +    if (tpgt > UINT16_MAX) {
> +        fprintf(stderr, "vhost-scsi: \"%s\" needs a 16-bit tpgt\n", id);
> +        return NULL;
> +    }
> +
> +    return vhost_scsi_add(id, wwpn, tpgt);
> +}
> diff --git a/hw/vhost-scsi.h b/hw/vhost-scsi.h
> new file mode 100644
> index 0000000..f3096dc
> --- /dev/null
> +++ b/hw/vhost-scsi.h
> @@ -0,0 +1,50 @@
> +/*
> + * vhost_scsi host device
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef VHOST_SCSI_H
> +#define VHOST_SCSI_H
> +
> +#include "qemu-common.h"
> +#include "qemu-option.h"
> +
> +/*
> + * Used by QEMU userspace to ensure a consistent vhost-scsi ABI.
> + *
> + * ABI Rev 0: July 2012 version starting point for v3.6-rc merge candidate +
> + *            RFC-v2 vhost-scsi userspace.  Add GET_ABI_VERSION ioctl usage
> + */
> +
> +#define VHOST_SCSI_ABI_VERSION 0
> +
> +/* TODO #include <linux/vhost.h> properly */
> +/* For VHOST_SCSI_SET_ENDPOINT/VHOST_SCSI_CLEAR_ENDPOINT ioctl */
> +struct vhost_scsi_target {
> +    int abi_version;
> +    unsigned char vhost_wwpn[224];
> +    unsigned short vhost_tpgt;
> +};
> +
> +#define VHOST_VIRTIO 0xAF
> +#define VHOST_SCSI_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x40, struct vhost_scsi_target)
> +#define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct vhost_scsi_target)
> +#define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, struct vhost_scsi_target)
> +
> +VHostSCSI *find_vhost_scsi(const char *id);
> +const char *vhost_scsi_get_id(VHostSCSI *vs);
> +
> +VHostSCSI *vhost_scsi_add_opts(QemuOpts *opts);
> +
> +int vhost_scsi_start(VHostSCSI *vs, VirtIODevice *vdev);
> +void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev);
> +
> +#endif
> diff --git a/qemu-common.h b/qemu-common.h
> index f9deca6..ec36002 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -280,6 +280,7 @@ typedef struct EventNotifier EventNotifier;
>  typedef struct VirtIODevice VirtIODevice;
>  typedef struct QEMUSGList QEMUSGList;
>  typedef struct SHPCDevice SHPCDevice;
> +typedef struct VHostSCSI VHostSCSI;
>
>  typedef uint64_t pcibus_t;
>
> diff --git a/qemu-config.c b/qemu-config.c
> index 5c3296b..33399ea 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -626,6 +626,21 @@ QemuOptsList qemu_boot_opts = {
>      },
>  };
>
> +QemuOptsList qemu_vhost_scsi_opts = {
> +    .name = "vhost-scsi",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_vhost_scsi_opts.head),
> +    .desc = {
> +        {
> +            .name = "wwpn",
> +            .type = QEMU_OPT_STRING,
> +        }, {
> +            .name = "tpgt",
> +            .type = QEMU_OPT_NUMBER,
> +        },
> +        { /* end of list */ }
> +    },
> +};
> +
>  static QemuOptsList *vm_config_groups[32] = {
>      &qemu_drive_opts,
>      &qemu_chardev_opts,
> @@ -641,6 +656,7 @@ static QemuOptsList *vm_config_groups[32] = {
>      &qemu_machine_opts,
>      &qemu_boot_opts,
>      &qemu_iscsi_opts,
> +    &qemu_vhost_scsi_opts,
>      NULL,
>  };
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 47cb5bd..4e7a03c 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -565,6 +565,10 @@ possible drivers and properties, use @code{-device ?} and
>  ETEXI
>
>  DEFHEADING()
> +DEF("vhost-scsi", HAS_ARG, QEMU_OPTION_vhost_scsi,
> +    "-vhost-scsi wwpn=string0,tpgt=number0\n"
> +    "                add vhost-scsi device\n",
> +    QEMU_ARCH_ALL)
>
>  DEFHEADING(File system options:)
>
> diff --git a/vl.c b/vl.c
> index 91076f0..61c8284 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -144,6 +144,7 @@ int main(int argc, char **argv)
>  #include "qemu-options.h"
>  #include "qmp-commands.h"
>  #include "main-loop.h"
> +#include "hw/vhost-scsi.h"
>  #ifdef CONFIG_VIRTFS
>  #include "fsdev/qemu-fsdev.h"
>  #endif
> @@ -1858,6 +1859,14 @@ static int fsdev_init_func(QemuOpts *opts, void *opaque)
>  }
>  #endif
>
> +static int vhost_scsi_init_func(QemuOpts *opts, void *opaque)
> +{
> +    if (!vhost_scsi_add_opts(opts)) {
> +        return -1;
> +    }
> +    return 0;
> +}
> +
>  static int mon_init_func(QemuOpts *opts, void *opaque)
>  {
>      CharDriverState *chr;
> @@ -2617,6 +2626,11 @@ int main(int argc, char **argv, char **envp)
>                  }
>                  break;
>  #endif
> +            case QEMU_OPTION_vhost_scsi:
> +                if (!qemu_opts_parse(qemu_find_opts("vhost-scsi"), optarg, 0)) {
> +                    exit(1);
> +                }
> +                break;
>  #ifdef CONFIG_SLIRP
>              case QEMU_OPTION_tftp:
>                  legacy_tftp_prefix = optarg;
> @@ -3337,6 +3351,10 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  #endif
> +    if (qemu_opts_foreach(qemu_find_opts("vhost-scsi"),
> +                          vhost_scsi_init_func, NULL, 1)) {
> +        exit(1);
> +    }
>
>      os_daemonize();
>
> --
> 1.7.2.5
>
>

Re: [Qemu-devel] [RFC-v2 1/6] msix: Work-around for vhost-scsi with KVM in-kernel MSI injection

[Nicholas A. Bellinger]

On Mon, 2012-08-13 at 21:17 +0300, Michael S. Tsirkin wrote:
> On Mon, Aug 13, 2012 at 08:06:17PM +0200, Jan Kiszka wrote:
> > On 2012-08-13 20:03, Michael S. Tsirkin wrote:
> > > On Mon, Aug 13, 2012 at 02:06:10PM +0200, Jan Kiszka wrote:
> > >> On 2012-08-13 10:35, Nicholas A. Bellinger wrote:
> > >>> From: Nicholas Bellinger <nab@linux-iscsi.org>
> > >>>
> > >>> This is required to get past the following assert with:
> > >>>
> > >>> commit 1523ed9e1d46b0b54540049d491475ccac7e6421
> > >>> Author: Jan Kiszka <jan.kiszka@siemens.com>
> > >>> Date:   Thu May 17 10:32:39 2012 -0300
> > >>>
> > >>>     virtio/vhost: Add support for KVM in-kernel MSI injection
> > >>>
> > >>> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > >>> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> > >>> Cc: Paolo Bonzini <pbonzini@redhat.com>
> > >>> Cc: Anthony Liguori <aliguori@us.ibm.com>
> > >>> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > >>> ---
> > >>>  hw/msix.c |    3 +++
> > >>>  1 files changed, 3 insertions(+), 0 deletions(-)
> > >>>
> > >>> diff --git a/hw/msix.c b/hw/msix.c
> > >>> index 800fc32..c1e6dc3 100644
> > >>> --- a/hw/msix.c
> > >>> +++ b/hw/msix.c
> > >>> @@ -544,6 +544,9 @@ void msix_unset_vector_notifiers(PCIDevice *dev)
> > >>>  {
> > >>>      int vector;
> > >>>  
> > >>> +    if (!dev->msix_vector_use_notifier && !dev->msix_vector_release_notifier)
> > >>> +        return;
> > >>> +
> > >>>      assert(dev->msix_vector_use_notifier &&
> > >>>             dev->msix_vector_release_notifier);
> > >>>  
> > >>>
> > >>
> > >> I think to remember pointing out that there is a bug somewhere in the
> > >> reset code which deactivates a non-active vhost instance, no?
> > >>
> > >> Jan
> > > 
> > > Could not find it. Could you dig it up pls?
> > 
> > http://thread.gmane.org/gmane.linux.scsi.target.devel/2277/focus=2309
> > 
> > Jan
> 
> Ah yes. So let's not work around, need to get to the bottom of that.
> 

Ok, so the assert being triggered in msix_unset_vector_notifiers()
appears to have been a side effect of the memory corruption bug in
virtio-scsi fixed in Patch #6, and is no longer required to start
vhost-scsi with the bugfix in place.

That said, dropping this patch for RFC-v3..

Re: [Qemu-devel] [RFC-v2 6/6] virtio-scsi: Fix incorrect VirtIOSCSI->cmd_vqs[0] definition

[Nicholas A. Bellinger]

On Mon, 2012-08-13 at 12:02 +0300, Michael S. Tsirkin wrote:
> On Mon, Aug 13, 2012 at 08:35:17AM +0000, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > 
> > This patch fixes bug in the definition of VirtIOSCSI->cmd_vqs[0],
> > where the return of virtio_add_queue() in virtio_scsi_init() ends up
> > overwriting past the end of ->cmd_vqs[0].
> > 
> > Since virtio_scsi currently assumes a single vqs for data, this patch
> > simply changes ->cmd_vqs[1] to handle the single VirtQueue.
> > 
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> This is a bugfix we need even without vhost, right?
> 

I believe so, as it appears to be stomping past the end of memory for
every virtio-scsi initialization regardless of vhost usage.. 

Paolo, can you pickup this fix now for stable so it can be dropped from
RFC-v3..?

--nab

> > ---
> >  hw/virtio-scsi.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
> > index 5e2ff6b..2c70f89 100644
> > --- a/hw/virtio-scsi.c
> > +++ b/hw/virtio-scsi.c
> > @@ -150,7 +150,7 @@ typedef struct {
> >      bool events_dropped;
> >      VirtQueue *ctrl_vq;
> >      VirtQueue *event_vq;
> > -    VirtQueue *cmd_vqs[0];
> > +    VirtQueue *cmd_vqs[1];
> >  
> >      bool vhost_started;
> >      VHostSCSI *vhost_scsi;
> > -- 
> > 1.7.2.5
> --
> To unsubscribe from this list: send the line "unsubscribe target-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost

[Nicholas A. Bellinger]

On Mon, 2012-08-13 at 11:53 +0300, Michael S. Tsirkin wrote:
> On Mon, Aug 13, 2012 at 08:35:14AM +0000, Nicholas A. Bellinger wrote:
> > From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > 
> > This patch adds a new type of host device that drives the vhost_scsi
> > device.  The syntax to add vhost-scsi is:
> > 
> >   qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123
> > 
> > The virtio-scsi emulated device will make use of vhost-scsi to process
> > virtio-scsi requests inside the kernel and hand them to the in-kernel
> > SCSI target stack using the tcm_vhost fabric driver.

<SNIP>

> > +static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
> > +                                 uint16_t tpgt)
> > +{
> > +    VHostSCSI *vs = g_malloc0(sizeof(*vs));
> > +    int ret;
> > +
> > +    /* TODO set up vhost-scsi device and bind to tcm_vhost/$wwpm/tpgt_$tpgt */
> > +    fprintf(stderr, "wwpn = \"%s\" tpgt = \"%u\"\n", id, tpgt);
> > +
> > +    ret = vhost_dev_init(&vs->dev, -1, "/dev/vhost-scsi", false);
> 
> This -1 is a hack. You need to support passing in fd from
> the monitor, and pass it here.
> 

Mmm, looking at how vhost_net_init + tap.c does this, but am not quite
what fd needs to be propagated up for virtio-scsi -> vhost-scsi..

Can you please elaborate on this one a bit more..?

--nab

Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost

[Nicholas A. Bellinger]

On Mon, 2012-08-13 at 11:59 +0300, Michael S. Tsirkin wrote:
> On Mon, Aug 13, 2012 at 08:35:14AM +0000, Nicholas A. Bellinger wrote:
> > From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > 
> > This patch adds a new type of host device that drives the vhost_scsi
> > device.  The syntax to add vhost-scsi is:
> > 
> >   qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123
> > 
> > The virtio-scsi emulated device will make use of vhost-scsi to process
> > virtio-scsi requests inside the kernel and hand them to the in-kernel
> > SCSI target stack using the tcm_vhost fabric driver.
> > 
> > The tcm_vhost driver was merged into the upstream linux kernel for 3.6-rc2,
> > and the commit can be found here:
> > 
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297
> > 
> > Changelog v1 -> v2:
> > 
> > - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as
> >   starting point for v3.6-rc code (Stefan + ALiguori + nab)
> > - Fix upstream qemu conflict in hw/qdev-properties.c
> > - Make GET_ABI_VERSION use int (nab + mst)
> > - Fix vhost-scsi case lables in configure (reported by paolo)
> > - Convert qdev_prop_vhost_scsi to use ->get() + ->set() following
> >   qdev_prop_netdev (reported by paolo)
> > - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo)
> > 
> > Changelog v0 -> v1:
> > 
> > - Add VHOST_SCSI_SET_ENDPOINT call (stefan)
> > - Enable vhost notifiers for multiple queues (Zhi)
> > - clear vhost-scsi endpoint on stopped (Zhi)
> > - Add CONFIG_VHOST_SCSI for QEMU build configure (nab)
> > - Rename vhost_vring_target -> vhost_scsi_target (mst + nab)
> > - Add support for VHOST_SCSI_GET_ABI_VERSION ioctl (aliguori + nab)
> > 
> > Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> > Cc: Anthony Liguori <aliguori@us.ibm.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> 
> Sent mail too fast, sorry. More comments below.
> 
> > ---
> >  configure            |   10 +++
> >  hw/Makefile.objs     |    1 +
> >  hw/qdev-properties.c |   40 ++++++++++++
> >  hw/qdev.h            |    3 +
> >  hw/vhost-scsi.c      |  170 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  hw/vhost-scsi.h      |   50 +++++++++++++++
> >  qemu-common.h        |    1 +
> >  qemu-config.c        |   16 +++++
> >  qemu-options.hx      |    4 +
> >  vl.c                 |   18 +++++
> >  10 files changed, 313 insertions(+), 0 deletions(-)
> >  create mode 100644 hw/vhost-scsi.c
> >  create mode 100644 hw/vhost-scsi.h
> > 
> > diff --git a/configure b/configure
> > index f0dbc03..1f03202 100755
> > --- a/configure
> > +++ b/configure
> > @@ -168,6 +168,7 @@ libattr=""
> >  xfs=""
> >  
> >  vhost_net="no"
> > +vhost_scsi="no"
> >  kvm="no"
> >  gprof="no"
> >  debug_tcg="no"
> > @@ -513,6 +514,7 @@ Haiku)
> >    usb="linux"
> >    kvm="yes"
> >    vhost_net="yes"
> > +  vhost_scsi="yes"
> >    if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
> >      audio_possible_drivers="$audio_possible_drivers fmod"
> >    fi
> > @@ -818,6 +820,10 @@ for opt do
> >    ;;
> >    --enable-vhost-net) vhost_net="yes"
> >    ;;
> > +  --disable-vhost-scsi) vhost_scsi="no"
> > +  ;;
> > +  --enable-vhost-scsi) vhost_scsi="yes"
> > +  ;;
> >    --disable-opengl) opengl="no"
> >    ;;
> >    --enable-opengl) opengl="yes"
> > @@ -3116,6 +3122,7 @@ echo "posix_madvise     $posix_madvise"
> >  echo "uuid support      $uuid"
> >  echo "libcap-ng support $cap_ng"
> >  echo "vhost-net support $vhost_net"
> > +echo "vhost-scsi support $vhost_scsi"
> >  echo "Trace backend     $trace_backend"
> >  echo "Trace output file $trace_file-<pid>"
> >  echo "spice support     $spice"
> > @@ -3828,6 +3835,9 @@ case "$target_arch2" in
> >        if test "$vhost_net" = "yes" ; then
> >          echo "CONFIG_VHOST_NET=y" >> $config_target_mak
> >        fi
> > +      if test "$vhost_scsi" = "yes" ; then
> > +        echo "CONFIG_VHOST_SCSI=y" >> $config_target_mak
> > +      fi
> >      fi
> >  esac
> >  case "$target_arch2" in
> > diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> > index 3ba5dd0..6ab75ec 100644
> > --- a/hw/Makefile.objs
> > +++ b/hw/Makefile.objs
> > @@ -169,6 +169,7 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o
> >  obj-$(CONFIG_VIRTIO) += virtio-serial-bus.o virtio-scsi.o
> >  obj-$(CONFIG_SOFTMMU) += vhost_net.o
> >  obj-$(CONFIG_VHOST_NET) += vhost.o
> > +obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o
> >  obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/
> >  obj-$(CONFIG_NO_PCI) += pci-stub.o
> >  obj-$(CONFIG_VGA) += vga.o
> > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> > index 8aca0d4..0266266 100644
> > --- a/hw/qdev-properties.c
> > +++ b/hw/qdev-properties.c
> > @@ -4,6 +4,7 @@
> >  #include "blockdev.h"
> >  #include "hw/block-common.h"
> >  #include "net/hub.h"
> > +#include "vhost-scsi.h"
> >  
> >  void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
> >  {
> > @@ -696,6 +697,45 @@ PropertyInfo qdev_prop_vlan = {
> >      .set   = set_vlan,
> >  };
> >  
> > +/* --- vhost-scsi --- */
> > +
> > +static int parse_vhost_scsi_dev(DeviceState *dev, const char *str, void **ptr)
> > +{
> > +   VHostSCSI *p;
> > +
> > +   p = find_vhost_scsi(str);
> > +   if (p == NULL)
> > +       return -ENOENT;
> > +
> > +   *ptr = p;
> > +   return 0;
> > +}
> > +
> > +static const char *print_vhost_scsi_dev(void *ptr)
> > +{
> > +    VHostSCSI *p = ptr;
> > +
> > +    return (p) ? vhost_scsi_get_id(p) : "<null>";
> > +}
> > +
> > +static void get_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> > +                       const char *name, Error **errp)
> > +{
> > +    get_pointer(obj, v, opaque, print_vhost_scsi_dev, name, errp);
> > +}
> > +
> > +static void set_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> > +                               const char *name, Error **errp)
> > +{
> > +    set_pointer(obj, v, opaque, parse_vhost_scsi_dev, name, errp);
> > +}
> > +
> > +PropertyInfo qdev_prop_vhost_scsi = {
> > +     .name = "vhost-scsi",
> > +     .get  = get_vhost_scsi_dev,
> > +     .set  = set_vhost_scsi_dev,
> > +};
> > +
> >  /* --- pointer --- */
> >  
> >  /* Not a proper property, just for dirty hacks.  TODO Remove it!  */
> 
> Why does this make sense in the generic qdev-properties?
> There's exactly one device that can use this, no?
> 

Mmmm, not sure on this one either..  Stefan..?

> > diff --git a/hw/qdev.h b/hw/qdev.h
> > index d699194..d5873bb 100644
> > --- a/hw/qdev.h
> > +++ b/hw/qdev.h
> > @@ -238,6 +238,7 @@ extern PropertyInfo qdev_prop_vlan;
> >  extern PropertyInfo qdev_prop_pci_devfn;
> >  extern PropertyInfo qdev_prop_blocksize;
> >  extern PropertyInfo qdev_prop_pci_host_devaddr;
> > +extern PropertyInfo qdev_prop_vhost_scsi;
> >  
> >  #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
> >          .name      = (_name),                                    \
> > @@ -305,6 +306,8 @@ extern PropertyInfo qdev_prop_pci_host_devaddr;
> >      DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
> >  #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
> >      DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
> > +#define DEFINE_PROP_VHOST_SCSI(_n, _s, _f)       \
> > +    DEFINE_PROP(_n, _s, _f, qdev_prop_vhost_scsi, VHostSCSI*)
> >
> 
> Can this move to vhost-scsi.c?
>   

Done

> >  #define DEFINE_PROP_END_OF_LIST()               \
> >      {}
> > diff --git a/hw/vhost-scsi.c b/hw/vhost-scsi.c
> > new file mode 100644
> > index 0000000..7145b2d
> > --- /dev/null
> > +++ b/hw/vhost-scsi.c
> > @@ -0,0 +1,170 @@
> > +/*
> > + * vhost_scsi host device
> > + *
> > + * Copyright IBM, Corp. 2011
> > + *
> > + * Authors:
> > + *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> > + * See the COPYING.LIB file in the top-level directory.
> > + *
> > + */
> > +
> > +#include <sys/ioctl.h>
> > +#include "config.h"
> > +#include "qemu-queue.h"
> > +#include "vhost-scsi.h"
> > +#include "vhost.h"
> > +
> > +struct VHostSCSI {
> > +    const char *id;
> > +    const char *wwpn;
> > +    uint16_t tpgt;
> > +    struct vhost_dev dev;
> > +    struct vhost_virtqueue vqs[3];
> 
> Could you add enum for vq numbers pls?
> 

Done

> > +    QLIST_ENTRY(VHostSCSI) list;
> > +};
> > +
> > +static QLIST_HEAD(, VHostSCSI) vhost_scsi_list =
> > +    QLIST_HEAD_INITIALIZER(vhost_scsi_list);
> > +
> > +VHostSCSI *find_vhost_scsi(const char *id)
> > +{
> > +    VHostSCSI *vs;
> > +
> > +    QLIST_FOREACH(vs, &vhost_scsi_list, list) {
> > +        if (strcmp(id, vs->id) == 0) {
> 
> !strcmp
> 

Done

> > +            return vs;
> > +        }
> > +    }
> > +    return NULL;
> > +}
> > +
> > +const char *vhost_scsi_get_id(VHostSCSI *vs)
> > +{
> > +    return vs->id;
> > +}
> > +
> > +int vhost_scsi_start(VHostSCSI *vs, VirtIODevice *vdev)
> > +{
> > +    int ret, abi_version;
> > +    struct vhost_scsi_target backend;
> > +
> > +    if (!vhost_dev_query(&vs->dev, vdev)) {
> > +        return -ENOTSUP;
> > +    }
> > +
> > +    vs->dev.nvqs = 3;
> > +    vs->dev.vqs = vs->vqs;
> > +
> > +    ret = vhost_dev_enable_notifiers(&vs->dev, vdev);
> > +    if (ret < 0) {
> > +        return ret;
> > +    }
> > +
> > +    ret = vhost_dev_start(&vs->dev, vdev);
> > +    if (ret < 0) {
> > +        return ret;
> > +    }
> > +
> > +    memset(&backend, 0, sizeof(backend));
> > +    ret = ioctl(vs->dev.control, VHOST_SCSI_GET_ABI_VERSION, &abi_version);
> > +    if (ret < 0) {
> > +        ret = -errno;
> > +        vhost_dev_stop(&vs->dev, vdev);
> > +        return ret;
> > +    }
> > +    if (abi_version > VHOST_SCSI_ABI_VERSION) {
> > +        fprintf(stderr, "The running tcm_vhost kernel abi_version: %d is greater"
> > +		" than vhost_scsi userspace supports: %d\n", abi_version,
> > +		VHOST_SCSI_ABI_VERSION);
> > +        ret = -ENOSYS;
> > +        vhost_dev_stop(&vs->dev, vdev);
> > +        return ret;
> > +    }
> > +    fprintf(stdout, "TCM_vHost ABI version: %d\n", abi_version);
> > +
> > +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
> > +    backend.vhost_tpgt = vs->tpgt;
> > +    ret = ioctl(vs->dev.control, VHOST_SCSI_SET_ENDPOINT, &backend);
> > +    if (ret < 0) {
> > +        ret = -errno;
> > +        vhost_dev_stop(&vs->dev, vdev);
> > +        return ret;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev)
> > +{
> > +    int ret;
> > +    struct vhost_scsi_target backend;
> > +
> > +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
> > +    backend.vhost_tpgt = vs->tpgt;
> > +    ret = ioctl(vs->dev.control, VHOST_SCSI_CLEAR_ENDPOINT, &backend);
> > +    if (ret < 0) {
> > +        fprintf(stderr, "Failed to clear endpoint\n");
> > +    }
> > +
> > +    vhost_dev_stop(&vs->dev, vdev);
> > +}
> > +
> > +static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
> > +                                 uint16_t tpgt)
> > +{
> > +    VHostSCSI *vs = g_malloc0(sizeof(*vs));
> > +    int ret;
> > +
> > +    /* TODO set up vhost-scsi device and bind to tcm_vhost/$wwpm/tpgt_$tpgt */
> > +    fprintf(stderr, "wwpn = \"%s\" tpgt = \"%u\"\n", id, tpgt);
> > +
> 
> Please do not keep debugging fprintfs around.
> 

Dropped

> > +    ret = vhost_dev_init(&vs->dev, -1, "/dev/vhost-scsi", false);
> 
> commented on this separately
> 

...

> > +    if (ret < 0) {
> > +        fprintf(stderr, "vhost-scsi: vhost initialization failed: %s\n",
> > +                strerror(-ret));
> 
> errors should go to monitor, here and elsewhere.
> 

I think this means using monitor_printf() right..?

Looking at that now..

Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost

[Nicholas A. Bellinger]

On Mon, 2012-08-13 at 19:47 +0000, Blue Swirl wrote:
> On Mon, Aug 13, 2012 at 8:35 AM, Nicholas A. Bellinger
> <nab@linux-iscsi.org> wrote:
> > From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> >
> > This patch adds a new type of host device that drives the vhost_scsi
> > device.  The syntax to add vhost-scsi is:
> >
> >   qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123
> >
> > The virtio-scsi emulated device will make use of vhost-scsi to process
> > virtio-scsi requests inside the kernel and hand them to the in-kernel
> > SCSI target stack using the tcm_vhost fabric driver.
> >
> > The tcm_vhost driver was merged into the upstream linux kernel for 3.6-rc2,
> > and the commit can be found here:
> >
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297
> >
> > Changelog v1 -> v2:
> >
> > - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as
> >   starting point for v3.6-rc code (Stefan + ALiguori + nab)
> > - Fix upstream qemu conflict in hw/qdev-properties.c
> > - Make GET_ABI_VERSION use int (nab + mst)
> > - Fix vhost-scsi case lables in configure (reported by paolo)
> > - Convert qdev_prop_vhost_scsi to use ->get() + ->set() following
> >   qdev_prop_netdev (reported by paolo)
> > - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo)
> >
> > Changelog v0 -> v1:
> >
> > - Add VHOST_SCSI_SET_ENDPOINT call (stefan)
> > - Enable vhost notifiers for multiple queues (Zhi)
> > - clear vhost-scsi endpoint on stopped (Zhi)
> > - Add CONFIG_VHOST_SCSI for QEMU build configure (nab)
> > - Rename vhost_vring_target -> vhost_scsi_target (mst + nab)
> > - Add support for VHOST_SCSI_GET_ABI_VERSION ioctl (aliguori + nab)
> >
> > Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> > Cc: Anthony Liguori <aliguori@us.ibm.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > ---
> >  configure            |   10 +++
> >  hw/Makefile.objs     |    1 +
> >  hw/qdev-properties.c |   40 ++++++++++++
> >  hw/qdev.h            |    3 +
> >  hw/vhost-scsi.c      |  170 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  hw/vhost-scsi.h      |   50 +++++++++++++++
> >  qemu-common.h        |    1 +
> >  qemu-config.c        |   16 +++++
> >  qemu-options.hx      |    4 +
> >  vl.c                 |   18 +++++
> >  10 files changed, 313 insertions(+), 0 deletions(-)
> >  create mode 100644 hw/vhost-scsi.c
> >  create mode 100644 hw/vhost-scsi.h
> >

<SNIP>

> >
> > +/* --- vhost-scsi --- */
> > +
> > +static int parse_vhost_scsi_dev(DeviceState *dev, const char *str, void **ptr)
> > +{
> > +   VHostSCSI *p;
> > +
> > +   p = find_vhost_scsi(str);
> > +   if (p == NULL)
> > +       return -ENOENT;
> 
> Braces, please.
> 

Fixed

> > +
> > +   *ptr = p;
> > +   return 0;
> > +}
> > +
> > +static const char *print_vhost_scsi_dev(void *ptr)
> > +{
> > +    VHostSCSI *p = ptr;
> > +
> > +    return (p) ? vhost_scsi_get_id(p) : "<null>";
> > +}
> > +
> > +static void get_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> > +                       const char *name, Error **errp)
> > +{
> > +    get_pointer(obj, v, opaque, print_vhost_scsi_dev, name, errp);
> > +}
> > +
> > +static void set_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> > +                               const char *name, Error **errp)
> > +{
> > +    set_pointer(obj, v, opaque, parse_vhost_scsi_dev, name, errp);
> > +}
> > +
> > +PropertyInfo qdev_prop_vhost_scsi = {
> > +     .name = "vhost-scsi",
> > +     .get  = get_vhost_scsi_dev,
> > +     .set  = set_vhost_scsi_dev,
> > +};
> > +
> >  /* --- pointer --- */
> >
> >  /* Not a proper property, just for dirty hacks.  TODO Remove it!  */
> > diff --git a/hw/qdev.h b/hw/qdev.h
> > index d699194..d5873bb 100644
> > --- a/hw/qdev.h
> > +++ b/hw/qdev.h
> > @@ -238,6 +238,7 @@ extern PropertyInfo qdev_prop_vlan;
> >  extern PropertyInfo qdev_prop_pci_devfn;
> >  extern PropertyInfo qdev_prop_blocksize;
> >  extern PropertyInfo qdev_prop_pci_host_devaddr;
> > +extern PropertyInfo qdev_prop_vhost_scsi;
> >
> >  #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
> >          .name      = (_name),                                    \
> > @@ -305,6 +306,8 @@ extern PropertyInfo qdev_prop_pci_host_devaddr;
> >      DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
> >  #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
> >      DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
> > +#define DEFINE_PROP_VHOST_SCSI(_n, _s, _f)       \
> > +    DEFINE_PROP(_n, _s, _f, qdev_prop_vhost_scsi, VHostSCSI*)
> >
> >  #define DEFINE_PROP_END_OF_LIST()               \
> >      {}
> > diff --git a/hw/vhost-scsi.c b/hw/vhost-scsi.c
> > new file mode 100644
> > index 0000000..7145b2d
> > --- /dev/null
> > +++ b/hw/vhost-scsi.c
> > @@ -0,0 +1,170 @@
> > +/*
> > + * vhost_scsi host device
> > + *
> > + * Copyright IBM, Corp. 2011
> > + *
> > + * Authors:
> > + *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> > + * See the COPYING.LIB file in the top-level directory.
> > + *
> > + */
> > +
> > +#include <sys/ioctl.h>
> > +#include "config.h"
> > +#include "qemu-queue.h"
> > +#include "vhost-scsi.h"
> > +#include "vhost.h"
> > +
> > +struct VHostSCSI {
> > +    const char *id;
> > +    const char *wwpn;
> > +    uint16_t tpgt;
> > +    struct vhost_dev dev;
> > +    struct vhost_virtqueue vqs[3];
> > +    QLIST_ENTRY(VHostSCSI) list;
> > +};
> > +
> > +static QLIST_HEAD(, VHostSCSI) vhost_scsi_list =
> > +    QLIST_HEAD_INITIALIZER(vhost_scsi_list);
> > +
> > +VHostSCSI *find_vhost_scsi(const char *id)
> > +{
> > +    VHostSCSI *vs;
> > +
> > +    QLIST_FOREACH(vs, &vhost_scsi_list, list) {
> > +        if (strcmp(id, vs->id) == 0) {
> > +            return vs;
> > +        }
> > +    }
> > +    return NULL;
> > +}
> > +
> > +const char *vhost_scsi_get_id(VHostSCSI *vs)
> > +{
> > +    return vs->id;
> > +}
> > +
> > +int vhost_scsi_start(VHostSCSI *vs, VirtIODevice *vdev)
> > +{
> > +    int ret, abi_version;
> > +    struct vhost_scsi_target backend;
> > +
> > +    if (!vhost_dev_query(&vs->dev, vdev)) {
> > +        return -ENOTSUP;
> > +    }
> > +
> > +    vs->dev.nvqs = 3;
> > +    vs->dev.vqs = vs->vqs;
> > +
> > +    ret = vhost_dev_enable_notifiers(&vs->dev, vdev);
> > +    if (ret < 0) {
> > +        return ret;
> > +    }
> > +
> > +    ret = vhost_dev_start(&vs->dev, vdev);
> > +    if (ret < 0) {
> > +        return ret;
> > +    }
> > +
> > +    memset(&backend, 0, sizeof(backend));
> > +    ret = ioctl(vs->dev.control, VHOST_SCSI_GET_ABI_VERSION, &abi_version);
> > +    if (ret < 0) {
> > +        ret = -errno;
> > +        vhost_dev_stop(&vs->dev, vdev);
> > +        return ret;
> > +    }
> > +    if (abi_version > VHOST_SCSI_ABI_VERSION) {
> > +        fprintf(stderr, "The running tcm_vhost kernel abi_version: %d is greater"
> > +               " than vhost_scsi userspace supports: %d\n", abi_version,
> > +               VHOST_SCSI_ABI_VERSION);
> > +        ret = -ENOSYS;
> > +        vhost_dev_stop(&vs->dev, vdev);
> > +        return ret;
> > +    }
> > +    fprintf(stdout, "TCM_vHost ABI version: %d\n", abi_version);
> > +
> > +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
> 
> Please change vhost_wwpn to plain char *, then the cast can be removed.
> 

<nod>, changed to char *, and updating tcm_vhost on the kernel side to
do the same.

> > +    backend.vhost_tpgt = vs->tpgt;
> > +    ret = ioctl(vs->dev.control, VHOST_SCSI_SET_ENDPOINT, &backend);
> > +    if (ret < 0) {
> > +        ret = -errno;
> > +        vhost_dev_stop(&vs->dev, vdev);
> > +        return ret;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev)
> > +{
> > +    int ret;
> > +    struct vhost_scsi_target backend;
> > +
> > +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
> 
> Also here.
> 

Done

Thanks for your review Blue!

--nab

Re: [Qemu-devel] [RFC-v2 6/6] virtio-scsi: Fix incorrect VirtIOSCSI->cmd_vqs[0] definition

[Paolo Bonzini]

Il 14/08/2012 22:20, Nicholas A. Bellinger ha scritto:
>>> > > Since virtio_scsi currently assumes a single vqs for data, this patch
>>> > > simply changes ->cmd_vqs[1] to handle the single VirtQueue.

Wrong, multiqueue works just fine. :)  It's just the kernel driver that
doesn't support it yet.

>>> > > Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> > > Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>>> > > Cc: Michael S. Tsirkin <mst@redhat.com>
>>> > > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
>> > 
>> > This is a bugfix we need even without vhost, right?
>> > 
> I believe so, as it appears to be stomping past the end of memory for
> every virtio-scsi initialization regardless of vhost usage.. 

You just did a wrong merge.  When commit d2ad7dd (virtio-scsi: add
multiqueue capability, 2012-04-06) changed cmd_vq from pointer to array
of pointers, you should have moved the following fields to the middle of
the struct, just like that commit did.

Paolo

Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost

[Michael S. Tsirkin]

On Tue, Aug 14, 2012 at 02:12:29PM -0700, Nicholas A. Bellinger wrote:
> On Mon, 2012-08-13 at 11:59 +0300, Michael S. Tsirkin wrote:
> > On Mon, Aug 13, 2012 at 08:35:14AM +0000, Nicholas A. Bellinger wrote:
> > > From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > > 
> > > This patch adds a new type of host device that drives the vhost_scsi
> > > device.  The syntax to add vhost-scsi is:
> > > 
> > >   qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123
> > > 
> > > The virtio-scsi emulated device will make use of vhost-scsi to process
> > > virtio-scsi requests inside the kernel and hand them to the in-kernel
> > > SCSI target stack using the tcm_vhost fabric driver.
> > > 
> > > The tcm_vhost driver was merged into the upstream linux kernel for 3.6-rc2,
> > > and the commit can be found here:
> > > 
> > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297
> > > 
> > > Changelog v1 -> v2:
> > > 
> > > - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as
> > >   starting point for v3.6-rc code (Stefan + ALiguori + nab)
> > > - Fix upstream qemu conflict in hw/qdev-properties.c
> > > - Make GET_ABI_VERSION use int (nab + mst)
> > > - Fix vhost-scsi case lables in configure (reported by paolo)
> > > - Convert qdev_prop_vhost_scsi to use ->get() + ->set() following
> > >   qdev_prop_netdev (reported by paolo)
> > > - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo)
> > > 
> > > Changelog v0 -> v1:
> > > 
> > > - Add VHOST_SCSI_SET_ENDPOINT call (stefan)
> > > - Enable vhost notifiers for multiple queues (Zhi)
> > > - clear vhost-scsi endpoint on stopped (Zhi)
> > > - Add CONFIG_VHOST_SCSI for QEMU build configure (nab)
> > > - Rename vhost_vring_target -> vhost_scsi_target (mst + nab)
> > > - Add support for VHOST_SCSI_GET_ABI_VERSION ioctl (aliguori + nab)
> > > 
> > > Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > > Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> > > Cc: Anthony Liguori <aliguori@us.ibm.com>
> > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > 
> > 
> > Sent mail too fast, sorry. More comments below.
> > 
> > > ---
> > >  configure            |   10 +++
> > >  hw/Makefile.objs     |    1 +
> > >  hw/qdev-properties.c |   40 ++++++++++++
> > >  hw/qdev.h            |    3 +
> > >  hw/vhost-scsi.c      |  170 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  hw/vhost-scsi.h      |   50 +++++++++++++++
> > >  qemu-common.h        |    1 +
> > >  qemu-config.c        |   16 +++++
> > >  qemu-options.hx      |    4 +
> > >  vl.c                 |   18 +++++
> > >  10 files changed, 313 insertions(+), 0 deletions(-)
> > >  create mode 100644 hw/vhost-scsi.c
> > >  create mode 100644 hw/vhost-scsi.h
> > > 
> > > diff --git a/configure b/configure
> > > index f0dbc03..1f03202 100755
> > > --- a/configure
> > > +++ b/configure
> > > @@ -168,6 +168,7 @@ libattr=""
> > >  xfs=""
> > >  
> > >  vhost_net="no"
> > > +vhost_scsi="no"
> > >  kvm="no"
> > >  gprof="no"
> > >  debug_tcg="no"
> > > @@ -513,6 +514,7 @@ Haiku)
> > >    usb="linux"
> > >    kvm="yes"
> > >    vhost_net="yes"
> > > +  vhost_scsi="yes"
> > >    if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
> > >      audio_possible_drivers="$audio_possible_drivers fmod"
> > >    fi
> > > @@ -818,6 +820,10 @@ for opt do
> > >    ;;
> > >    --enable-vhost-net) vhost_net="yes"
> > >    ;;
> > > +  --disable-vhost-scsi) vhost_scsi="no"
> > > +  ;;
> > > +  --enable-vhost-scsi) vhost_scsi="yes"
> > > +  ;;
> > >    --disable-opengl) opengl="no"
> > >    ;;
> > >    --enable-opengl) opengl="yes"
> > > @@ -3116,6 +3122,7 @@ echo "posix_madvise     $posix_madvise"
> > >  echo "uuid support      $uuid"
> > >  echo "libcap-ng support $cap_ng"
> > >  echo "vhost-net support $vhost_net"
> > > +echo "vhost-scsi support $vhost_scsi"
> > >  echo "Trace backend     $trace_backend"
> > >  echo "Trace output file $trace_file-<pid>"
> > >  echo "spice support     $spice"
> > > @@ -3828,6 +3835,9 @@ case "$target_arch2" in
> > >        if test "$vhost_net" = "yes" ; then
> > >          echo "CONFIG_VHOST_NET=y" >> $config_target_mak
> > >        fi
> > > +      if test "$vhost_scsi" = "yes" ; then
> > > +        echo "CONFIG_VHOST_SCSI=y" >> $config_target_mak
> > > +      fi
> > >      fi
> > >  esac
> > >  case "$target_arch2" in
> > > diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> > > index 3ba5dd0..6ab75ec 100644
> > > --- a/hw/Makefile.objs
> > > +++ b/hw/Makefile.objs
> > > @@ -169,6 +169,7 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o
> > >  obj-$(CONFIG_VIRTIO) += virtio-serial-bus.o virtio-scsi.o
> > >  obj-$(CONFIG_SOFTMMU) += vhost_net.o
> > >  obj-$(CONFIG_VHOST_NET) += vhost.o
> > > +obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o
> > >  obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/
> > >  obj-$(CONFIG_NO_PCI) += pci-stub.o
> > >  obj-$(CONFIG_VGA) += vga.o
> > > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> > > index 8aca0d4..0266266 100644
> > > --- a/hw/qdev-properties.c
> > > +++ b/hw/qdev-properties.c
> > > @@ -4,6 +4,7 @@
> > >  #include "blockdev.h"
> > >  #include "hw/block-common.h"
> > >  #include "net/hub.h"
> > > +#include "vhost-scsi.h"
> > >  
> > >  void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
> > >  {
> > > @@ -696,6 +697,45 @@ PropertyInfo qdev_prop_vlan = {
> > >      .set   = set_vlan,
> > >  };
> > >  
> > > +/* --- vhost-scsi --- */
> > > +
> > > +static int parse_vhost_scsi_dev(DeviceState *dev, const char *str, void **ptr)
> > > +{
> > > +   VHostSCSI *p;
> > > +
> > > +   p = find_vhost_scsi(str);
> > > +   if (p == NULL)
> > > +       return -ENOENT;
> > > +
> > > +   *ptr = p;
> > > +   return 0;
> > > +}
> > > +
> > > +static const char *print_vhost_scsi_dev(void *ptr)
> > > +{
> > > +    VHostSCSI *p = ptr;
> > > +
> > > +    return (p) ? vhost_scsi_get_id(p) : "<null>";
> > > +}
> > > +
> > > +static void get_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> > > +                       const char *name, Error **errp)
> > > +{
> > > +    get_pointer(obj, v, opaque, print_vhost_scsi_dev, name, errp);
> > > +}
> > > +
> > > +static void set_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> > > +                               const char *name, Error **errp)
> > > +{
> > > +    set_pointer(obj, v, opaque, parse_vhost_scsi_dev, name, errp);
> > > +}
> > > +
> > > +PropertyInfo qdev_prop_vhost_scsi = {
> > > +     .name = "vhost-scsi",
> > > +     .get  = get_vhost_scsi_dev,
> > > +     .set  = set_vhost_scsi_dev,
> > > +};
> > > +
> > >  /* --- pointer --- */
> > >  
> > >  /* Not a proper property, just for dirty hacks.  TODO Remove it!  */
> > 
> > Why does this make sense in the generic qdev-properties?
> > There's exactly one device that can use this, no?
> > 
> 
> Mmmm, not sure on this one either..  Stefan..?
> 
> > > diff --git a/hw/qdev.h b/hw/qdev.h
> > > index d699194..d5873bb 100644
> > > --- a/hw/qdev.h
> > > +++ b/hw/qdev.h
> > > @@ -238,6 +238,7 @@ extern PropertyInfo qdev_prop_vlan;
> > >  extern PropertyInfo qdev_prop_pci_devfn;
> > >  extern PropertyInfo qdev_prop_blocksize;
> > >  extern PropertyInfo qdev_prop_pci_host_devaddr;
> > > +extern PropertyInfo qdev_prop_vhost_scsi;
> > >  
> > >  #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
> > >          .name      = (_name),                                    \
> > > @@ -305,6 +306,8 @@ extern PropertyInfo qdev_prop_pci_host_devaddr;
> > >      DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
> > >  #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
> > >      DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
> > > +#define DEFINE_PROP_VHOST_SCSI(_n, _s, _f)       \
> > > +    DEFINE_PROP(_n, _s, _f, qdev_prop_vhost_scsi, VHostSCSI*)
> > >
> > 
> > Can this move to vhost-scsi.c?
> >   
> 
> Done
> 
> > >  #define DEFINE_PROP_END_OF_LIST()               \
> > >      {}
> > > diff --git a/hw/vhost-scsi.c b/hw/vhost-scsi.c
> > > new file mode 100644
> > > index 0000000..7145b2d
> > > --- /dev/null
> > > +++ b/hw/vhost-scsi.c
> > > @@ -0,0 +1,170 @@
> > > +/*
> > > + * vhost_scsi host device
> > > + *
> > > + * Copyright IBM, Corp. 2011
> > > + *
> > > + * Authors:
> > > + *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
> > > + *
> > > + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> > > + * See the COPYING.LIB file in the top-level directory.
> > > + *
> > > + */
> > > +
> > > +#include <sys/ioctl.h>
> > > +#include "config.h"
> > > +#include "qemu-queue.h"
> > > +#include "vhost-scsi.h"
> > > +#include "vhost.h"
> > > +
> > > +struct VHostSCSI {
> > > +    const char *id;
> > > +    const char *wwpn;
> > > +    uint16_t tpgt;
> > > +    struct vhost_dev dev;
> > > +    struct vhost_virtqueue vqs[3];
> > 
> > Could you add enum for vq numbers pls?
> > 
> 
> Done
> 
> > > +    QLIST_ENTRY(VHostSCSI) list;
> > > +};
> > > +
> > > +static QLIST_HEAD(, VHostSCSI) vhost_scsi_list =
> > > +    QLIST_HEAD_INITIALIZER(vhost_scsi_list);
> > > +
> > > +VHostSCSI *find_vhost_scsi(const char *id)
> > > +{
> > > +    VHostSCSI *vs;
> > > +
> > > +    QLIST_FOREACH(vs, &vhost_scsi_list, list) {
> > > +        if (strcmp(id, vs->id) == 0) {
> > 
> > !strcmp
> > 
> 
> Done
> 
> > > +            return vs;
> > > +        }
> > > +    }
> > > +    return NULL;
> > > +}
> > > +
> > > +const char *vhost_scsi_get_id(VHostSCSI *vs)
> > > +{
> > > +    return vs->id;
> > > +}
> > > +
> > > +int vhost_scsi_start(VHostSCSI *vs, VirtIODevice *vdev)
> > > +{
> > > +    int ret, abi_version;
> > > +    struct vhost_scsi_target backend;
> > > +
> > > +    if (!vhost_dev_query(&vs->dev, vdev)) {
> > > +        return -ENOTSUP;
> > > +    }
> > > +
> > > +    vs->dev.nvqs = 3;
> > > +    vs->dev.vqs = vs->vqs;
> > > +
> > > +    ret = vhost_dev_enable_notifiers(&vs->dev, vdev);
> > > +    if (ret < 0) {
> > > +        return ret;
> > > +    }
> > > +
> > > +    ret = vhost_dev_start(&vs->dev, vdev);
> > > +    if (ret < 0) {
> > > +        return ret;
> > > +    }
> > > +
> > > +    memset(&backend, 0, sizeof(backend));
> > > +    ret = ioctl(vs->dev.control, VHOST_SCSI_GET_ABI_VERSION, &abi_version);
> > > +    if (ret < 0) {
> > > +        ret = -errno;
> > > +        vhost_dev_stop(&vs->dev, vdev);
> > > +        return ret;
> > > +    }
> > > +    if (abi_version > VHOST_SCSI_ABI_VERSION) {
> > > +        fprintf(stderr, "The running tcm_vhost kernel abi_version: %d is greater"
> > > +		" than vhost_scsi userspace supports: %d\n", abi_version,
> > > +		VHOST_SCSI_ABI_VERSION);
> > > +        ret = -ENOSYS;
> > > +        vhost_dev_stop(&vs->dev, vdev);
> > > +        return ret;
> > > +    }
> > > +    fprintf(stdout, "TCM_vHost ABI version: %d\n", abi_version);
> > > +
> > > +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
> > > +    backend.vhost_tpgt = vs->tpgt;
> > > +    ret = ioctl(vs->dev.control, VHOST_SCSI_SET_ENDPOINT, &backend);
> > > +    if (ret < 0) {
> > > +        ret = -errno;
> > > +        vhost_dev_stop(&vs->dev, vdev);
> > > +        return ret;
> > > +    }
> > > +
> > > +    return 0;
> > > +}
> > > +
> > > +void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev)
> > > +{
> > > +    int ret;
> > > +    struct vhost_scsi_target backend;
> > > +
> > > +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
> > > +    backend.vhost_tpgt = vs->tpgt;
> > > +    ret = ioctl(vs->dev.control, VHOST_SCSI_CLEAR_ENDPOINT, &backend);
> > > +    if (ret < 0) {
> > > +        fprintf(stderr, "Failed to clear endpoint\n");
> > > +    }
> > > +
> > > +    vhost_dev_stop(&vs->dev, vdev);
> > > +}
> > > +
> > > +static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
> > > +                                 uint16_t tpgt)
> > > +{
> > > +    VHostSCSI *vs = g_malloc0(sizeof(*vs));
> > > +    int ret;
> > > +
> > > +    /* TODO set up vhost-scsi device and bind to tcm_vhost/$wwpm/tpgt_$tpgt */
> > > +    fprintf(stderr, "wwpn = \"%s\" tpgt = \"%u\"\n", id, tpgt);
> > > +
> > 
> > Please do not keep debugging fprintfs around.
> > 
> 
> Dropped
> 
> > > +    ret = vhost_dev_init(&vs->dev, -1, "/dev/vhost-scsi", false);
> > 
> > commented on this separately
> > 
> 
> ...
> 
> > > +    if (ret < 0) {
> > > +        fprintf(stderr, "vhost-scsi: vhost initialization failed: %s\n",
> > > +                strerror(-ret));
> > 
> > errors should go to monitor, here and elsewhere.
> > 
> 
> I think this means using monitor_printf() right..?
> 
> Looking at that now..


error_report is handier.

Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost

[Michael S. Tsirkin]

On Tue, Aug 14, 2012 at 01:31:14PM -0700, Nicholas A. Bellinger wrote:
> On Mon, 2012-08-13 at 11:53 +0300, Michael S. Tsirkin wrote:
> > On Mon, Aug 13, 2012 at 08:35:14AM +0000, Nicholas A. Bellinger wrote:
> > > From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > > 
> > > This patch adds a new type of host device that drives the vhost_scsi
> > > device.  The syntax to add vhost-scsi is:
> > > 
> > >   qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123
> > > 
> > > The virtio-scsi emulated device will make use of vhost-scsi to process
> > > virtio-scsi requests inside the kernel and hand them to the in-kernel
> > > SCSI target stack using the tcm_vhost fabric driver.
> 
> <SNIP>
> 
> > > +static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
> > > +                                 uint16_t tpgt)
> > > +{
> > > +    VHostSCSI *vs = g_malloc0(sizeof(*vs));
> > > +    int ret;
> > > +
> > > +    /* TODO set up vhost-scsi device and bind to tcm_vhost/$wwpm/tpgt_$tpgt */
> > > +    fprintf(stderr, "wwpn = \"%s\" tpgt = \"%u\"\n", id, tpgt);
> > > +
> > > +    ret = vhost_dev_init(&vs->dev, -1, "/dev/vhost-scsi", false);
> > 
> > This -1 is a hack. You need to support passing in fd from
> > the monitor, and pass it here.
> > 
> 
> Mmm, looking at how vhost_net_init + tap.c does this, but am not quite
> what fd needs to be propagated up for virtio-scsi -> vhost-scsi..
> 
> Can you please elaborate on this one a bit more..?
> 
> --nab
> 


The idea is to allow running as a user without access to
/dev/vhost-scsi.
For this, allow passing in the fd of /dev/vhost-scsi through unix domain sockets.

-- 
MST

Re: [Qemu-devel] [RFC-v2 6/6] virtio-scsi: Fix incorrect VirtIOSCSI->cmd_vqs[0] definition

[Nicholas A. Bellinger]

On Sat, 2012-08-18 at 20:52 +0200, Paolo Bonzini wrote:
> Il 14/08/2012 22:20, Nicholas A. Bellinger ha scritto:
> >>> > > Since virtio_scsi currently assumes a single vqs for data, this patch
> >>> > > simply changes ->cmd_vqs[1] to handle the single VirtQueue.
> 
> Wrong, multiqueue works just fine. :)  It's just the kernel driver that
> doesn't support it yet.
> 

<nod>

> >>> > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> >>> > > Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> >>> > > Cc: Michael S. Tsirkin <mst@redhat.com>
> >>> > > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> >> > 
> >> > This is a bugfix we need even without vhost, right?
> >> > 
> > I believe so, as it appears to be stomping past the end of memory for
> > every virtio-scsi initialization regardless of vhost usage.. 
> 
> You just did a wrong merge.  When commit d2ad7dd (virtio-scsi: add
> multiqueue capability, 2012-04-06) changed cmd_vq from pointer to array
> of pointers, you should have moved the following fields to the middle of
> the struct, just like that commit did.

Ahh, I see how virtio_scsi_init() -> virtio_common_init() are setting up
the memory now..  Apologies, my mistake.

So moving the vhost-scsi related structure members ahead of the
VirtQueue releated definitions for RFC-v3, and dropping this patch.

Thanks Paolo!

--nab

Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost

[Nicholas A. Bellinger]

On Sat, 2012-08-18 at 22:10 +0300, Michael S. Tsirkin wrote:
> On Tue, Aug 14, 2012 at 02:12:29PM -0700, Nicholas A. Bellinger wrote:
> > On Mon, 2012-08-13 at 11:59 +0300, Michael S. Tsirkin wrote:
> > > On Mon, Aug 13, 2012 at 08:35:14AM +0000, Nicholas A. Bellinger wrote:

<SNIP>

> > 
> > > > +    if (ret < 0) {
> > > > +        fprintf(stderr, "vhost-scsi: vhost initialization failed: %s\n",
> > > > +                strerror(-ret));
> > > 
> > > errors should go to monitor, here and elsewhere.
> > > 
> > 
> > I think this means using monitor_printf() right..?
> > 
> > Looking at that now..
> 
> 
> error_report is handier.
> 

Converted all fprintf(stderr, ...) -> error_report() usage for RFC-v3.

Thanks MST!

Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost

[Nicholas A. Bellinger]

On Sat, 2012-08-18 at 22:12 +0300, Michael S. Tsirkin wrote:
> On Tue, Aug 14, 2012 at 01:31:14PM -0700, Nicholas A. Bellinger wrote:
> > On Mon, 2012-08-13 at 11:53 +0300, Michael S. Tsirkin wrote:
> > > On Mon, Aug 13, 2012 at 08:35:14AM +0000, Nicholas A. Bellinger wrote:
> > > > From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

<SNIP>

> > > > +static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
> > > > +                                 uint16_t tpgt)
> > > > +{
> > > > +    VHostSCSI *vs = g_malloc0(sizeof(*vs));
> > > > +    int ret;
> > > > +
> > > > +    /* TODO set up vhost-scsi device and bind to tcm_vhost/$wwpm/tpgt_$tpgt */
> > > > +    fprintf(stderr, "wwpn = \"%s\" tpgt = \"%u\"\n", id, tpgt);
> > > > +
> > > > +    ret = vhost_dev_init(&vs->dev, -1, "/dev/vhost-scsi", false);
> > > 
> > > This -1 is a hack. You need to support passing in fd from
> > > the monitor, and pass it here.
> > > 
> > 
> > Mmm, looking at how vhost_net_init + tap.c does this, but am not quite
> > what fd needs to be propagated up for virtio-scsi -> vhost-scsi..
> > 
> > Can you please elaborate on this one a bit more..?
> > 
> 
> The idea is to allow running as a user without access to
> /dev/vhost-scsi.
> For this, allow passing in the fd of /dev/vhost-scsi through unix domain sockets.
> 

Ah, that is a pretty neat trick..   So for vhost-scsi code, this would
mean something along the lines of the following, yes..?

Thanks MST!

diff --git a/hw/vhost-scsi.c b/hw/vhost-scsi.c
index 4206a75..8af8758 100644
--- a/hw/vhost-scsi.c
+++ b/hw/vhost-scsi.c
@@ -21,6 +21,7 @@ struct VHostSCSI {
     const char *id;
     const char *wwpn;
     uint16_t tpgt;
+    int vhostfd;
     struct vhost_dev dev;
     struct vhost_virtqueue vqs[VHOST_SCSI_VQ_NUM];
     QLIST_ENTRY(VHostSCSI) list;
@@ -114,13 +115,32 @@ void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev)
 }
 
 static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
-                                 uint16_t tpgt)
+                                 uint16_t tpgt, const char *vhostfd_str)
 {
-    VHostSCSI *vs = g_malloc0(sizeof(*vs));
+    VHostSCSI *vs;
     int ret;
 
+    vs = g_malloc0(sizeof(*vs));
+    if (!vs) {
+        error_report("vhost-scsi: unable to allocate *vs\n");
+        return NULL;
+    }
+    vs->vhostfd = -1;
+
+    if (vhostfd_str) {
+        if (!qemu_isdigit(vhostfd_str[0])) {
+            error_report("vhost-scsi: passed vhostfd value is not a digit\n");
+            return NULL;
+        }
+
+        vs->vhostfd = qemu_parse_fd(vhostfd_str);
+        if (vs->vhostfd == -1) {
+            error_report("vhost-scsi: unable to parse vs->vhostfd\n");
+            return NULL;
+        }
+    }
     /* TODO set up vhost-scsi device and bind to tcm_vhost/$wwpm/tpgt_$tpgt */
-    ret = vhost_dev_init(&vs->dev, -1, "/dev/vhost-scsi", false);
+    ret = vhost_dev_init(&vs->dev, vs->vhostfd, "/dev/vhost-scsi", false);
     if (ret < 0) {
         error_report("vhost-scsi: vhost initialization failed: %s\n",
                 strerror(-ret));
@@ -140,7 +160,7 @@ static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
 VHostSCSI *vhost_scsi_add_opts(QemuOpts *opts)
 {
     const char *id;
-    const char *wwpn;
+    const char *wwpn, *vhostfd;
     uint64_t tpgt;
 
     id = qemu_opts_id(opts);
@@ -164,6 +184,7 @@ VHostSCSI *vhost_scsi_add_opts(QemuOpts *opts)
         error_report("vhost-scsi: \"%s\" needs a 16-bit tpgt\n", id);
         return NULL;
     }
+    vhostfd = qemu_opt_get(opts, "vhostfd");
 
-    return vhost_scsi_add(id, wwpn, tpgt);
+    return vhost_scsi_add(id, wwpn, tpgt, vhostfd);
 }
diff --git a/qemu-config.c b/qemu-config.c
index 33399ea..2d4884c 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -636,6 +636,9 @@ QemuOptsList qemu_vhost_scsi_opts = {
         }, {
             .name = "tpgt",
             .type = QEMU_OPT_NUMBER,
+        }, {
+            .name = "vhostfd",
+            .type = QEMU_OPT_STRING,
         },
         { /* end of list */ }
     },

Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost

[Michael S. Tsirkin]

On Sat, Aug 18, 2012 at 05:36:26PM -0700, Nicholas A. Bellinger wrote:
> On Sat, 2012-08-18 at 22:12 +0300, Michael S. Tsirkin wrote:
> > On Tue, Aug 14, 2012 at 01:31:14PM -0700, Nicholas A. Bellinger wrote:
> > > On Mon, 2012-08-13 at 11:53 +0300, Michael S. Tsirkin wrote:
> > > > On Mon, Aug 13, 2012 at 08:35:14AM +0000, Nicholas A. Bellinger wrote:
> > > > > From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> 
> <SNIP>
> 
> > > > > +static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
> > > > > +                                 uint16_t tpgt)
> > > > > +{
> > > > > +    VHostSCSI *vs = g_malloc0(sizeof(*vs));
> > > > > +    int ret;
> > > > > +
> > > > > +    /* TODO set up vhost-scsi device and bind to tcm_vhost/$wwpm/tpgt_$tpgt */
> > > > > +    fprintf(stderr, "wwpn = \"%s\" tpgt = \"%u\"\n", id, tpgt);
> > > > > +
> > > > > +    ret = vhost_dev_init(&vs->dev, -1, "/dev/vhost-scsi", false);
> > > > 
> > > > This -1 is a hack. You need to support passing in fd from
> > > > the monitor, and pass it here.
> > > > 
> > > 
> > > Mmm, looking at how vhost_net_init + tap.c does this, but am not quite
> > > what fd needs to be propagated up for virtio-scsi -> vhost-scsi..
> > > 
> > > Can you please elaborate on this one a bit more..?
> > > 
> > 
> > The idea is to allow running as a user without access to
> > /dev/vhost-scsi.
> > For this, allow passing in the fd of /dev/vhost-scsi through unix domain sockets.
> > 
> 
> Ah, that is a pretty neat trick..   So for vhost-scsi code, this would
> mean something along the lines of the following, yes..?

Yes but with one correction. See below.

> Thanks MST!

> diff --git a/hw/vhost-scsi.c b/hw/vhost-scsi.c
> index 4206a75..8af8758 100644
> --- a/hw/vhost-scsi.c
> +++ b/hw/vhost-scsi.c
> @@ -21,6 +21,7 @@ struct VHostSCSI {
>      const char *id;
>      const char *wwpn;
>      uint16_t tpgt;
> +    int vhostfd;
>      struct vhost_dev dev;
>      struct vhost_virtqueue vqs[VHOST_SCSI_VQ_NUM];
>      QLIST_ENTRY(VHostSCSI) list;
> @@ -114,13 +115,32 @@ void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev)
>  }
>  
>  static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
> -                                 uint16_t tpgt)
> +                                 uint16_t tpgt, const char *vhostfd_str)
>  {
> -    VHostSCSI *vs = g_malloc0(sizeof(*vs));
> +    VHostSCSI *vs;
>      int ret;
>  
> +    vs = g_malloc0(sizeof(*vs));
> +    if (!vs) {
> +        error_report("vhost-scsi: unable to allocate *vs\n");
> +        return NULL;
> +    }
> +    vs->vhostfd = -1;
> +
> +    if (vhostfd_str) {
> +        if (!qemu_isdigit(vhostfd_str[0])) {
> +            error_report("vhost-scsi: passed vhostfd value is not a digit\n");
> +            return NULL;

This let you use an fd which was open at exec
but does not allow for fd to be open later in
case device is hot-plugged.

See net_handle_fd_param - I think you can just rename it
qemu_handle_fd_param to avoid code duplication.

> +        }
> +
> +        vs->vhostfd = qemu_parse_fd(vhostfd_str);
> +        if (vs->vhostfd == -1) {
> +            error_report("vhost-scsi: unable to parse vs->vhostfd\n");
> +            return NULL;
> +        }
> +    }
>      /* TODO set up vhost-scsi device and bind to tcm_vhost/$wwpm/tpgt_$tpgt */
> -    ret = vhost_dev_init(&vs->dev, -1, "/dev/vhost-scsi", false);
> +    ret = vhost_dev_init(&vs->dev, vs->vhostfd, "/dev/vhost-scsi", false);
>      if (ret < 0) {
>          error_report("vhost-scsi: vhost initialization failed: %s\n",
>                  strerror(-ret));
> @@ -140,7 +160,7 @@ static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
>  VHostSCSI *vhost_scsi_add_opts(QemuOpts *opts)
>  {
>      const char *id;
> -    const char *wwpn;
> +    const char *wwpn, *vhostfd;
>      uint64_t tpgt;
>  
>      id = qemu_opts_id(opts);
> @@ -164,6 +184,7 @@ VHostSCSI *vhost_scsi_add_opts(QemuOpts *opts)
>          error_report("vhost-scsi: \"%s\" needs a 16-bit tpgt\n", id);
>          return NULL;
>      }
> +    vhostfd = qemu_opt_get(opts, "vhostfd");
>  
> -    return vhost_scsi_add(id, wwpn, tpgt);
> +    return vhost_scsi_add(id, wwpn, tpgt, vhostfd);
>  }
> diff --git a/qemu-config.c b/qemu-config.c
> index 33399ea..2d4884c 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -636,6 +636,9 @@ QemuOptsList qemu_vhost_scsi_opts = {
>          }, {
>              .name = "tpgt",
>              .type = QEMU_OPT_NUMBER,
> +        }, {
> +            .name = "vhostfd",
> +            .type = QEMU_OPT_STRING,
>          },
>          { /* end of list */ }
>      },

Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost

[Paolo Bonzini]

Il 13/08/2012 10:35, Nicholas A. Bellinger ha scritto:
> From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> 
> This patch adds a new type of host device that drives the vhost_scsi
> device.  The syntax to add vhost-scsi is:
> 
>   qemu -vhost-scsi id=vhost-scsi0,wwpn=...,tpgt=123
> 
> The virtio-scsi emulated device will make use of vhost-scsi to process
> virtio-scsi requests inside the kernel and hand them to the in-kernel
> SCSI target stack using the tcm_vhost fabric driver.
> 
> The tcm_vhost driver was merged into the upstream linux kernel for 3.6-rc2,
> and the commit can be found here:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=057cbf49a1f08297
> 
> Changelog v1 -> v2:
> 
> - Expose ABI version via VHOST_SCSI_GET_ABI_VERSION + use Rev 0 as
>   starting point for v3.6-rc code (Stefan + ALiguori + nab)
> - Fix upstream qemu conflict in hw/qdev-properties.c
> - Make GET_ABI_VERSION use int (nab + mst)
> - Fix vhost-scsi case lables in configure (reported by paolo)
> - Convert qdev_prop_vhost_scsi to use ->get() + ->set() following
>   qdev_prop_netdev (reported by paolo)
> - Fix typo in qemu-options.hx definition of vhost-scsi (reported by paolo)
> 
> Changelog v0 -> v1:
> 
> - Add VHOST_SCSI_SET_ENDPOINT call (stefan)
> - Enable vhost notifiers for multiple queues (Zhi)
> - clear vhost-scsi endpoint on stopped (Zhi)
> - Add CONFIG_VHOST_SCSI for QEMU build configure (nab)
> - Rename vhost_vring_target -> vhost_scsi_target (mst + nab)
> - Add support for VHOST_SCSI_GET_ABI_VERSION ioctl (aliguori + nab)
> 
> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>  configure            |   10 +++
>  hw/Makefile.objs     |    1 +
>  hw/qdev-properties.c |   40 ++++++++++++
>  hw/qdev.h            |    3 +
>  hw/vhost-scsi.c      |  170 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/vhost-scsi.h      |   50 +++++++++++++++
>  qemu-common.h        |    1 +
>  qemu-config.c        |   16 +++++
>  qemu-options.hx      |    4 +
>  vl.c                 |   18 +++++
>  10 files changed, 313 insertions(+), 0 deletions(-)
>  create mode 100644 hw/vhost-scsi.c
>  create mode 100644 hw/vhost-scsi.h
> 
> diff --git a/configure b/configure
> index f0dbc03..1f03202 100755
> --- a/configure
> +++ b/configure
> @@ -168,6 +168,7 @@ libattr=""
>  xfs=""
>  
>  vhost_net="no"
> +vhost_scsi="no"
>  kvm="no"
>  gprof="no"
>  debug_tcg="no"
> @@ -513,6 +514,7 @@ Haiku)
>    usb="linux"
>    kvm="yes"
>    vhost_net="yes"
> +  vhost_scsi="yes"
>    if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
>      audio_possible_drivers="$audio_possible_drivers fmod"
>    fi
> @@ -818,6 +820,10 @@ for opt do
>    ;;
>    --enable-vhost-net) vhost_net="yes"
>    ;;
> +  --disable-vhost-scsi) vhost_scsi="no"
> +  ;;
> +  --enable-vhost-scsi) vhost_scsi="yes"
> +  ;;
>    --disable-opengl) opengl="no"
>    ;;
>    --enable-opengl) opengl="yes"
> @@ -3116,6 +3122,7 @@ echo "posix_madvise     $posix_madvise"
>  echo "uuid support      $uuid"
>  echo "libcap-ng support $cap_ng"
>  echo "vhost-net support $vhost_net"
> +echo "vhost-scsi support $vhost_scsi"
>  echo "Trace backend     $trace_backend"
>  echo "Trace output file $trace_file-<pid>"
>  echo "spice support     $spice"
> @@ -3828,6 +3835,9 @@ case "$target_arch2" in
>        if test "$vhost_net" = "yes" ; then
>          echo "CONFIG_VHOST_NET=y" >> $config_target_mak
>        fi
> +      if test "$vhost_scsi" = "yes" ; then
> +        echo "CONFIG_VHOST_SCSI=y" >> $config_target_mak
> +      fi
>      fi
>  esac
>  case "$target_arch2" in
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 3ba5dd0..6ab75ec 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -169,6 +169,7 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o
>  obj-$(CONFIG_VIRTIO) += virtio-serial-bus.o virtio-scsi.o
>  obj-$(CONFIG_SOFTMMU) += vhost_net.o
>  obj-$(CONFIG_VHOST_NET) += vhost.o
> +obj-$(CONFIG_VHOST_SCSI) += vhost-scsi.o
>  obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/
>  obj-$(CONFIG_NO_PCI) += pci-stub.o
>  obj-$(CONFIG_VGA) += vga.o
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 8aca0d4..0266266 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -4,6 +4,7 @@
>  #include "blockdev.h"
>  #include "hw/block-common.h"
>  #include "net/hub.h"
> +#include "vhost-scsi.h"
>  
>  void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
>  {
> @@ -696,6 +697,45 @@ PropertyInfo qdev_prop_vlan = {
>      .set   = set_vlan,
>  };
>  
> +/* --- vhost-scsi --- */
> +
> +static int parse_vhost_scsi_dev(DeviceState *dev, const char *str, void **ptr)
> +{
> +   VHostSCSI *p;
> +
> +   p = find_vhost_scsi(str);
> +   if (p == NULL)
> +       return -ENOENT;
> +
> +   *ptr = p;
> +   return 0;
> +}
> +
> +static const char *print_vhost_scsi_dev(void *ptr)
> +{
> +    VHostSCSI *p = ptr;
> +
> +    return (p) ? vhost_scsi_get_id(p) : "<null>";
> +}
> +
> +static void get_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> +                       const char *name, Error **errp)
> +{
> +    get_pointer(obj, v, opaque, print_vhost_scsi_dev, name, errp);
> +}
> +
> +static void set_vhost_scsi_dev(Object *obj, Visitor *v, void *opaque,
> +                               const char *name, Error **errp)
> +{
> +    set_pointer(obj, v, opaque, parse_vhost_scsi_dev, name, errp);
> +}
> +
> +PropertyInfo qdev_prop_vhost_scsi = {
> +     .name = "vhost-scsi",
> +     .get  = get_vhost_scsi_dev,
> +     .set  = set_vhost_scsi_dev,
> +};
> +
>  /* --- pointer --- */
>  
>  /* Not a proper property, just for dirty hacks.  TODO Remove it!  */
> diff --git a/hw/qdev.h b/hw/qdev.h
> index d699194..d5873bb 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -238,6 +238,7 @@ extern PropertyInfo qdev_prop_vlan;
>  extern PropertyInfo qdev_prop_pci_devfn;
>  extern PropertyInfo qdev_prop_blocksize;
>  extern PropertyInfo qdev_prop_pci_host_devaddr;
> +extern PropertyInfo qdev_prop_vhost_scsi;
>  
>  #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
>          .name      = (_name),                                    \
> @@ -305,6 +306,8 @@ extern PropertyInfo qdev_prop_pci_host_devaddr;
>      DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
>  #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
>      DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
> +#define DEFINE_PROP_VHOST_SCSI(_n, _s, _f)       \
> +    DEFINE_PROP(_n, _s, _f, qdev_prop_vhost_scsi, VHostSCSI*)
>  
>  #define DEFINE_PROP_END_OF_LIST()               \
>      {}
> diff --git a/hw/vhost-scsi.c b/hw/vhost-scsi.c
> new file mode 100644
> index 0000000..7145b2d
> --- /dev/null
> +++ b/hw/vhost-scsi.c
> @@ -0,0 +1,170 @@
> +/*
> + * vhost_scsi host device
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include <sys/ioctl.h>
> +#include "config.h"
> +#include "qemu-queue.h"
> +#include "vhost-scsi.h"
> +#include "vhost.h"
> +
> +struct VHostSCSI {
> +    const char *id;
> +    const char *wwpn;
> +    uint16_t tpgt;
> +    struct vhost_dev dev;
> +    struct vhost_virtqueue vqs[3];
> +    QLIST_ENTRY(VHostSCSI) list;
> +};
> +
> +static QLIST_HEAD(, VHostSCSI) vhost_scsi_list =
> +    QLIST_HEAD_INITIALIZER(vhost_scsi_list);
> +
> +VHostSCSI *find_vhost_scsi(const char *id)
> +{
> +    VHostSCSI *vs;
> +
> +    QLIST_FOREACH(vs, &vhost_scsi_list, list) {
> +        if (strcmp(id, vs->id) == 0) {
> +            return vs;
> +        }
> +    }
> +    return NULL;
> +}
> +
> +const char *vhost_scsi_get_id(VHostSCSI *vs)
> +{
> +    return vs->id;
> +}
> +
> +int vhost_scsi_start(VHostSCSI *vs, VirtIODevice *vdev)
> +{
> +    int ret, abi_version;
> +    struct vhost_scsi_target backend;
> +
> +    if (!vhost_dev_query(&vs->dev, vdev)) {
> +        return -ENOTSUP;
> +    }
> +
> +    vs->dev.nvqs = 3;
> +    vs->dev.vqs = vs->vqs;
> +
> +    ret = vhost_dev_enable_notifiers(&vs->dev, vdev);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    ret = vhost_dev_start(&vs->dev, vdev);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    memset(&backend, 0, sizeof(backend));
> +    ret = ioctl(vs->dev.control, VHOST_SCSI_GET_ABI_VERSION, &abi_version);
> +    if (ret < 0) {
> +        ret = -errno;
> +        vhost_dev_stop(&vs->dev, vdev);
> +        return ret;
> +    }
> +    if (abi_version > VHOST_SCSI_ABI_VERSION) {
> +        fprintf(stderr, "The running tcm_vhost kernel abi_version: %d is greater"
> +		" than vhost_scsi userspace supports: %d\n", abi_version,
> +		VHOST_SCSI_ABI_VERSION);
> +        ret = -ENOSYS;
> +        vhost_dev_stop(&vs->dev, vdev);
> +        return ret;
> +    }
> +    fprintf(stdout, "TCM_vHost ABI version: %d\n", abi_version);
> +
> +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
> +    backend.vhost_tpgt = vs->tpgt;
> +    ret = ioctl(vs->dev.control, VHOST_SCSI_SET_ENDPOINT, &backend);
> +    if (ret < 0) {
> +        ret = -errno;
> +        vhost_dev_stop(&vs->dev, vdev);
> +        return ret;
> +    }
> +
> +    return 0;
> +}
> +
> +void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev)
> +{
> +    int ret;
> +    struct vhost_scsi_target backend;
> +
> +    pstrcpy((char *)backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->wwpn);
> +    backend.vhost_tpgt = vs->tpgt;
> +    ret = ioctl(vs->dev.control, VHOST_SCSI_CLEAR_ENDPOINT, &backend);
> +    if (ret < 0) {
> +        fprintf(stderr, "Failed to clear endpoint\n");
> +    }
> +
> +    vhost_dev_stop(&vs->dev, vdev);
> +}
> +
> +static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
> +                                 uint16_t tpgt)
> +{
> +    VHostSCSI *vs = g_malloc0(sizeof(*vs));
> +    int ret;
> +
> +    /* TODO set up vhost-scsi device and bind to tcm_vhost/$wwpm/tpgt_$tpgt */
> +    fprintf(stderr, "wwpn = \"%s\" tpgt = \"%u\"\n", id, tpgt);
> +
> +    ret = vhost_dev_init(&vs->dev, -1, "/dev/vhost-scsi", false);
> +    if (ret < 0) {
> +        fprintf(stderr, "vhost-scsi: vhost initialization failed: %s\n",
> +                strerror(-ret));
> +        return NULL;
> +    }
> +    vs->dev.backend_features = 0;
> +    vs->dev.acked_features = 0;
> +
> +    vs->id = g_strdup(id);
> +    vs->wwpn = g_strdup(wwpn);
> +    vs->tpgt = tpgt;
> +    QLIST_INSERT_HEAD(&vhost_scsi_list, vs, list);
> +
> +    return vs;
> +}
> +
> +VHostSCSI *vhost_scsi_add_opts(QemuOpts *opts)
> +{
> +    const char *id;
> +    const char *wwpn;
> +    uint64_t tpgt;
> +
> +    id = qemu_opts_id(opts);
> +    if (!id) {
> +        fprintf(stderr, "vhost-scsi: no id specified\n");
> +        return NULL;
> +    }
> +    if (find_vhost_scsi(id)) {
> +        fprintf(stderr, "duplicate vhost-scsi: \"%s\"\n", id);
> +        return NULL;
> +    }
> +
> +    wwpn = qemu_opt_get(opts, "wwpn");
> +    if (!wwpn) {
> +        fprintf(stderr, "vhost-scsi: \"%s\" missing wwpn\n", id);
> +        return NULL;
> +    }
> +
> +    tpgt = qemu_opt_get_number(opts, "tpgt", UINT64_MAX);
> +    if (tpgt > UINT16_MAX) {
> +        fprintf(stderr, "vhost-scsi: \"%s\" needs a 16-bit tpgt\n", id);
> +        return NULL;
> +    }
> +
> +    return vhost_scsi_add(id, wwpn, tpgt);
> +}
> diff --git a/hw/vhost-scsi.h b/hw/vhost-scsi.h
> new file mode 100644
> index 0000000..f3096dc
> --- /dev/null
> +++ b/hw/vhost-scsi.h
> @@ -0,0 +1,50 @@
> +/*
> + * vhost_scsi host device
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Stefan Hajnoczi   <stefanha@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef VHOST_SCSI_H
> +#define VHOST_SCSI_H
> +
> +#include "qemu-common.h"
> +#include "qemu-option.h"
> +
> +/*
> + * Used by QEMU userspace to ensure a consistent vhost-scsi ABI.
> + *
> + * ABI Rev 0: July 2012 version starting point for v3.6-rc merge candidate +
> + *            RFC-v2 vhost-scsi userspace.  Add GET_ABI_VERSION ioctl usage
> + */
> +
> +#define VHOST_SCSI_ABI_VERSION	0
> +
> +/* TODO #include <linux/vhost.h> properly */
> +/* For VHOST_SCSI_SET_ENDPOINT/VHOST_SCSI_CLEAR_ENDPOINT ioctl */
> +struct vhost_scsi_target {
> +    int abi_version;
> +    unsigned char vhost_wwpn[224];
> +    unsigned short vhost_tpgt;
> +};
> +
> +#define VHOST_VIRTIO 0xAF
> +#define VHOST_SCSI_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x40, struct vhost_scsi_target)
> +#define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct vhost_scsi_target)
> +#define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, struct vhost_scsi_target)
> +
> +VHostSCSI *find_vhost_scsi(const char *id);
> +const char *vhost_scsi_get_id(VHostSCSI *vs);
> +
> +VHostSCSI *vhost_scsi_add_opts(QemuOpts *opts);
> +
> +int vhost_scsi_start(VHostSCSI *vs, VirtIODevice *vdev);
> +void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev);
> +
> +#endif
> diff --git a/qemu-common.h b/qemu-common.h
> index f9deca6..ec36002 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -280,6 +280,7 @@ typedef struct EventNotifier EventNotifier;
>  typedef struct VirtIODevice VirtIODevice;
>  typedef struct QEMUSGList QEMUSGList;
>  typedef struct SHPCDevice SHPCDevice;
> +typedef struct VHostSCSI VHostSCSI;
>  
>  typedef uint64_t pcibus_t;
>  
> diff --git a/qemu-config.c b/qemu-config.c
> index 5c3296b..33399ea 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -626,6 +626,21 @@ QemuOptsList qemu_boot_opts = {
>      },
>  };
>  
> +QemuOptsList qemu_vhost_scsi_opts = {
> +    .name = "vhost-scsi",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_vhost_scsi_opts.head),
> +    .desc = {
> +        {
> +            .name = "wwpn",
> +            .type = QEMU_OPT_STRING,
> +        }, {
> +            .name = "tpgt",
> +            .type = QEMU_OPT_NUMBER,
> +        },
> +        { /* end of list */ }
> +    },
> +};
> +
>  static QemuOptsList *vm_config_groups[32] = {
>      &qemu_drive_opts,
>      &qemu_chardev_opts,
> @@ -641,6 +656,7 @@ static QemuOptsList *vm_config_groups[32] = {
>      &qemu_machine_opts,
>      &qemu_boot_opts,
>      &qemu_iscsi_opts,
> +    &qemu_vhost_scsi_opts,
>      NULL,
>  };
>  
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 47cb5bd..4e7a03c 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -565,6 +565,10 @@ possible drivers and properties, use @code{-device ?} and
>  ETEXI
>  
>  DEFHEADING()
> +DEF("vhost-scsi", HAS_ARG, QEMU_OPTION_vhost_scsi,
> +    "-vhost-scsi wwpn=string0,tpgt=number0\n"
> +    "                add vhost-scsi device\n",
> +    QEMU_ARCH_ALL)
>  
>  DEFHEADING(File system options:)
>  
> diff --git a/vl.c b/vl.c
> index 91076f0..61c8284 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -144,6 +144,7 @@ int main(int argc, char **argv)
>  #include "qemu-options.h"
>  #include "qmp-commands.h"
>  #include "main-loop.h"
> +#include "hw/vhost-scsi.h"
>  #ifdef CONFIG_VIRTFS
>  #include "fsdev/qemu-fsdev.h"
>  #endif
> @@ -1858,6 +1859,14 @@ static int fsdev_init_func(QemuOpts *opts, void *opaque)
>  }
>  #endif
>  
> +static int vhost_scsi_init_func(QemuOpts *opts, void *opaque)
> +{
> +    if (!vhost_scsi_add_opts(opts)) {
> +        return -1;
> +    }
> +    return 0;
> +}
> +
>  static int mon_init_func(QemuOpts *opts, void *opaque)
>  {
>      CharDriverState *chr;
> @@ -2617,6 +2626,11 @@ int main(int argc, char **argv, char **envp)
>                  }
>                  break;
>  #endif
> +            case QEMU_OPTION_vhost_scsi:
> +                if (!qemu_opts_parse(qemu_find_opts("vhost-scsi"), optarg, 0)) {
> +                    exit(1);
> +                }
> +                break;
>  #ifdef CONFIG_SLIRP
>              case QEMU_OPTION_tftp:
>                  legacy_tftp_prefix = optarg;
> @@ -3337,6 +3351,10 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  #endif
> +    if (qemu_opts_foreach(qemu_find_opts("vhost-scsi"),
> +                          vhost_scsi_init_func, NULL, 1)) {
> +        exit(1);
> +    }
>  
>      os_daemonize();
>  
> 

Anthony, do you think your -object patches are going to go in?  Perhaps
we should reuse that option instead of inventing <option-of-the-day>
every time.

Paolo

Re: [Qemu-devel] [RFC-v2 4/6] virtio-scsi: Add start/stop functionality for vhost-scsi

[Paolo Bonzini]

Il 13/08/2012 10:35, Nicholas A. Bellinger ha scritto:
> From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> 
> This patch starts and stops vhost as the virtio device transitions
> through its status phases.  Vhost can only be started once the guest
> reports its driver has successfully initialized, which means the
> virtqueues have been set up by the guest.
> 
> v2: - Squash virtio-scsi: use the vhost-scsi host device from stefan (nab)
>     - Fix up virtio_scsi_properties[] conflict w/ upstream qemu (nab)
>     - Drop usage of to_virtio_scsi() in virtio_scsi_set_status()
>       (reported by paolo)
>     - Use modern VirtIOSCSIConf define in virtio-scsi.h (reported by paolo)
>     - Use s->conf->vhost_scsi instead of proxyconf->vhost_scsi in
>       virtio_scsi_init() (reported by paolo)
>     - Only register QEMU SCSI bus is vhost-scsi is not active (reported
>       by paolo)

How much of the functionality of virtio-scsi.[ch] is still in use at
this point?  Would it make more sense to use a separate vhost-scsi-pci
device instead?

Especially since advertising VIRTIO_SCSI_F_HOTPLUG and
VIRTIO_SCSI_F_CHANGE is probably wrong for vhost-scsi...

Paolo

> Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>  hw/virtio-pci.c  |    1 +
>  hw/virtio-scsi.c |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/virtio-scsi.h |    1 +
>  3 files changed, 50 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 125eded..b29fc3b 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -1036,6 +1036,7 @@ static void virtio_scsi_exit_pci(PCIDevice *pci_dev)
>  }
>  
>  static Property virtio_scsi_properties[] = {
> +    DEFINE_PROP_VHOST_SCSI("vhost-scsi", VirtIOPCIProxy, scsi.vhost_scsi),
>      DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
>      DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, DEV_NVECTORS_UNSPECIFIED),
>      DEFINE_VIRTIO_SCSI_PROPERTIES(VirtIOPCIProxy, host_features, scsi),
> diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
> index 5f737ac..8130956 100644
> --- a/hw/virtio-scsi.c
> +++ b/hw/virtio-scsi.c
> @@ -13,9 +13,13 @@
>   *
>   */
>  
> +#include "qemu-common.h"
> +#include "qemu-error.h"
> +#include "vhost-scsi.h"
>  #include "virtio-scsi.h"
>  #include <hw/scsi.h>
>  #include <hw/scsi-defs.h>
> +#include "vhost.h"
>  
>  #define VIRTIO_SCSI_VQ_SIZE     128
>  #define VIRTIO_SCSI_CDB_SIZE    32
> @@ -147,6 +151,9 @@ typedef struct {
>      VirtQueue *ctrl_vq;
>      VirtQueue *event_vq;
>      VirtQueue *cmd_vqs[0];
> +
> +    bool vhost_started;
> +    VHostSCSI *vhost_scsi;
>  } VirtIOSCSI;
>  
>  typedef struct VirtIOSCSIReq {
> @@ -699,6 +706,38 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
>      .load_request = virtio_scsi_load_request,
>  };
>  
> +static bool virtio_scsi_started(VirtIOSCSI *s, uint8_t val)
> +{
> +    return (val & VIRTIO_CONFIG_S_DRIVER_OK) && s->vdev.vm_running;
> +}
> +
> +static void virtio_scsi_set_status(VirtIODevice *vdev, uint8_t val)
> +{
> +    VirtIOSCSI *s = (VirtIOSCSI *)vdev;
> +    bool start = virtio_scsi_started(s, val);
> +
> +    if (s->vhost_started == start) {
> +        return;
> +    }
> +
> +    if (start) {
> +        int ret;
> +
> +        ret = vhost_scsi_start(s->vhost_scsi, vdev);
> +        if (ret < 0) {
> +            error_report("virtio-scsi: unable to start vhost: %s\n",
> +                         strerror(-ret));
> +
> +            /* There is no userspace virtio-scsi fallback so exit */
> +            exit(1);
> +        }
> +    } else {
> +        vhost_scsi_stop(s->vhost_scsi, vdev);
> +    }
> +
> +    s->vhost_started = start;
> +}
> +
>  VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf)
>  {
>      VirtIOSCSI *s;
> @@ -712,12 +751,17 @@ VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf)
>  
>      s->qdev = dev;
>      s->conf = proxyconf;
> +    s->vhost_started = false;
> +    s->vhost_scsi = s->conf->vhost_scsi;
>  
>      /* TODO set up vdev function pointers */
>      s->vdev.get_config = virtio_scsi_get_config;
>      s->vdev.set_config = virtio_scsi_set_config;
>      s->vdev.get_features = virtio_scsi_get_features;
>      s->vdev.reset = virtio_scsi_reset;
> +    if (s->vhost_scsi) {
> +        s->vdev.set_status = virtio_scsi_set_status;
> +    }
>  
>      s->ctrl_vq = virtio_add_queue(&s->vdev, VIRTIO_SCSI_VQ_SIZE,
>                                     virtio_scsi_handle_ctrl);
> @@ -743,5 +787,9 @@ void virtio_scsi_exit(VirtIODevice *vdev)
>  {
>      VirtIOSCSI *s = (VirtIOSCSI *)vdev;
>      unregister_savevm(s->qdev, "virtio-scsi", s);
> +
> +    /* This will stop vhost backend if appropriate. */
> +    virtio_scsi_set_status(vdev, 0);
> +
>      virtio_cleanup(vdev);
>  }
> diff --git a/hw/virtio-scsi.h b/hw/virtio-scsi.h
> index 4bc889d..74e9422 100644
> --- a/hw/virtio-scsi.h
> +++ b/hw/virtio-scsi.h
> @@ -22,6 +22,7 @@
>  #define VIRTIO_ID_SCSI  8
>  
>  struct VirtIOSCSIConf {
> +    VHostSCSI *vhost_scsi;
>      uint32_t num_queues;
>      uint32_t max_sectors;
>      uint32_t cmd_per_lun;
> 

Re: [Qemu-devel] [RFC-v2 4/6] virtio-scsi: Add start/stop functionality for vhost-scsi

[Stefan Hajnoczi]

On Mon, Aug 20, 2012 at 10:04 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 13/08/2012 10:35, Nicholas A. Bellinger ha scritto:
>> From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>>
>> This patch starts and stops vhost as the virtio device transitions
>> through its status phases.  Vhost can only be started once the guest
>> reports its driver has successfully initialized, which means the
>> virtqueues have been set up by the guest.
>>
>> v2: - Squash virtio-scsi: use the vhost-scsi host device from stefan (nab)
>>     - Fix up virtio_scsi_properties[] conflict w/ upstream qemu (nab)
>>     - Drop usage of to_virtio_scsi() in virtio_scsi_set_status()
>>       (reported by paolo)
>>     - Use modern VirtIOSCSIConf define in virtio-scsi.h (reported by paolo)
>>     - Use s->conf->vhost_scsi instead of proxyconf->vhost_scsi in
>>       virtio_scsi_init() (reported by paolo)
>>     - Only register QEMU SCSI bus is vhost-scsi is not active (reported
>>       by paolo)
>
> How much of the functionality of virtio-scsi.[ch] is still in use at
> this point?  Would it make more sense to use a separate vhost-scsi-pci
> device instead?

Since the SCSI target lives in the kernel, almost everything is driven
from tcm_vhost.ko.  tcm_vhost.ko basically implements the full device
so I see the argument for -device vhost-scsi-pci.

> Especially since advertising VIRTIO_SCSI_F_HOTPLUG and
> VIRTIO_SCSI_F_CHANGE is probably wrong for vhost-scsi...

vhost participates in feature bit negotiation, see
VHOST_GET_FEATURES/VHOST_SET_FEATURES ioctls.

Stefan

Re: [Qemu-devel] [RFC-v2 4/6] virtio-scsi: Add start/stop functionality for vhost-scsi

[Michael S. Tsirkin]

On Mon, Aug 20, 2012 at 12:31:01PM +0100, Stefan Hajnoczi wrote:
> On Mon, Aug 20, 2012 at 10:04 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > Il 13/08/2012 10:35, Nicholas A. Bellinger ha scritto:
> >> From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> >>
> >> This patch starts and stops vhost as the virtio device transitions
> >> through its status phases.  Vhost can only be started once the guest
> >> reports its driver has successfully initialized, which means the
> >> virtqueues have been set up by the guest.
> >>
> >> v2: - Squash virtio-scsi: use the vhost-scsi host device from stefan (nab)
> >>     - Fix up virtio_scsi_properties[] conflict w/ upstream qemu (nab)
> >>     - Drop usage of to_virtio_scsi() in virtio_scsi_set_status()
> >>       (reported by paolo)
> >>     - Use modern VirtIOSCSIConf define in virtio-scsi.h (reported by paolo)
> >>     - Use s->conf->vhost_scsi instead of proxyconf->vhost_scsi in
> >>       virtio_scsi_init() (reported by paolo)
> >>     - Only register QEMU SCSI bus is vhost-scsi is not active (reported
> >>       by paolo)
> >
> > How much of the functionality of virtio-scsi.[ch] is still in use at
> > this point?  Would it make more sense to use a separate vhost-scsi-pci
> > device instead?
> 
> Since the SCSI target lives in the kernel, almost everything is driven
> from tcm_vhost.ko.  tcm_vhost.ko basically implements the full device
> so I see the argument for -device vhost-scsi-pci.

A bit unhappy that there is no virtio in the name since it is
implementing virtio protocol.

> > Especially since advertising VIRTIO_SCSI_F_HOTPLUG and
> > VIRTIO_SCSI_F_CHANGE is probably wrong for vhost-scsi...
> 
> vhost participates in feature bit negotiation, see
> VHOST_GET_FEATURES/VHOST_SET_FEATURES ioctls.
> 
> Stefan

Re: [Qemu-devel] [RFC-v2 4/6] virtio-scsi: Add start/stop functionality for vhost-scsi

[Paolo Bonzini]

Il 20/08/2012 13:57, Michael S. Tsirkin ha scritto:
>>> > > How much of the functionality of virtio-scsi.[ch] is still in use at
>>> > > this point?  Would it make more sense to use a separate vhost-scsi-pci
>>> > > device instead?
>> > 
>> > Since the SCSI target lives in the kernel, almost everything is driven
>> > from tcm_vhost.ko.  tcm_vhost.ko basically implements the full device
>> > so I see the argument for -device vhost-scsi-pci.
> A bit unhappy that there is no virtio in the name since it is
> implementing virtio protocol.
> 

Yeah, but vhost \subseteq virtio...

Paolo

Re: [Qemu-devel] [RFC-v2 3/6] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost

[Nicholas A. Bellinger]

On Sun, 2012-08-19 at 11:44 +0300, Michael S. Tsirkin wrote:
> On Sat, Aug 18, 2012 at 05:36:26PM -0700, Nicholas A. Bellinger wrote:
> > On Sat, 2012-08-18 at 22:12 +0300, Michael S. Tsirkin wrote:
> > > On Tue, Aug 14, 2012 at 01:31:14PM -0700, Nicholas A. Bellinger wrote:
> > > > On Mon, 2012-08-13 at 11:53 +0300, Michael S. Tsirkin wrote:
> > > > > On Mon, Aug 13, 2012 at 08:35:14AM +0000, Nicholas A. Bellinger wrote:
> > > > > > From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

<SNIP>

> > > > Mmm, looking at how vhost_net_init + tap.c does this, but am not quite
> > > > what fd needs to be propagated up for virtio-scsi -> vhost-scsi..
> > > > 
> > > > Can you please elaborate on this one a bit more..?
> > > > 
> > > 
> > > The idea is to allow running as a user without access to
> > > /dev/vhost-scsi.
> > > For this, allow passing in the fd of /dev/vhost-scsi through unix domain sockets.
> > > 
> > 
> > Ah, that is a pretty neat trick..   So for vhost-scsi code, this would
> > mean something along the lines of the following, yes..?
> 
> Yes but with one correction. See below.
> 
> > Thanks MST!
> 
> > diff --git a/hw/vhost-scsi.c b/hw/vhost-scsi.c
> > index 4206a75..8af8758 100644
> > --- a/hw/vhost-scsi.c
> > +++ b/hw/vhost-scsi.c
> > @@ -21,6 +21,7 @@ struct VHostSCSI {
> >      const char *id;
> >      const char *wwpn;
> >      uint16_t tpgt;
> > +    int vhostfd;
> >      struct vhost_dev dev;
> >      struct vhost_virtqueue vqs[VHOST_SCSI_VQ_NUM];
> >      QLIST_ENTRY(VHostSCSI) list;
> > @@ -114,13 +115,32 @@ void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev)
> >  }
> >  
> >  static VHostSCSI *vhost_scsi_add(const char *id, const char *wwpn,
> > -                                 uint16_t tpgt)
> > +                                 uint16_t tpgt, const char *vhostfd_str)
> >  {
> > -    VHostSCSI *vs = g_malloc0(sizeof(*vs));
> > +    VHostSCSI *vs;
> >      int ret;
> >  
> > +    vs = g_malloc0(sizeof(*vs));
> > +    if (!vs) {
> > +        error_report("vhost-scsi: unable to allocate *vs\n");
> > +        return NULL;
> > +    }
> > +    vs->vhostfd = -1;
> > +
> > +    if (vhostfd_str) {
> > +        if (!qemu_isdigit(vhostfd_str[0])) {
> > +            error_report("vhost-scsi: passed vhostfd value is not a digit\n");
> > +            return NULL;
> 
> This let you use an fd which was open at exec
> but does not allow for fd to be open later in
> case device is hot-plugged.
> 
> See net_handle_fd_param - I think you can just rename it
> qemu_handle_fd_param to avoid code duplication.
> 

OK, so monitor_get_fd() will set this up for the case where the device
is hot-plugged.  That makes alot more sense now..

So renaming net_handle_fd_param -> qemu_handle_fd_param + moving into
cutils.c, and will include as a leading patch for RFC-v3.

Thanks MST!

--nab