Merge remote-tracking branch 'bonzini/scsi-next' into staging

* bonzini/scsi-next:
  virtio-scsi: add backwards-compatibility properties for 1.1 and earlier machines
  iscsi: fix races between task completion and abort
  iscsi: simplify iscsi_schedule_bh
  iscsi: move iscsi_schedule_bh and iscsi_readv_writev_bh_cb
  Revert "iscsi: Fix NULL dereferences / races between task completion and abort"

4 files changed, 79 insertions, 67 deletions

diff --git a/block/iscsi.c b/block/iscsi.c
index bb9cf82459..4828b83927 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -73,13 +73,43 @@ struct IscsiTask {
 };
 
 static void
+iscsi_bh_cb(void *p)
+{
+    IscsiAIOCB *acb = p;
+
+    qemu_bh_delete(acb->bh);
+
+    if (acb->canceled == 0) {
+        acb->common.cb(acb->common.opaque, acb->status);
+    }
+
+    if (acb->task != NULL) {
+        scsi_free_scsi_task(acb->task);
+        acb->task = NULL;
+    }
+
+    qemu_aio_release(acb);
+}
+
+static void
+iscsi_schedule_bh(IscsiAIOCB *acb)
+{
+    if (acb->bh) {
+        return;
+    }
+    acb->bh = qemu_bh_new(iscsi_bh_cb, acb);
+    qemu_bh_schedule(acb->bh);
+}
+
+
+static void
 iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void *command_data,
                     void *private_data)
 {
-    IscsiAIOCB *acb = (IscsiAIOCB *)private_data;
+    IscsiAIOCB *acb = private_data;
 
-    scsi_free_scsi_task(acb->task);
-    acb->task = NULL;
+    acb->status = -ECANCELED;
+    iscsi_schedule_bh(acb);
 }
 
 static void
@@ -88,15 +118,19 @@ iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
     IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
     IscsiLun *iscsilun = acb->iscsilun;
 
-    acb->canceled = 1;
+    if (acb->status != -EINPROGRESS) {
+        return;
+    }
 
-    acb->common.cb(acb->common.opaque, -ECANCELED);
+    acb->canceled = 1;
 
-    /* send a task mgmt call to the target to cancel the task on the target
-     * this also cancels the task in libiscsi
-     */
+    /* send a task mgmt call to the target to cancel the task on the target */
     iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
-                                     iscsi_abort_task_cb, &acb);
+                                     iscsi_abort_task_cb, acb);
+
+    while (acb->status == -EINPROGRESS) {
+        qemu_aio_wait();
+    }
 }
 
 static AIOPool iscsi_aio_pool = {
@@ -163,41 +197,6 @@ iscsi_process_write(void *arg)
 }
 
 
-static int
-iscsi_schedule_bh(QEMUBHFunc *cb, IscsiAIOCB *acb)
-{
-    acb->bh = qemu_bh_new(cb, acb);
-    if (!acb->bh) {
-        error_report("oom: could not create iscsi bh");
-        return -EIO;
-    }
-
-    qemu_bh_schedule(acb->bh);
-    return 0;
-}
-
-static void
-iscsi_readv_writev_bh_cb(void *p)
-{
-    IscsiAIOCB *acb = p;
-
-    qemu_bh_delete(acb->bh);
-
-    if (!acb->canceled) {
-        acb->common.cb(acb->common.opaque, acb->status);
-    }
-
-    qemu_aio_release(acb);
-
-    if (acb->canceled) {
-        return;
-    }
-
-    scsi_free_scsi_task(acb->task);
-    acb->task = NULL;
-}
-
-
 static void
 iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status,
                      void *command_data, void *opaque)
@@ -208,8 +207,7 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status,
 
     g_free(acb->buf);
 
-    if (acb->canceled) {
-        qemu_aio_release(acb);
+    if (acb->canceled != 0) {
         return;
     }
 
@@ -220,7 +218,7 @@ iscsi_aio_write16_cb(struct iscsi_context *iscsi, int status,
         acb->status = -EIO;
     }
 
-    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+    iscsi_schedule_bh(acb);
 }
 
 static int64_t sector_qemu2lun(int64_t sector, IscsiLun *iscsilun)
@@ -249,6 +247,8 @@ iscsi_aio_writev(BlockDriverState *bs, int64_t sector_num,
     acb->qiov     = qiov;
 
     acb->canceled   = 0;
+    acb->bh         = NULL;
+    acb->status     = -EINPROGRESS;
 
     /* XXX we should pass the iovec to write16 to avoid the extra copy */
     /* this will allow us to get rid of 'buf' completely */
@@ -305,8 +305,7 @@ iscsi_aio_read16_cb(struct iscsi_context *iscsi, int status,
 
     trace_iscsi_aio_read16_cb(iscsi, status, acb, acb->canceled);
 
-    if (acb->canceled) {
-        qemu_aio_release(acb);
+    if (acb->canceled != 0) {
         return;
     }
 
@@ -317,7 +316,7 @@ iscsi_aio_read16_cb(struct iscsi_context *iscsi, int status,
         acb->status = -EIO;
     }
 
-    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+    iscsi_schedule_bh(acb);
 }
 
 static BlockDriverAIOCB *
@@ -343,6 +342,8 @@ iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num,
     acb->qiov     = qiov;
 
     acb->canceled    = 0;
+    acb->bh          = NULL;
+    acb->status      = -EINPROGRESS;
     acb->read_size   = qemu_read_size;
     acb->buf         = NULL;
 
@@ -417,8 +418,7 @@ iscsi_synccache10_cb(struct iscsi_context *iscsi, int status,
 {
     IscsiAIOCB *acb = opaque;
 
-    if (acb->canceled) {
-        qemu_aio_release(acb);
+    if (acb->canceled != 0) {
         return;
     }
 
@@ -429,7 +429,7 @@ iscsi_synccache10_cb(struct iscsi_context *iscsi, int status,
         acb->status = -EIO;
     }
 
-    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+    iscsi_schedule_bh(acb);
 }
 
 static BlockDriverAIOCB *
@@ -444,6 +444,8 @@ iscsi_aio_flush(BlockDriverState *bs,
 
     acb->iscsilun = iscsilun;
     acb->canceled   = 0;
+    acb->bh         = NULL;
+    acb->status     = -EINPROGRESS;
 
     acb->task = iscsi_synchronizecache10_task(iscsi, iscsilun->lun,
                                          0, 0, 0, 0,
@@ -467,8 +469,7 @@ iscsi_unmap_cb(struct iscsi_context *iscsi, int status,
 {
     IscsiAIOCB *acb = opaque;
 
-    if (acb->canceled) {
-        qemu_aio_release(acb);
+    if (acb->canceled != 0) {
         return;
     }
 
@@ -479,7 +480,7 @@ iscsi_unmap_cb(struct iscsi_context *iscsi, int status,
         acb->status = -EIO;
     }
 
-    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+    iscsi_schedule_bh(acb);
 }
 
 static BlockDriverAIOCB *
@@ -496,6 +497,8 @@ iscsi_aio_discard(BlockDriverState *bs,
 
     acb->iscsilun = iscsilun;
     acb->canceled   = 0;
+    acb->bh         = NULL;
+    acb->status     = -EINPROGRESS;
 
     list[0].lba = sector_qemu2lun(sector_num, iscsilun);
     list[0].num = nb_sectors * BDRV_SECTOR_SIZE / iscsilun->block_size;
@@ -523,8 +526,7 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
 {
     IscsiAIOCB *acb = opaque;
 
-    if (acb->canceled) {
-        qemu_aio_release(acb);
+    if (acb->canceled != 0) {
         return;
     }
 
@@ -552,7 +554,7 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
         memcpy(acb->ioh->sbp, &acb->task->datain.data[2], ss);
     }
 
-    iscsi_schedule_bh(iscsi_readv_writev_bh_cb, acb);
+    iscsi_schedule_bh(acb);
 }
 
 static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
@@ -570,6 +572,8 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
 
     acb->iscsilun = iscsilun;
     acb->canceled    = 0;
+    acb->bh          = NULL;
+    acb->status      = -EINPROGRESS;
     acb->buf         = NULL;
     acb->ioh         = buf;
 
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index d68dbb2320..88ff0411ca 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -360,6 +360,14 @@ static QEMUMachine pc_machine_v1_2 = {
 
 #define PC_COMPAT_1_1 \
         {\
+            .driver   = "virtio-scsi-pci",\
+            .property = "hotplug",\
+            .value    = "off",\
+        },{\
+            .driver   = "virtio-scsi-pci",\
+            .property = "param_change",\
+            .value    = "off",\
+        },{\
             .driver   = "VGA",\
             .property = "vgamem_mb",\
             .value    = stringify(8),\
diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index 5f737acd97..c1b47a8f4d 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -24,11 +24,6 @@
 #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
-#define VIRTIO_SCSI_F_CHANGE                   2
-
 /* Response codes */
 #define VIRTIO_SCSI_S_OK                       0
 #define VIRTIO_SCSI_S_OVERRUN                  1
@@ -561,8 +556,6 @@ 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);
-    requested_features |= (1UL << VIRTIO_SCSI_F_CHANGE);
     return requested_features;
 }
 
diff --git a/hw/virtio-scsi.h b/hw/virtio-scsi.h
index 4bc889de02..91924f6dfc 100644
--- a/hw/virtio-scsi.h
+++ b/hw/virtio-scsi.h
@@ -21,6 +21,11 @@
 /* The ID for virtio_scsi */
 #define VIRTIO_ID_SCSI  8
 
+/* Feature Bits */
+#define VIRTIO_SCSI_F_INOUT                    0
+#define VIRTIO_SCSI_F_HOTPLUG                  1
+#define VIRTIO_SCSI_F_CHANGE                   2
+
 struct VirtIOSCSIConf {
     uint32_t num_queues;
     uint32_t max_sectors;
@@ -31,6 +36,8 @@ struct VirtIOSCSIConf {
     DEFINE_VIRTIO_COMMON_FEATURES(_state, _features_field), \
     DEFINE_PROP_UINT32("num_queues", _state, _conf_field.num_queues, 1), \
     DEFINE_PROP_UINT32("max_sectors", _state, _conf_field.max_sectors, 0xFFFF), \
-    DEFINE_PROP_UINT32("cmd_per_lun", _state, _conf_field.cmd_per_lun, 128)
+    DEFINE_PROP_UINT32("cmd_per_lun", _state, _conf_field.cmd_per_lun, 128), \
+    DEFINE_PROP_BIT("hotplug", _state, _features_field, VIRTIO_SCSI_F_HOTPLUG, true), \
+    DEFINE_PROP_BIT("param_change", _state, _features_field, VIRTIO_SCSI_F_CHANGE, true)
 
 #endif /* _QEMU_VIRTIO_SCSI_H */