Re: [Qemu-devel] [PATCH v8 05/16] virtio-scsi: Catch BDS-BB removal/insertion

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2890 bytes --]

On 29.01.2016 13:41, Kevin Wolf wrote:
> Am 27.01.2016 um 18:59 hat Max Reitz geschrieben:
>> Make use of the BDS-BB removal and insertion notifiers to remove or set
>> up, respectively, virtio-scsi's op blockers.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  hw/scsi/virtio-scsi.c           | 55 +++++++++++++++++++++++++++++++++++++++++
>>  include/hw/virtio/virtio-scsi.h | 10 ++++++++
>>  2 files changed, 65 insertions(+)
>>
>> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
>> index 607593c..b508b81 100644
>> --- a/hw/scsi/virtio-scsi.c
>> +++ b/hw/scsi/virtio-scsi.c
>> @@ -757,6 +757,22 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense)
>>      }
>>  }
>>  
>> +static void virtio_scsi_blk_insert_notifier(Notifier *n, void *data)
>> +{
>> +    VirtIOSCSIBlkChangeNotifier *cn = DO_UPCAST(VirtIOSCSIBlkChangeNotifier,
>> +                                                n, n);
>> +    assert(cn->sd->conf.blk == data);
>> +    blk_op_block_all(cn->sd->conf.blk, cn->s->blocker);
>> +}
>> +
>> +static void virtio_scsi_blk_remove_notifier(Notifier *n, void *data)
>> +{
>> +    VirtIOSCSIBlkChangeNotifier *cn = DO_UPCAST(VirtIOSCSIBlkChangeNotifier,
>> +                                                n, n);
>> +    assert(cn->sd->conf.blk == data);
>> +    blk_op_unblock_all(cn->sd->conf.blk, cn->s->blocker);
>> +}
>> +
>>  static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
>>                                  Error **errp)
>>  {
>> @@ -765,6 +781,22 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
>>      SCSIDevice *sd = SCSI_DEVICE(dev);
>>  
>>      if (s->ctx && !s->dataplane_disabled) {
>> +        VirtIOSCSIBlkChangeNotifier *insert_notifier, *remove_notifier;
>> +
>> +        insert_notifier = g_new0(VirtIOSCSIBlkChangeNotifier, 1);
>> +        insert_notifier->n.notify = virtio_scsi_blk_insert_notifier;
>> +        insert_notifier->s = s;
>> +        insert_notifier->sd = sd;
>> +        blk_add_insert_bs_notifier(sd->conf.blk, &insert_notifier->n);
>> +        QTAILQ_INSERT_TAIL(&s->insert_notifiers, insert_notifier, next);
>> +
>> +        remove_notifier = g_new0(VirtIOSCSIBlkChangeNotifier, 1);
>> +        remove_notifier->n.notify = virtio_scsi_blk_remove_notifier;
>> +        remove_notifier->s = s;
>> +        remove_notifier->sd = sd;
>> +        blk_add_remove_bs_notifier(sd->conf.blk, &remove_notifier->n);
>> +        QTAILQ_INSERT_TAIL(&s->remove_notifiers, remove_notifier, next);
>> +
>>          if (blk_op_is_blocked(sd->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) {
>>              return;
>>          }
> 
> If we take the error path here, won't we have dangling pointers in the
> notifier list?

Yes, I'll move it below that error path.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]