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

2012-08-20 Thread Paolo Bonzini 
Il 13/08/2012 10:35, Nicholas A. Bellinger ha scritto:
 From: Stefan Hajnoczi stefa...@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 stefa...@linux.vnet.ibm.com
 Cc: Zhi Yong Wu wu...@linux.vnet.ibm.com
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Paolo Bonzini pbonz...@redhat.com
 Signed-off-by: Nicholas Bellinger n...@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_SIZE32
 @@ -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

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

2012-08-20 Thread Stefan Hajnoczi 
On Mon, Aug 20, 2012 at 10:04 AM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 13/08/2012 10:35, Nicholas A. Bellinger ha scritto:
 From: Stefan Hajnoczi stefa...@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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2012-08-20 Thread 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 pbonz...@redhat.com wrote:
  Il 13/08/2012 10:35, Nicholas A. Bellinger ha scritto:
  From: Stefan Hajnoczi stefa...@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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2012-08-20 Thread 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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2012-08-13 Thread Nicholas A. Bellinger 
From: Stefan Hajnoczi stefa...@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 stefa...@linux.vnet.ibm.com
Cc: Zhi Yong Wu wu...@linux.vnet.ibm.com
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Nicholas Bellinger n...@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_SIZE32
@@ -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

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html