The device core first places a device on the bus and then realizes it. Make scsi_device_find avoid returing such devices to avoid races in drivers that use an iothread (currently virtio-scsi)
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1812399 Suggested-by: Paolo Bonzini <pbonz...@redhat.com> Signed-off-by: Maxim Levitsky <mlevi...@redhat.com> --- hw/scsi/scsi-bus.c | 88 ++++++++++++++++++++++++++++------------------ 1 file changed, 53 insertions(+), 35 deletions(-) diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index 92d412b65c..7ceae2c92b 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -51,6 +51,56 @@ static const TypeInfo scsi_bus_info = { }; static int next_scsi_bus; +static SCSIDevice *_scsi_device_find(SCSIBus *bus, int channel, int id, int lun, + bool include_unrealized) +{ + BusChild *kid; + SCSIDevice *retval = NULL; + + rcu_read_lock(); + + QTAILQ_FOREACH_RCU(kid, &bus->qbus.children, sibling) { + DeviceState *qdev = kid->child; + SCSIDevice *dev = SCSI_DEVICE(qdev); + + if (dev->channel == channel && dev->id == id) { + if (dev->lun == lun) { + retval = dev; + goto out; + } + + /* + * If we don't find exact match (channel/bus/lun), + * we will return the first device which matches channel/bus + */ + + if (!retval) { + retval = dev; + } + } + } +out: + /* + * This function might run on the IO thread and we might race against + * main thread hot-plugging the device. + * We assume that as soon as .realized is set to true we can let + * the user access the device. + */ + + if (retval && !include_unrealized && + !atomic_load_acquire(&retval->qdev.realized)) { + retval = NULL; + } + + rcu_read_unlock(); + return retval; +} + +SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun) +{ + return _scsi_device_find(bus, channel, id, lun, false); +} + static void scsi_device_realize(SCSIDevice *s, Error **errp) { SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(s); @@ -186,7 +236,7 @@ static void scsi_qdev_realize(DeviceState *qdev, Error **errp) dev->lun = 0; } do { - d = scsi_device_find(bus, dev->channel, ++id, dev->lun); + d = _scsi_device_find(bus, dev->channel, ++id, dev->lun, true); } while (d && d->lun == dev->lun && id < bus->info->max_target); if (d && d->lun == dev->lun) { error_setg(errp, "no free target"); @@ -196,7 +246,7 @@ static void scsi_qdev_realize(DeviceState *qdev, Error **errp) } else if (dev->lun == -1) { int lun = -1; do { - d = scsi_device_find(bus, dev->channel, dev->id, ++lun); + d = _scsi_device_find(bus, dev->channel, dev->id, ++lun, true); } while (d && d->lun == lun && lun < bus->info->max_lun); if (d && d->lun == lun) { error_setg(errp, "no free lun"); @@ -204,7 +254,7 @@ static void scsi_qdev_realize(DeviceState *qdev, Error **errp) } dev->lun = lun; } else { - d = scsi_device_find(bus, dev->channel, dev->id, dev->lun); + d = _scsi_device_find(bus, dev->channel, dev->id, dev->lun, true); assert(d); if (d->lun == dev->lun && dev != d) { error_setg(errp, "lun already used by '%s'", d->qdev.id); @@ -1573,38 +1623,6 @@ static char *scsibus_get_fw_dev_path(DeviceState *dev) qdev_fw_name(dev), d->id, d->lun); } -SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun) -{ - BusChild *kid; - SCSIDevice *target_dev = NULL; - - rcu_read_lock(); - - QTAILQ_FOREACH_RCU(kid, &bus->qbus.children, sibling) { - DeviceState *qdev = kid->child; - SCSIDevice *dev = SCSI_DEVICE(qdev); - - if (dev->channel == channel && dev->id == id) { - if (dev->lun == lun) { - rcu_read_unlock(); - return dev; - } - - /* - * If we don't find exact match (channel/bus/lun), - * we will return the first device which matches channel/bus - */ - - if (!target_dev) { - target_dev = dev; - } - } - } - - rcu_read_unlock(); - return target_dev; -} - /* SCSI request list. For simplicity, pv points to the whole device */ static int put_scsi_requests(QEMUFile *f, void *pv, size_t size, -- 2.26.2
