Skip to contentSkip to conversation

Hotplug support for virtio-scsi

PATCH Superseded Cong Meng 4 participants 10 messages Jun 20, 2012 – Jun 25, 2012 virtio core

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
author
#1 lore raw

PATCH 1/2 scsi bus: introduce hotplug() and hot_unplug() interfaces for SCSI bus

In reply to Cong Meng

Add two interfaces hotplug() and hot_unplug() to scsi bus info. The embody scsi bus can implement these two interfaces to signal the HBA driver of guest kernel to add/remove the scsi device in question.

---
 hw/scsi-bus.c |   16 +++++++++++++++-
 hw/scsi.h     |    2 ++
 2 files changed, 17 insertions(+), 1 deletions(-)
hw/scsi-bus.c +15 -1
index dbdb99c..cc3ec75 100644
@@ -177,6 +177,10 @@ static int scsi_qdev_init(DeviceState *qdev)
                                                          dev);
     }
 
+    if (bus->info->hotplug) {
+	bus->info->hotplug(bus, dev);
+    }
+
 err:
     return rc;
 }
@@ -1539,6 +1543,16 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
+static int scsi_qdev_unplug(DeviceState *qdev)
+{
+    SCSIDevice *dev = SCSI_DEVICE(qdev);
+    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus);
+
+    if (bus->info->hot_unplug)
+	bus->info->hot_unplug(bus, dev);
+    return qdev_simple_unplug_cb(qdev);
+}
+
 const VMStateInfo vmstate_info_scsi_requests = {
     .name = "scsi-requests",
     .get  = get_scsi_requests,
@@ -1575,7 +1589,7 @@ static void scsi_device_class_init(ObjectClass *klass, void *data)
     DeviceClass *k = DEVICE_CLASS(klass);
     k->bus_info = &scsi_bus_info;
     k->init     = scsi_qdev_init;
-    k->unplug   = qdev_simple_unplug_cb;
+    k->unplug   = scsi_qdev_unplug;
     k->exit     = scsi_qdev_exit;
 }
 
hw/scsi.h +2 -0
index 2eb66f7..5768071 100644
@@ -130,6 +130,8 @@ struct SCSIBusInfo {
     void (*transfer_data)(SCSIRequest *req, uint32_t arg);
     void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid);
     void (*cancel)(SCSIRequest *req);
+    void (*hotplug)(SCSIBus *bus, SCSIDevice *dev);
+    void (*hot_unplug)(SCSIBus *bus, SCSIDevice *dev);
     QEMUSGList *(*get_sg_list)(SCSIRequest *req);
 
     void (*save_request)(QEMUFile *f, SCSIRequest *req);
Signature · 1 line
1.7.7
author
#5 lore raw

PATCH 2/2 virtio-scsi: Implement hotplug support for virtio-scsi

In reply to Cong Meng

Implement the hotplug() and hot_unplug() interfaces in virtio-scsi, by signal the virtio_scsi.ko in guest kernel via event virtual queue.

The counterpart patch of virtio_scsi.ko will be sent soon in another thread.

---
 hw/virtio-scsi.c |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 69 insertions(+), 3 deletions(-)
hw/virtio-scsi.c +69 -3
index e8328f4..626ec5f 100644
@@ -24,6 +24,10 @@
 #define VIRTIO_SCSI_MAX_TARGET  255
 #define VIRTIO_SCSI_MAX_LUN     16383
 
+/* Feature Bits */
+#define VIRTIO_SCSI_F_INOUT                    0
+#define VIRTIO_SCSI_F_HOTPLUG                  1
+
 /* Response codes */
 #define VIRTIO_SCSI_S_OK                       0
 #define VIRTIO_SCSI_S_OVERRUN                  1
@@ -60,6 +64,11 @@
 #define VIRTIO_SCSI_T_TRANSPORT_RESET          1
 #define VIRTIO_SCSI_T_ASYNC_NOTIFY             2
 
+/* Reasons of transport reset event */     
+#define VIRTIO_SCSI_EVT_RESET_HARD             0
+#define VIRTIO_SCSI_EVT_RESET_RESCAN           1
+#define VIRTIO_SCSI_EVT_RESET_REMOVED          2
+
 /* SCSI command request, followed by data-out */
 typedef struct {
     uint8_t lun[8];              /* Logical Unit Number */
@@ -206,11 +215,12 @@ static void qemu_sgl_init_external(QEMUSGList *qsgl, struct iovec *sg,
 static void virtio_scsi_parse_req(VirtIOSCSI *s, VirtQueue *vq,
                                   VirtIOSCSIReq *req)
 {
-    assert(req->elem.out_num && req->elem.in_num);
+    assert(req->elem.in_num);
     req->vq = vq;
     req->dev = s;
     req->sreq = NULL;
-    req->req.buf = req->elem.out_sg[0].iov_base;
+    if (req->elem.out_num)
+        req->req.buf = req->elem.out_sg[0].iov_base;
     req->resp.buf = req->elem.in_sg[0].iov_base;
 
     if (req->elem.out_num > 1) {
@@ -405,6 +415,10 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
     }
 }
 
+static void virtio_scsi_handle_event(VirtIODevice *vdev, VirtQueue *vq)
+{
+}
+
 static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
                                          size_t resid)
 {
@@ -541,6 +555,7 @@ static void virtio_scsi_set_config(VirtIODevice *vdev,
 static uint32_t virtio_scsi_get_features(VirtIODevice *vdev,
                                          uint32_t requested_features)
 {
+    requested_features |= (1UL << VIRTIO_SCSI_F_HOTPLUG);
     return requested_features;
 }
 
@@ -568,6 +583,55 @@ static int virtio_scsi_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
+static void virtio_scsi_push_event(VirtIOSCSI *s, uint32_t target, uint32_t lun,
+                                   uint32_t event, uint32_t reason)
+{
+    VirtIOSCSIReq *req;
+    VirtIOSCSIEvent *evt;
+
+    if ((req = virtio_scsi_pop_req(s, s->event_vq))) {
+        int in_size;
+        if (req->elem.out_num || req->elem.in_num != 1) {
+            virtio_scsi_bad_req();
+        }
+
+        in_size = req->elem.in_sg[0].iov_len;
+        if (in_size < sizeof(VirtIOSCSIEvent)) {
+            virtio_scsi_bad_req();
+        }
+
+        evt = req->resp.event;
+        evt->event = event;
+        evt->reason = reason;
+        evt->lun[0] = 0;
+        evt->lun[1] = target;
+        evt->lun[2] = (lun >> 8);
+        evt->lun[3] = lun & 0xFF;
+        virtio_scsi_complete_req(req);
+    }
+}
+
+static void virtio_scsi_hotplug(SCSIBus *bus, SCSIDevice *dev)
+{
+    VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
+
+    if (((s->vdev.guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) &&
+        (s->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+        virtio_scsi_push_event(s, dev->id, dev->lun, 
+            VIRTIO_SCSI_T_TRANSPORT_RESET, VIRTIO_SCSI_EVT_RESET_RESCAN);
+    }
+}
+
+static void virtio_scsi_hot_unplug(SCSIBus *bus, SCSIDevice *dev)
+{
+    VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
+
+    if ((s->vdev.guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) {
+        virtio_scsi_push_event(s, dev->id, dev->lun, 
+            VIRTIO_SCSI_T_TRANSPORT_RESET, VIRTIO_SCSI_EVT_RESET_REMOVED);
+    }
+}
+
 static struct SCSIBusInfo virtio_scsi_scsi_info = {
     .tcq = true,
     .max_channel = VIRTIO_SCSI_MAX_CHANNEL,
@@ -576,6 +640,8 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
 
     .complete = virtio_scsi_command_complete,
     .cancel = virtio_scsi_request_cancelled,
+    .hotplug = virtio_scsi_hotplug,
+    .hot_unplug = virtio_scsi_hot_unplug,
     .get_sg_list = virtio_scsi_get_sg_list,
     .save_request = virtio_scsi_save_request,
     .load_request = virtio_scsi_load_request,
@@ -604,7 +670,7 @@ VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf)
     s->ctrl_vq = virtio_add_queue(&s->vdev, VIRTIO_SCSI_VQ_SIZE,
                                    virtio_scsi_handle_ctrl);
     s->event_vq = virtio_add_queue(&s->vdev, VIRTIO_SCSI_VQ_SIZE,
-                                   NULL);
+                                   virtio_scsi_handle_event);
     for (i = 0; i < s->conf->num_queues; i++) {
         s->cmd_vqs[i] = virtio_add_queue(&s->vdev, VIRTIO_SCSI_VQ_SIZE,
                                          virtio_scsi_handle_cmd);
Signature · 1 line
1.7.7

Cong Meng

author
#0 lore raw

cover Hotplug support for virtio-scsi

These patches implement the hotplug support for virtio-scsi.
When a new device attaches/detaches to virtio-scsi bus via device_add/device_del commands, the HBA driver in guest kernel will be signaled to add/remove the scsi device.

Cong Meng (2):
scsi bus: introduce hotplug() and hot_unplug() interfaces for SCSI bus
virtio-scsi: Implement hotplug support for virtio-scsi

 hw/scsi-bus.c    |   16 +++++++++++-
 hw/scsi.h        |    2 +
 hw/virtio-scsi.c |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 86 insertions(+), 4 deletions(-)
Signature · 1 line
1.7.7
author
#6 lore raw

PATCH 2/2 virtio-scsi: Implement hotplug support for virtio-scsi

Implement the hotplug() and hot_unplug() interfaces in virtio-scsi, by signal the virtio_scsi.ko in guest kernel via event virtual queue.

The counterpart patch of virtio_scsi.ko will be sent soon in another thread.

---
 hw/virtio-scsi.c |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 69 insertions(+), 3 deletions(-)
hw/virtio-scsi.c +69 -3
index e8328f4..626ec5f 100644
@@ -24,6 +24,10 @@
 #define VIRTIO_SCSI_MAX_TARGET  255
 #define VIRTIO_SCSI_MAX_LUN     16383
 
+/* Feature Bits */
+#define VIRTIO_SCSI_F_INOUT                    0
+#define VIRTIO_SCSI_F_HOTPLUG                  1
+
 /* Response codes */
 #define VIRTIO_SCSI_S_OK                       0
 #define VIRTIO_SCSI_S_OVERRUN                  1
@@ -60,6 +64,11 @@
 #define VIRTIO_SCSI_T_TRANSPORT_RESET          1
 #define VIRTIO_SCSI_T_ASYNC_NOTIFY             2
 
+/* Reasons of transport reset event */     
+#define VIRTIO_SCSI_EVT_RESET_HARD             0
+#define VIRTIO_SCSI_EVT_RESET_RESCAN           1
+#define VIRTIO_SCSI_EVT_RESET_REMOVED          2
+
 /* SCSI command request, followed by data-out */
 typedef struct {
     uint8_t lun[8];              /* Logical Unit Number */
@@ -206,11 +215,12 @@ static void qemu_sgl_init_external(QEMUSGList *qsgl, struct iovec *sg,
 static void virtio_scsi_parse_req(VirtIOSCSI *s, VirtQueue *vq,
                                   VirtIOSCSIReq *req)
 {
-    assert(req->elem.out_num && req->elem.in_num);
+    assert(req->elem.in_num);
     req->vq = vq;
     req->dev = s;
     req->sreq = NULL;
-    req->req.buf = req->elem.out_sg[0].iov_base;
+    if (req->elem.out_num)
+        req->req.buf = req->elem.out_sg[0].iov_base;
     req->resp.buf = req->elem.in_sg[0].iov_base;
 
     if (req->elem.out_num > 1) {
@@ -405,6 +415,10 @@ static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
     }
 }
 
+static void virtio_scsi_handle_event(VirtIODevice *vdev, VirtQueue *vq)
+{
+}
+
 static void virtio_scsi_command_complete(SCSIRequest *r, uint32_t status,
                                          size_t resid)
 {
@@ -541,6 +555,7 @@ static void virtio_scsi_set_config(VirtIODevice *vdev,
 static uint32_t virtio_scsi_get_features(VirtIODevice *vdev,
                                          uint32_t requested_features)
 {
+    requested_features |= (1UL << VIRTIO_SCSI_F_HOTPLUG);
     return requested_features;
 }
 
@@ -568,6 +583,55 @@ static int virtio_scsi_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
+static void virtio_scsi_push_event(VirtIOSCSI *s, uint32_t target, uint32_t lun,
+                                   uint32_t event, uint32_t reason)
+{
+    VirtIOSCSIReq *req;
+    VirtIOSCSIEvent *evt;
+
+    if ((req = virtio_scsi_pop_req(s, s->event_vq))) {
+        int in_size;
+        if (req->elem.out_num || req->elem.in_num != 1) {
+            virtio_scsi_bad_req();
+        }
+
+        in_size = req->elem.in_sg[0].iov_len;
+        if (in_size < sizeof(VirtIOSCSIEvent)) {
+            virtio_scsi_bad_req();
+        }
+
+        evt = req->resp.event;
+        evt->event = event;
+        evt->reason = reason;
+        evt->lun[0] = 0;
+        evt->lun[1] = target;
+        evt->lun[2] = (lun >> 8);
+        evt->lun[3] = lun & 0xFF;
+        virtio_scsi_complete_req(req);
+    }
+}
+
+static void virtio_scsi_hotplug(SCSIBus *bus, SCSIDevice *dev)
+{
+    VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
+
+    if (((s->vdev.guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) &&
+        (s->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+        virtio_scsi_push_event(s, dev->id, dev->lun, 
+            VIRTIO_SCSI_T_TRANSPORT_RESET, VIRTIO_SCSI_EVT_RESET_RESCAN);
+    }
+}
+
+static void virtio_scsi_hot_unplug(SCSIBus *bus, SCSIDevice *dev)
+{
+    VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
+
+    if ((s->vdev.guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) {
+        virtio_scsi_push_event(s, dev->id, dev->lun, 
+            VIRTIO_SCSI_T_TRANSPORT_RESET, VIRTIO_SCSI_EVT_RESET_REMOVED);
+    }
+}
+
 static struct SCSIBusInfo virtio_scsi_scsi_info = {
     .tcq = true,
     .max_channel = VIRTIO_SCSI_MAX_CHANNEL,
@@ -576,6 +640,8 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
 
     .complete = virtio_scsi_command_complete,
     .cancel = virtio_scsi_request_cancelled,
+    .hotplug = virtio_scsi_hotplug,
+    .hot_unplug = virtio_scsi_hot_unplug,
     .get_sg_list = virtio_scsi_get_sg_list,
     .save_request = virtio_scsi_save_request,
     .load_request = virtio_scsi_load_request,
@@ -604,7 +670,7 @@ VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf)
     s->ctrl_vq = virtio_add_queue(&s->vdev, VIRTIO_SCSI_VQ_SIZE,
                                    virtio_scsi_handle_ctrl);
     s->event_vq = virtio_add_queue(&s->vdev, VIRTIO_SCSI_VQ_SIZE,
-                                   NULL);
+                                   virtio_scsi_handle_event);
     for (i = 0; i < s->conf->num_queues; i++) {
         s->cmd_vqs[i] = virtio_add_queue(&s->vdev, VIRTIO_SCSI_VQ_SIZE,
                                          virtio_scsi_handle_cmd);
Signature · 1 line
1.7.7
author
#2 lore raw

PATCH 1/2 scsi bus: introduce hotplug() and hot_unplug() interfaces for SCSI bus

In reply to Cong Meng

Add two interfaces hotplug() and hot_unplug() to scsi bus info. The embody scsi bus can implement these two interfaces to signal the HBA driver of guest kernel to add/remove the scsi device in question.

---
 hw/scsi-bus.c |   16 +++++++++++++++-
 hw/scsi.h     |    2 ++
 2 files changed, 17 insertions(+), 1 deletions(-)
hw/scsi-bus.c +15 -1
index dbdb99c..cc3ec75 100644
@@ -177,6 +177,10 @@ static int scsi_qdev_init(DeviceState *qdev)
                                                          dev);
     }
 
+    if (bus->info->hotplug) {
+	bus->info->hotplug(bus, dev);
+    }
+
 err:
     return rc;
 }
@@ -1539,6 +1543,16 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
+static int scsi_qdev_unplug(DeviceState *qdev)
+{
+    SCSIDevice *dev = SCSI_DEVICE(qdev);
+    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus);
+
+    if (bus->info->hot_unplug)
+	bus->info->hot_unplug(bus, dev);
+    return qdev_simple_unplug_cb(qdev);
+}
+
 const VMStateInfo vmstate_info_scsi_requests = {
     .name = "scsi-requests",
     .get  = get_scsi_requests,
@@ -1575,7 +1589,7 @@ static void scsi_device_class_init(ObjectClass *klass, void *data)
     DeviceClass *k = DEVICE_CLASS(klass);
     k->bus_info = &scsi_bus_info;
     k->init     = scsi_qdev_init;
-    k->unplug   = qdev_simple_unplug_cb;
+    k->unplug   = scsi_qdev_unplug;
     k->exit     = scsi_qdev_exit;
 }
 
hw/scsi.h +2 -0
index 2eb66f7..5768071 100644
@@ -130,6 +130,8 @@ struct SCSIBusInfo {
     void (*transfer_data)(SCSIRequest *req, uint32_t arg);
     void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid);
     void (*cancel)(SCSIRequest *req);
+    void (*hotplug)(SCSIBus *bus, SCSIDevice *dev);
+    void (*hot_unplug)(SCSIBus *bus, SCSIDevice *dev);
     QEMUSGList *(*get_sg_list)(SCSIRequest *req);
 
     void (*save_request)(QEMUFile *f, SCSIRequest *req);
Signature · 1 line
1.7.7
#3 lore raw

Re: [PATCH 1/2] scsi bus: introduce hotplug() and hot_unplug() interfaces for SCSI bus

Am 20.06.2012 08:47, schrieb Cong Meng:

Quoted 16 lines

Add two interfaces hotplug() and hot_unplug() to scsi bus info. The embody scsi bus can implement these two interfaces to signal the HBA driver of guest kernel to add/remove the scsi device in question.

---
 hw/scsi-bus.c |   16 +++++++++++++++-
 hw/scsi.h     |    2 ++
 2 files changed, 17 insertions(+), 1 deletions(-)
hw/scsi-bus.c +0 -0
index dbdb99c..cc3ec75 100644

hw/scsi-bus.c static int scsi_qdev_init(DeviceState *qdev) · line 181

Quoted 5 lines
hw/scsi-bus.c +2 -0
@@ -177,6 +177,10 @@ static int scsi_qdev_init(DeviceState *qdev)
                                                          dev);
     }
 
+    if (bus->info->hotplug) {
+	bus->info->hotplug(bus, dev);

Tab.

hw/scsi-bus.c static int get_scsi_requests(QEMUFile *f, void *pv, size_t size) · line 1549

Quoted 13 lines
hw/scsi-bus.c +6 -0
+    }
+
 err:
     return rc;
 }
@@ -1539,6 +1543,16 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
+static int scsi_qdev_unplug(DeviceState *qdev)
+{
+    SCSIDevice *dev = SCSI_DEVICE(qdev);
+    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus);

Since the tabs need to be fixed anyway, I would suggest to avoid using DO_UPCAST() with QOM types:

SCSIBus *bus = SCSI_BUS(qdev->parent_bus);

Also I'd like to raise the question towards Paolo and Anthony whether we might want to start naming new functions like this one scsi_device_unplug() to avoid the "qdev"? In this case sticking to "qdev" provides symmetry so there's good reasons for both approaches.

hw/scsi-bus.c static int get_scsi_requests(QEMUFile *f, void *pv, size_t size) · line 1552

Quoted 3 lines
hw/scsi-bus.c +3 -0
+
+    if (bus->info->hot_unplug)
+	bus->info->hot_unplug(bus, dev);

Tab. It seems your editor converts all 8-space indents, please check the second patch.

Otherwise looks okay from my side.

Andreas

Quoted 28 lines
hw/scsi-bus.c +4 -1
+    return qdev_simple_unplug_cb(qdev);
+}
+
 const VMStateInfo vmstate_info_scsi_requests = {
     .name = "scsi-requests",
     .get  = get_scsi_requests,
@@ -1575,7 +1589,7 @@ static void scsi_device_class_init(ObjectClass *klass, void *data)
     DeviceClass *k = DEVICE_CLASS(klass);
     k->bus_info = &scsi_bus_info;
     k->init     = scsi_qdev_init;
-    k->unplug   = qdev_simple_unplug_cb;
+    k->unplug   = scsi_qdev_unplug;
     k->exit     = scsi_qdev_exit;
 }
 
hw/scsi.h +2 -0
index 2eb66f7..5768071 100644
@@ -130,6 +130,8 @@ struct SCSIBusInfo {
     void (*transfer_data)(SCSIRequest *req, uint32_t arg);
     void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid);
     void (*cancel)(SCSIRequest *req);
+    void (*hotplug)(SCSIBus *bus, SCSIDevice *dev);
+    void (*hot_unplug)(SCSIBus *bus, SCSIDevice *dev);
     QEMUSGList *(*get_sg_list)(SCSIRequest *req);
 
     void (*save_request)(QEMUFile *f, SCSIRequest *req);
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
#4 lore raw

Re: [Qemu-devel] [PATCH 1/2] scsi bus: introduce hotplug() and hot_unplug() interfaces for SCSI bus

In reply to Cong Meng

Am 20.06.2012 08:47, schrieb Cong Meng:

Quoted 16 lines

Add two interfaces hotplug() and hot_unplug() to scsi bus info. The embody scsi bus can implement these two interfaces to signal the HBA driver of guest kernel to add/remove the scsi device in question.

---
 hw/scsi-bus.c |   16 +++++++++++++++-
 hw/scsi.h     |    2 ++
 2 files changed, 17 insertions(+), 1 deletions(-)
hw/scsi-bus.c +0 -0
index dbdb99c..cc3ec75 100644

hw/scsi-bus.c static int scsi_qdev_init(DeviceState *qdev) · line 181

Quoted 5 lines
hw/scsi-bus.c +2 -0
@@ -177,6 +177,10 @@ static int scsi_qdev_init(DeviceState *qdev)
                                                          dev);
     }
 
+    if (bus->info->hotplug) {
+	bus->info->hotplug(bus, dev);

Tab.

hw/scsi-bus.c static int get_scsi_requests(QEMUFile *f, void *pv, size_t size) · line 1549

Quoted 13 lines
hw/scsi-bus.c +6 -0
+    }
+
 err:
     return rc;
 }
@@ -1539,6 +1543,16 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
+static int scsi_qdev_unplug(DeviceState *qdev)
+{
+    SCSIDevice *dev = SCSI_DEVICE(qdev);
+    SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus);

Since the tabs need to be fixed anyway, I would suggest to avoid using DO_UPCAST() with QOM types:

SCSIBus *bus = SCSI_BUS(qdev->parent_bus);

Also I'd like to raise the question towards Paolo and Anthony whether we might want to start naming new functions like this one scsi_device_unplug() to avoid the "qdev"? In this case sticking to "qdev" provides symmetry so there's good reasons for both approaches.

hw/scsi-bus.c static int get_scsi_requests(QEMUFile *f, void *pv, size_t size) · line 1552

Quoted 3 lines
hw/scsi-bus.c +3 -0
+
+    if (bus->info->hot_unplug)
+	bus->info->hot_unplug(bus, dev);

Tab. It seems your editor converts all 8-space indents, please check the second patch.

Otherwise looks okay from my side.

Andreas

Quoted 28 lines
hw/scsi-bus.c +4 -1
+    return qdev_simple_unplug_cb(qdev);
+}
+
 const VMStateInfo vmstate_info_scsi_requests = {
     .name = "scsi-requests",
     .get  = get_scsi_requests,
@@ -1575,7 +1589,7 @@ static void scsi_device_class_init(ObjectClass *klass, void *data)
     DeviceClass *k = DEVICE_CLASS(klass);
     k->bus_info = &scsi_bus_info;
     k->init     = scsi_qdev_init;
-    k->unplug   = qdev_simple_unplug_cb;
+    k->unplug   = scsi_qdev_unplug;
     k->exit     = scsi_qdev_exit;
 }
 
hw/scsi.h +2 -0
index 2eb66f7..5768071 100644
@@ -130,6 +130,8 @@ struct SCSIBusInfo {
     void (*transfer_data)(SCSIRequest *req, uint32_t arg);
     void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid);
     void (*cancel)(SCSIRequest *req);
+    void (*hotplug)(SCSIBus *bus, SCSIDevice *dev);
+    void (*hot_unplug)(SCSIBus *bus, SCSIDevice *dev);
     QEMUSGList *(*get_sg_list)(SCSIRequest *req);
 
     void (*save_request)(QEMUFile *f, SCSIRequest *req);
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
#7 lore raw

Re: [Qemu-devel] [PATCH 2/2] virtio-scsi: Implement hotplug support for virtio-scsi

In reply to Cong Meng
Quoted 10 lines · Cong Meng

Implement the hotplug() and hot_unplug() interfaces in virtio-scsi, by signal the virtio_scsi.ko in guest kernel via event virtual queue.

The counterpart patch of virtio_scsi.ko will be sent soon in another thread.

---
 hw/virtio-scsi.c |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++--

 1 files changed, 69 insertions(+), 3 deletions(-)

I compared against the virtio-scsi specification and this looks good:
http://ozlabs.org/~rusty/virtio-spec/virtio-0.9.5.pdf

Dropped events and event throttling are not implemented by this patch.
This means that the guest can miss events if it runs out of event
queue elements. A scenario that might be able to trigger this is if multiple LUNs are hotplugged in a single QEMU monitor callback.

Implementing dropped events is easy in hw/virtio-scsi.c. Keep a bool or counter of dropped events and report them when the guest kicks us with a free event element (virtio_scsi_handle_event).

Stefan

#8 lore raw

Re: [Qemu-devel] [PATCH 2/2] virtio-scsi: Implement hotplug support for virtio-scsi

Quoting Stefan Hajnoczi <[email protected]>:

Quoted 16 lines · Cong Meng
Quoted 11 lines

Implement the hotplug() and hot_unplug() interfaces in virtio-scsi, by signal the virtio_scsi.ko in guest kernel via event virtual queue.

The counterpart patch of virtio_scsi.ko will be sent soon in another thread.

---
 hw/virtio-scsi.c |   72
+++++++++++++++++++++++++++++++++++++++++++++++++++--

 1 files changed, 69 insertions(+), 3 deletions(-)

I compared against the virtio-scsi specification and this looks good:
http://ozlabs.org/~rusty/virtio-spec/virtio-0.9.5.pdf

Quoted 8 lines

Dropped events and event throttling are not implemented by this patch.
This means that the guest can miss events if it runs out of event
queue elements. A scenario that might be able to trigger this is if multiple LUNs are hotplugged in a single QEMU monitor callback.

Implementing dropped events is easy in hw/virtio-scsi.c. Keep a bool or counter of dropped events and report them when the guest kicks us with a free event element (virtio_scsi_handle_event).

Yes. It's easy to do this in qemu. But I'm not sure what should be done in virtio-scsi.ko to respond the "VIRTIO_SCSI_T_EVENTS_MISSED" event. The spec says "poll the logical units for unit attention conditions", or just a whole bus rescan?

Quoted 2 lines

Stefan

#9 lore raw

Re: [Qemu-devel] [PATCH 2/2] virtio-scsi: Implement hotplug support for virtio-scsi

Quoted 25 lines · On Mon, Jun 25, 2012 at 03:51:13AM -0400, [email protected] wrote:

Quoting Stefan Hajnoczi <[email protected]>:

Quoted 22 lines · Cong Meng
Quoted 4 lines

Implement the hotplug() and hot_unplug() interfaces in virtio-scsi, by signal
the virtio_scsi.ko in guest kernel via event virtual queue.

Quoted 8 lines

The counterpart patch of virtio_scsi.ko will be sent soon in another thread.

---
 hw/virtio-scsi.c |   72
+++++++++++++++++++++++++++++++++++++++++++++++++++--

 1 files changed, 69 insertions(+), 3 deletions(-)

I compared against the virtio-scsi specification and this looks good:
http://ozlabs.org/~rusty/virtio-spec/virtio-0.9.5.pdf

Dropped events and event throttling are not implemented by this patch. This means that the guest can miss events if it runs out of event queue elements. A scenario that might be able to trigger this is if multiple LUNs are hotplugged in a single QEMU monitor callback.

Quoted 8 lines
Quoted 3 lines

Implementing dropped events is easy in hw/virtio-scsi.c. Keep a bool or counter of dropped events and report them when the guest kicks us with a free event element (virtio_scsi_handle_event).

Yes. It's easy to do this in qemu. But I'm not sure what should be done in virtio-scsi.ko to respond the "VIRTIO_SCSI_T_EVENTS_MISSED" event. The spec says "poll the logical units for unit attention conditions", or just a whole bus rescan?

I'm not sure what the answer is either, maybe you can find an existing SCSI LLD that does what you need.

Stefan