[Qemu-devel] [PATCH 0/6] Acquire the AioContext during _realize()

[Qemu-devel] [PATCH 0/6] Acquire the AioContext during _realize()

[Alberto Garcia]

As discussed yesterday, this series acquires the AioContext in the
_realize() functions of several devices before making use of their
block backends. This fixes at least a couple of crashes (in virtio-blk
and scsi).

Berto

Alberto Garcia (6):
  block: Acquire the AioContext in virtio_blk_device_realize()
  block: Acquire the AioContext in scsi_*_realize()
  block: Acquire the AioContext in floppy_drive_realize()
  block: Acquire the AioContext in nvme_realize()
  block: Acquire the AioContext in ide_dev_initfn()
  block: Acquire the AioContext in usb_msd_storage_realize()

 hw/block/fdc.c        | 15 +++++++++++----
 hw/block/nvme.c       | 13 ++++++++++---
 hw/block/virtio-blk.c | 22 +++++++++++++++-------
 hw/ide/qdev.c         | 17 ++++++++++++-----
 hw/scsi/scsi-disk.c   | 24 +++++++++++++++++++-----
 hw/usb/dev-storage.c  |  5 +++++
 6 files changed, 72 insertions(+), 24 deletions(-)

-- 
2.11.0

[Qemu-devel] [PATCH 1/6] block: Acquire the AioContext in virtio_blk_device_realize()

[Alberto Garcia]

This fixes the following crash:

{ "execute": "blockdev-add",
  "arguments": {"driver": "null-co", "node-name": "hd0"}}
{ "execute": "object-add",
  "arguments": {"qom-type": "iothread", "id": "iothread0"}}
{ "execute": "x-blockdev-set-iothread",
  "arguments": {"node-name": "hd0", "iothread": "iothread0"}}
{ "execute": "device_add",
  "arguments": {"id": "virtio0", "driver": "virtio-blk-pci",
                "drive": "hd0"}}
qemu: qemu_mutex_unlock_impl: Operation not permitted
Aborted

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 hw/block/virtio-blk.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index f208c6ddb9..5357da82af 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -912,6 +912,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOBlock *s = VIRTIO_BLK(dev);
     VirtIOBlkConf *conf = &s->conf;
+    AioContext *ctx;
     Error *err = NULL;
     unsigned i;
 
@@ -919,30 +920,34 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
         error_setg(errp, "drive property not set");
         return;
     }
+
+    ctx = blk_get_aio_context(conf->conf.blk);
+    aio_context_acquire(ctx);
+
     if (!blk_is_inserted(conf->conf.blk)) {
         error_setg(errp, "Device needs media, but drive is empty");
-        return;
+        goto out;
     }
     if (!conf->num_queues) {
         error_setg(errp, "num-queues property must be larger than 0");
-        return;
+        goto out;
     }
     if (!is_power_of_2(conf->queue_size) ||
         conf->queue_size > VIRTQUEUE_MAX_SIZE) {
         error_setg(errp, "invalid queue-size property (%" PRIu16 "), "
                    "must be a power of 2 (max %d)",
                    conf->queue_size, VIRTQUEUE_MAX_SIZE);
-        return;
+        goto out;
     }
 
     if (!blkconf_apply_backend_options(&conf->conf,
                                        blk_is_read_only(conf->conf.blk), true,
                                        errp)) {
-        return;
+        goto out;
     }
     s->original_wce = blk_enable_write_cache(conf->conf.blk);
     if (!blkconf_geometry(&conf->conf, NULL, 65535, 255, 255, errp)) {
-        return;
+        goto out;
     }
 
     blkconf_blocksizes(&conf->conf);
@@ -951,7 +956,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
         conf->conf.physical_block_size) {
         error_setg(errp,
                    "logical_block_size > physical_block_size not supported");
-        return;
+        goto out;
     }
 
     virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
@@ -968,7 +973,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
     if (err != NULL) {
         error_propagate(errp, err);
         virtio_cleanup(vdev);
-        return;
+        goto out;
     }
 
     s->change = qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
@@ -976,6 +981,9 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
     blk_set_guest_block_size(s->blk, s->conf.conf.logical_block_size);
 
     blk_iostatus_enable(s->blk);
+
+out:
+    aio_context_release(ctx);
 }
 
 static void virtio_blk_device_unrealize(DeviceState *dev, Error **errp)
-- 
2.11.0

[Qemu-devel] [PATCH 2/6] block: Acquire the AioContext in scsi_*_realize()

[Alberto Garcia]

This fixes the following crash:

{ "execute": "blockdev-add",
  "arguments": {"driver": "null-co", "node-name": "hd0"}}
{ "execute": "object-add",
  "arguments": {"qom-type": "iothread", "id": "iothread0"}}
{ "execute": "x-blockdev-set-iothread",
  "arguments": {"node-name": "hd0", "iothread": "iothread0"}}
{ "execute": "device_add",
   "arguments": {"id": "scsi-pci0", "driver": "virtio-scsi-pci"}}
{ "execute": "device_add",
  "arguments": {"id": "scsi-hd0", "driver": "scsi-hd", "drive": "hd0"}}
qemu: qemu_mutex_unlock_impl: Operation not permitted
Aborted

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 hw/scsi/scsi-disk.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 0e9027c8f3..8a22def7f3 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2318,16 +2318,20 @@ static void scsi_disk_unit_attention_reported(SCSIDevice *dev)
 static void scsi_realize(SCSIDevice *dev, Error **errp)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+    AioContext *ctx;
 
     if (!s->qdev.conf.blk) {
         error_setg(errp, "drive property not set");
         return;
     }
 
+    ctx = blk_get_aio_context(s->qdev.conf.blk);
+    aio_context_acquire(ctx);
+
     if (!(s->features & (1 << SCSI_DISK_F_REMOVABLE)) &&
         !blk_is_inserted(s->qdev.conf.blk)) {
         error_setg(errp, "Device needs media, but drive is empty");
-        return;
+        goto out;
     }
 
     blkconf_blocksizes(&s->qdev.conf);
@@ -2336,18 +2340,18 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
         s->qdev.conf.physical_block_size) {
         error_setg(errp,
                    "logical_block_size > physical_block_size not supported");
-        return;
+        goto out;
     }
 
     if (dev->type == TYPE_DISK) {
         if (!blkconf_geometry(&dev->conf, NULL, 65535, 255, 255, errp)) {
-            return;
+            goto out;
         }
     }
     if (!blkconf_apply_backend_options(&dev->conf,
                                        blk_is_read_only(s->qdev.conf.blk),
                                        dev->type == TYPE_DISK, errp)) {
-        return;
+        goto out;
     }
 
     if (s->qdev.conf.discard_granularity == -1) {
@@ -2364,7 +2368,7 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
 
     if (blk_is_sg(s->qdev.conf.blk)) {
         error_setg(errp, "unwanted /dev/sg*");
-        return;
+        goto out;
     }
 
     if ((s->features & (1 << SCSI_DISK_F_REMOVABLE)) &&
@@ -2376,6 +2380,9 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
     blk_set_guest_block_size(s->qdev.conf.blk, s->qdev.blocksize);
 
     blk_iostatus_enable(s->qdev.conf.blk);
+
+out:
+    aio_context_release(ctx);
 }
 
 static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
@@ -2385,7 +2392,10 @@ static void scsi_hd_realize(SCSIDevice *dev, Error **errp)
      * backend will be issued in scsi_realize
      */
     if (s->qdev.conf.blk) {
+        AioContext *ctx = blk_get_aio_context(s->qdev.conf.blk);
+        aio_context_acquire(ctx);
         blkconf_blocksizes(&s->qdev.conf);
+        aio_context_release(ctx);
     }
     s->qdev.blocksize = s->qdev.conf.logical_block_size;
     s->qdev.type = TYPE_DISK;
@@ -2553,6 +2563,7 @@ static int get_device_type(SCSIDiskState *s)
 static void scsi_block_realize(SCSIDevice *dev, Error **errp)
 {
     SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+    AioContext *ctx;
     int sg_version;
     int rc;
 
@@ -2568,7 +2579,10 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp)
     }
 
     /* check we are using a driver managing SG_IO (version 3 and after) */
+    ctx = blk_get_aio_context(s->qdev.conf.blk);
+    aio_context_acquire(ctx);
     rc = blk_ioctl(s->qdev.conf.blk, SG_GET_VERSION_NUM, &sg_version);
+    aio_context_release(ctx);
     if (rc < 0) {
         error_setg_errno(errp, -rc, "cannot get SG_IO version number");
         if (rc != -EPERM) {
-- 
2.11.0

[Qemu-devel] [PATCH 3/6] block: Acquire the AioContext in floppy_drive_realize()

[Alberto Garcia]

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 hw/block/fdc.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 6f19f127a5..d9bc80de83 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -512,6 +512,7 @@ static void floppy_drive_realize(DeviceState *qdev, Error **errp)
 {
     FloppyDrive *dev = FLOPPY_DRIVE(qdev);
     FloppyBus *bus = FLOPPY_BUS(qdev->parent_bus);
+    AioContext *ctx;
     FDrive *drive;
     int ret;
 
@@ -543,13 +544,16 @@ static void floppy_drive_realize(DeviceState *qdev, Error **errp)
         assert(ret == 0);
     }
 
+    ctx = blk_get_aio_context(dev->conf.blk);
+    aio_context_acquire(ctx);
+
     blkconf_blocksizes(&dev->conf);
     if (dev->conf.logical_block_size != 512 ||
         dev->conf.physical_block_size != 512)
     {
         error_setg(errp, "Physical and logical block size must "
                    "be 512 for floppy");
-        return;
+        goto out;
     }
 
     /* rerror/werror aren't supported by fdc and therefore not even registered
@@ -561,7 +565,7 @@ static void floppy_drive_realize(DeviceState *qdev, Error **errp)
     if (!blkconf_apply_backend_options(&dev->conf,
                                        blk_is_read_only(dev->conf.blk),
                                        false, errp)) {
-        return;
+        goto out;
     }
 
     /* 'enospc' is the default for -drive, 'report' is what blk_new() gives us
@@ -569,11 +573,11 @@ static void floppy_drive_realize(DeviceState *qdev, Error **errp)
     if (blk_get_on_error(dev->conf.blk, 0) != BLOCKDEV_ON_ERROR_ENOSPC &&
         blk_get_on_error(dev->conf.blk, 0) != BLOCKDEV_ON_ERROR_REPORT) {
         error_setg(errp, "fdc doesn't support drive option werror");
-        return;
+        goto out;
     }
     if (blk_get_on_error(dev->conf.blk, 1) != BLOCKDEV_ON_ERROR_REPORT) {
         error_setg(errp, "fdc doesn't support drive option rerror");
-        return;
+        goto out;
     }
 
     drive->conf = &dev->conf;
@@ -589,6 +593,9 @@ static void floppy_drive_realize(DeviceState *qdev, Error **errp)
     dev->type = drive->drive;
 
     fd_revalidate(drive);
+
+out:
+    aio_context_release(ctx);
 }
 
 static void floppy_drive_class_init(ObjectClass *klass, void *data)
-- 
2.11.0

[Qemu-devel] [PATCH 4/6] block: Acquire the AioContext in nvme_realize()

[Alberto Garcia]

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 hw/block/nvme.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 7c8c63e8f5..72e94aff86 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1203,6 +1203,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
 {
     NvmeCtrl *n = NVME(pci_dev);
     NvmeIdCtrl *id = &n->id_ctrl;
+    AioContext *ctx;
 
     int i;
     int64_t bs_size;
@@ -1213,20 +1214,23 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
         return;
     }
 
+    ctx = blk_get_aio_context(n->conf.blk);
+    aio_context_acquire(ctx);
+
     bs_size = blk_getlength(n->conf.blk);
     if (bs_size < 0) {
         error_setg(errp, "could not get backing file size");
-        return;
+        goto out;
     }
 
     if (!n->serial) {
         error_setg(errp, "serial property not set");
-        return;
+        goto out;
     }
     blkconf_blocksizes(&n->conf);
     if (!blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
                                        false, errp)) {
-        return;
+        goto out;
     }
 
     pci_conf = pci_dev->config;
@@ -1323,6 +1327,9 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
             cpu_to_le64(n->ns_size >>
                 id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas)].ds);
     }
+
+out:
+    aio_context_release(ctx);
 }
 
 static void nvme_exit(PCIDevice *pci_dev)
-- 
2.11.0

[Qemu-devel] [PATCH 5/6] block: Acquire the AioContext in ide_dev_initfn()

[Alberto Garcia]

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 hw/ide/qdev.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 573b022e1e..f355f2a352 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -160,6 +160,7 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
 {
     IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
     IDEState *s = bus->ifs + dev->unit;
+    AioContext *ctx;
     int ret;
 
     if (!dev->conf.blk) {
@@ -174,36 +175,39 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
         }
     }
 
+    ctx = blk_get_aio_context(dev->conf.blk);
+    aio_context_acquire(ctx);
+
     if (dev->conf.discard_granularity == -1) {
         dev->conf.discard_granularity = 512;
     } else if (dev->conf.discard_granularity &&
                dev->conf.discard_granularity != 512) {
         error_setg(errp, "discard_granularity must be 512 for ide");
-        return;
+        goto out;
     }
 
     blkconf_blocksizes(&dev->conf);
     if (dev->conf.logical_block_size != 512) {
         error_setg(errp, "logical_block_size must be 512 for IDE");
-        return;
+        goto out;
     }
 
     if (kind != IDE_CD) {
         if (!blkconf_geometry(&dev->conf, &dev->chs_trans, 65535, 16, 255,
                               errp)) {
-            return;
+            goto out;
         }
     }
     if (!blkconf_apply_backend_options(&dev->conf, kind == IDE_CD,
                                        kind != IDE_CD, errp)) {
-        return;
+        goto out;
     }
 
     if (ide_init_drive(s, dev->conf.blk, kind,
                        dev->version, dev->serial, dev->model, dev->wwn,
                        dev->conf.cyls, dev->conf.heads, dev->conf.secs,
                        dev->chs_trans, errp) < 0) {
-        return;
+        goto out;
     }
 
     if (!dev->version) {
@@ -215,6 +219,9 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
 
     add_boot_device_path(dev->conf.bootindex, &dev->qdev,
                          dev->unit ? "/disk@1" : "/disk@0");
+
+out:
+    aio_context_release(ctx);
 }
 
 static void ide_dev_get_bootindex(Object *obj, Visitor *v, const char *name,
-- 
2.11.0

[Qemu-devel] [PATCH 6/6] block: Acquire the AioContext in usb_msd_storage_realize()

[Alberto Garcia]

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 hw/usb/dev-storage.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index cd5551d94f..fa6e552f0f 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -593,13 +593,18 @@ static void usb_msd_storage_realize(USBDevice *dev, Error **errp)
     MSDState *s = USB_STORAGE_DEV(dev);
     BlockBackend *blk = s->conf.blk;
     SCSIDevice *scsi_dev;
+    AioContext *ctx;
 
     if (!blk) {
         error_setg(errp, "drive property not set");
         return;
     }
 
+    ctx = blk_get_aio_context(blk);
+    aio_context_acquire(ctx);
     blkconf_blocksizes(&s->conf);
+    aio_context_release(ctx);
+
     if (!blkconf_apply_backend_options(&s->conf, blk_is_read_only(blk), true,
                                        errp)) {
         return;
-- 
2.11.0

Re: [Qemu-devel] [PATCH 0/6] Acquire the AioContext during _realize()

[Markus Armbruster]

For what it's worth, this series seems to fix assertion failures related
to AioContext acquire/release I've seen.

Re: [Qemu-devel] [PATCH 2/6] block: Acquire the AioContext in scsi_*_realize()

[Kevin Wolf]

Am 10.01.2019 um 16:03 hat Alberto Garcia geschrieben:
> This fixes the following crash:
> 
> { "execute": "blockdev-add",
>   "arguments": {"driver": "null-co", "node-name": "hd0"}}
> { "execute": "object-add",
>   "arguments": {"qom-type": "iothread", "id": "iothread0"}}
> { "execute": "x-blockdev-set-iothread",
>   "arguments": {"node-name": "hd0", "iothread": "iothread0"}}
> { "execute": "device_add",
>    "arguments": {"id": "scsi-pci0", "driver": "virtio-scsi-pci"}}
> { "execute": "device_add",
>   "arguments": {"id": "scsi-hd0", "driver": "scsi-hd", "drive": "hd0"}}
> qemu: qemu_mutex_unlock_impl: Operation not permitted
> Aborted
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>

> @@ -2553,6 +2563,7 @@ static int get_device_type(SCSIDiskState *s)
>  static void scsi_block_realize(SCSIDevice *dev, Error **errp)
>  {
>      SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
> +    AioContext *ctx;
>      int sg_version;
>      int rc;
>  
> @@ -2568,7 +2579,10 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp)
>      }
>  
>      /* check we are using a driver managing SG_IO (version 3 and after) */
> +    ctx = blk_get_aio_context(s->qdev.conf.blk);
> +    aio_context_acquire(ctx);
>      rc = blk_ioctl(s->qdev.conf.blk, SG_GET_VERSION_NUM, &sg_version);
> +    aio_context_release(ctx);
>      if (rc < 0) {
>          error_setg_errno(errp, -rc, "cannot get SG_IO version number");
>          if (rc != -EPERM) {

This is probably not enough. get_device_type() and
scsi_generic_read_device_inquiry() below issue more ioctls (but we need
to be careful not to include the scsi_realize() call in the locked
section if you take the lock again there).

Kevin

Re: [Qemu-devel] [PATCH 2/6] block: Acquire the AioContext in scsi_*_realize()

[Alberto Garcia]

On Fri 11 Jan 2019 04:02:13 PM CET, Kevin Wolf wrote:
>> @@ -2568,7 +2579,10 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp)
>>      }
>>  
>>      /* check we are using a driver managing SG_IO (version 3 and after) */
>> +    ctx = blk_get_aio_context(s->qdev.conf.blk);
>> +    aio_context_acquire(ctx);
>>      rc = blk_ioctl(s->qdev.conf.blk, SG_GET_VERSION_NUM, &sg_version);
>> +    aio_context_release(ctx);
>>      if (rc < 0) {
>>          error_setg_errno(errp, -rc, "cannot get SG_IO version number");
>>          if (rc != -EPERM) {
>
> This is probably not enough. get_device_type() and
> scsi_generic_read_device_inquiry() below issue more ioctls (but we
> need to be careful not to include the scsi_realize() call in the
> locked section if you take the lock again there).

Hmmm, another alternative is not to take the lock in scsi_realize() and
take it instead in all the functions that call it (it's 4 or 5).

Berto

Re: [Qemu-devel] [PATCH 0/6] Acquire the AioContext during _realize()

[Kevin Wolf]

Am 10.01.2019 um 16:03 hat Alberto Garcia geschrieben:
> As discussed yesterday, this series acquires the AioContext in the
> _realize() functions of several devices before making use of their
> block backends. This fixes at least a couple of crashes (in virtio-blk
> and scsi).

Apart from the incomplete scsi-block fix, this looks good. Of course,
I think patches 3-6 are completely optional because none of these
devices actually support iothreads, so we can't hit a bug. But it might
be nicer to add it there as well anyway.

For patches 1 and 2, which actually fix bugs, it would be nice to have
an accompanying test case.

Kevin

Re: [Qemu-devel] [PATCH 0/6] Acquire the AioContext during _realize()

[Alberto Garcia]

On Fri 11 Jan 2019 04:09:17 PM CET, Kevin Wolf wrote:
> Am 10.01.2019 um 16:03 hat Alberto Garcia geschrieben:
>> As discussed yesterday, this series acquires the AioContext in the
>> _realize() functions of several devices before making use of their
>> block backends. This fixes at least a couple of crashes (in virtio-blk
>> and scsi).
>
> Apart from the incomplete scsi-block fix, this looks good. Of course,
> I think patches 3-6 are completely optional because none of these
> devices actually support iothreads, so we can't hit a bug. But it
> might be nicer to add it there as well anyway.
>
> For patches 1 and 2, which actually fix bugs, it would be nice to have
> an accompanying test case.

Ok, I'll prepare v2 with your suggestions.

Berto

Re: [Qemu-devel] [PATCH 2/6] block: Acquire the AioContext in scsi_*_realize()

[Kevin Wolf]

Am 11.01.2019 um 16:05 hat Alberto Garcia geschrieben:
> On Fri 11 Jan 2019 04:02:13 PM CET, Kevin Wolf wrote:
> >> @@ -2568,7 +2579,10 @@ static void scsi_block_realize(SCSIDevice *dev, Error **errp)
> >>      }
> >>  
> >>      /* check we are using a driver managing SG_IO (version 3 and after) */
> >> +    ctx = blk_get_aio_context(s->qdev.conf.blk);
> >> +    aio_context_acquire(ctx);
> >>      rc = blk_ioctl(s->qdev.conf.blk, SG_GET_VERSION_NUM, &sg_version);
> >> +    aio_context_release(ctx);
> >>      if (rc < 0) {
> >>          error_setg_errno(errp, -rc, "cannot get SG_IO version number");
> >>          if (rc != -EPERM) {
> >
> > This is probably not enough. get_device_type() and
> > scsi_generic_read_device_inquiry() below issue more ioctls (but we
> > need to be careful not to include the scsi_realize() call in the
> > locked section if you take the lock again there).
> 
> Hmmm, another alternative is not to take the lock in scsi_realize() and
> take it instead in all the functions that call it (it's 4 or 5).

We touch most of them anyway, so I think it is an option.

Kevin