Re: [Qemu-devel] [PATCH v3 05/11] block: Move BDS close notifiers into BB

[Max Reitz <mreitz@redhat.com>]

On 2015-02-25 at 02:52, Fam Zheng wrote:
> On Tue, 02/24 10:35, Max Reitz wrote:
>> The only remaining user of the BDS close notifiers is NBD which uses
>> them to determine when a BDS tree is being ejected. This patch removes
>> the BDS-level close notifiers and adds a notifier list to the
>> BlockBackend structure that is invoked whenever a BDS is removed.
>>
>> Symmetrically to that, another notifier list is added that is invoked
>> whenever a BDS is inserted. The dataplane implementations for virtio-blk
>> and virtio-scsi use both notifier types for setting up and removing op
>> blockers. This is not only important for setting up the op blockers on
>> insertion, but also for removing them on ejection since bdrv_delete()
>> asserts that there are no op blockers set up.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   block.c                         |  7 ----
>>   block/block-backend.c           | 19 +++++++--
>>   blockdev-nbd.c                  | 36 +---------------
>>   hw/block/dataplane/virtio-blk.c | 93 +++++++++++++++++++++++++++++++++--------
>>   hw/scsi/virtio-scsi.c           | 64 ++++++++++++++++++++++++----
>>   include/block/block.h           |  1 -
>>   include/block/block_int.h       |  2 -
>>   include/hw/virtio/virtio-scsi.h |  8 ++++
>>   include/sysemu/block-backend.h  |  3 +-
>>   nbd.c                           | 35 ++++++++++++++++
>>   10 files changed, 193 insertions(+), 75 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 7b912c6..41a9d24 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -371,7 +371,6 @@ BlockDriverState *bdrv_new(void)
>>       for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) {
>>           QLIST_INIT(&bs->op_blockers[i]);
>>       }
>> -    notifier_list_init(&bs->close_notifiers);
>>       notifier_with_return_list_init(&bs->before_write_notifiers);
>>       qemu_co_queue_init(&bs->throttled_reqs[0]);
>>       qemu_co_queue_init(&bs->throttled_reqs[1]);
>> @@ -381,11 +380,6 @@ BlockDriverState *bdrv_new(void)
>>       return bs;
>>   }
>>   
>> -void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify)
>> -{
>> -    notifier_list_add(&bs->close_notifiers, notify);
>> -}
>> -
>>   BlockDriver *bdrv_find_format(const char *format_name)
>>   {
>>       BlockDriver *drv1;
>> @@ -1898,7 +1892,6 @@ void bdrv_close(BlockDriverState *bs)
>>       bdrv_drain_all(); /* complete I/O */
>>       bdrv_flush(bs);
>>       bdrv_drain_all(); /* in case flush left pending I/O */
>> -    notifier_list_notify(&bs->close_notifiers, bs);
>>   
>>       if (bs->drv) {
>>           if (bs->backing_hd) {
>> diff --git a/block/block-backend.c b/block/block-backend.c
>> index 82ced04..254fde4 100644
>> --- a/block/block-backend.c
>> +++ b/block/block-backend.c
>> @@ -47,6 +47,8 @@ struct BlockBackend {
>>       BlockdevOnError on_read_error, on_write_error;
>>       bool iostatus_enabled;
>>       BlockDeviceIoStatus iostatus;
>> +
>> +    NotifierList remove_bs_notifiers, insert_bs_notifiers;
>
>>   };
>>   
>>   typedef struct BlockBackendAIOCB {
>> @@ -97,6 +99,8 @@ BlockBackend *blk_new(const char *name, Error **errp)
>>       blk = g_new0(BlockBackend, 1);
>>       blk->name = g_strdup(name);
>>       blk->refcnt = 1;
>> +    notifier_list_init(&blk->remove_bs_notifiers);
>> +    notifier_list_init(&blk->insert_bs_notifiers);
>>       QTAILQ_INSERT_TAIL(&blk_backends, blk, link);
>>       return blk;
>>   }
>> @@ -320,6 +324,8 @@ void blk_remove_bs(BlockBackend *blk)
>>           return;
>>       }
>>   
>> +    notifier_list_notify(&blk->remove_bs_notifiers, blk);
>> +
>>       blk_update_root_state(blk);
>>   
>>       bdrv_unref(blk->bs);
>> @@ -345,6 +351,8 @@ void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
>>       }
>>   
>>       bs->blk = blk;
>> +
>> +    notifier_list_notify(&blk->insert_bs_notifiers, blk);
>>   }
>>   
>>   /*
>> @@ -1067,11 +1075,14 @@ void blk_remove_aio_context_notifier(BlockBackend *blk,
>>       }
>>   }
>>   
>> -void blk_add_close_notifier(BlockBackend *blk, Notifier *notify)
>> +void blk_add_remove_bs_notifier(BlockBackend *blk, Notifier *notify)
>>   {
>> -    if (blk->bs) {
>> -        bdrv_add_close_notifier(blk->bs, notify);
>> -    }
>> +    notifier_list_add(&blk->remove_bs_notifiers, notify);
>> +}
>> +
>> +void blk_add_insert_bs_notifier(BlockBackend *blk, Notifier *notify)
>> +{
>> +    notifier_list_add(&blk->insert_bs_notifiers, notify);
>>   }
>>   
>>   void blk_io_plug(BlockBackend *blk)
>> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
>> index 22e95d1..eb5f9a0 100644
>> --- a/blockdev-nbd.c
>> +++ b/blockdev-nbd.c
>> @@ -47,36 +47,11 @@ void qmp_nbd_server_start(SocketAddress *addr, Error **errp)
>>       }
>>   }
>>   
>> -/* Hook into the BlockDriverState notifiers to close the export when
>> - * the file is closed.
>> - */
>> -typedef struct NBDCloseNotifier {
>> -    Notifier n;
>> -    NBDExport *exp;
>> -    QTAILQ_ENTRY(NBDCloseNotifier) next;
>> -} NBDCloseNotifier;
>> -
>> -static QTAILQ_HEAD(, NBDCloseNotifier) close_notifiers =
>> -    QTAILQ_HEAD_INITIALIZER(close_notifiers);
>> -
>> -static void nbd_close_notifier(Notifier *n, void *data)
>> -{
>> -    NBDCloseNotifier *cn = DO_UPCAST(NBDCloseNotifier, n, n);
>> -
>> -    notifier_remove(&cn->n);
>> -    QTAILQ_REMOVE(&close_notifiers, cn, next);
>> -
>> -    nbd_export_close(cn->exp);
>> -    nbd_export_put(cn->exp);
>> -    g_free(cn);
>> -}
>> -
>>   void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
>>                           Error **errp)
>>   {
>>       BlockBackend *blk;
>>       NBDExport *exp;
>> -    NBDCloseNotifier *n;
>>   
>>       if (server_fd == -1) {
>>           error_setg(errp, "NBD server not running");
>> @@ -108,20 +83,11 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
>>       exp = nbd_export_new(blk, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY, NULL);
>>   
>>       nbd_export_set_name(exp, device);
>> -
>> -    n = g_new0(NBDCloseNotifier, 1);
>> -    n->n.notify = nbd_close_notifier;
>> -    n->exp = exp;
>> -    blk_add_close_notifier(blk, &n->n);
>> -    QTAILQ_INSERT_TAIL(&close_notifiers, n, next);
>>   }
>>   
>>   void qmp_nbd_server_stop(Error **errp)
>>   {
>> -    while (!QTAILQ_EMPTY(&close_notifiers)) {
>> -        NBDCloseNotifier *cn = QTAILQ_FIRST(&close_notifiers);
>> -        nbd_close_notifier(&cn->n, nbd_export_get_blockdev(cn->exp));
>> -    }
>> +    nbd_export_close_all();
>>   
>>       if (server_fd != -1) {
>>           qemu_set_fd_handler2(server_fd, NULL, NULL, NULL, NULL);
>> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
>> index cd41478..4cd1e45 100644
>> --- a/hw/block/dataplane/virtio-blk.c
>> +++ b/hw/block/dataplane/virtio-blk.c
>> @@ -26,6 +26,8 @@
>>   #include "hw/virtio/virtio-bus.h"
>>   #include "qom/object_interfaces.h"
>>   
>> +typedef struct DataPlaneBlkChangeNotifier DataPlaneBlkChangeNotifier;
>> +
>>   struct VirtIOBlockDataPlane {
>>       bool started;
>>       bool starting;
>> @@ -39,6 +41,8 @@ struct VirtIOBlockDataPlane {
>>       EventNotifier *guest_notifier;  /* irq */
>>       QEMUBH *bh;                     /* bh for guest notification */
>>   
>> +    DataPlaneBlkChangeNotifier *insert_notifier, *remove_notifier;
>> +
> Bikeshedding, but why not just embed these fields?

Because I need a Notifier pointer to give to 
blk_add_{insert,remove}_bs_notifier(), and most importantly because the 
only "opaque" object the callbacks will receive is that Notifier pointer 
(which in this case is actually a DataPlaneBlkChangeNotifier pointer). I 
cannot influence the @data parameter, and I cannot (easily) identify the 
VirtIOBlockDataPlane object from the BlockBackend (which is @data) alone.

>>       /* Note that these EventNotifiers are assigned by value.  This is
>>        * fine as long as you do not call event_notifier_cleanup on them
>>        * (because you don't own the file descriptor or handle; you just
>> @@ -55,6 +59,11 @@ struct VirtIOBlockDataPlane {
>>                                      unsigned char status);
>>   };
>>   
>> +struct DataPlaneBlkChangeNotifier {
>> +    Notifier n;
>> +    VirtIOBlockDataPlane *s;
>> +};
>> +
>>   /* Raise an interrupt to signal guest, if necessary */
>>   static void notify_guest(VirtIOBlockDataPlane *s)
>>   {
>> @@ -138,6 +147,54 @@ static void handle_notify(EventNotifier *e)
>>       blk_io_unplug(s->conf->conf.blk);
>>   }
>>   
>> +static void data_plane_set_up_op_blockers(VirtIOBlockDataPlane *s)
>> +{
>> +    assert(!s->blocker);
>> +    error_setg(&s->blocker, "block device is in use by data plane");
>> +    blk_op_block_all(s->conf->conf.blk, s->blocker);
>> +    blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_RESIZE, s->blocker);
>> +    blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_DRIVE_DEL, s->blocker);
>> +    blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_BACKUP_SOURCE, s->blocker);
>> +    blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_CHANGE, s->blocker);
>> +    blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_COMMIT_SOURCE, s->blocker);
>> +    blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_COMMIT_TARGET, s->blocker);
>> +    blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_EJECT, s->blocker);
>> +    blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,
>> +                   s->blocker);
>> +    blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT,
>> +                   s->blocker);
>> +    blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE,
>> +                   s->blocker);
>> +    blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_MIRROR, s->blocker);
>> +    blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_STREAM, s->blocker);
>> +    blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_REPLACE, s->blocker);
>> +}
>> +
>> +static void data_plane_remove_op_blockers(VirtIOBlockDataPlane *s)
>> +{
>> +    if (s->blocker) {
>> +        blk_op_unblock_all(s->conf->conf.blk, s->blocker);
>> +        error_free(s->blocker);
>> +        s->blocker = NULL;
>> +    }
>> +}
>> +
>> +static void data_plane_blk_insert_notifier(Notifier *n, void *data)
>> +{
>> +    DataPlaneBlkChangeNotifier *cn = DO_UPCAST(DataPlaneBlkChangeNotifier,
>> +                                               n, n);
>> +    assert(cn->s->conf->conf.blk == data);
>> +    data_plane_set_up_op_blockers(cn->s);
>> +}
>> +
>> +static void data_plane_blk_remove_notifier(Notifier *n, void *data)
>> +{
>> +    DataPlaneBlkChangeNotifier *cn = DO_UPCAST(DataPlaneBlkChangeNotifier,
>> +                                               n, n);
>> +    assert(cn->s->conf->conf.blk == data);
>> +    data_plane_remove_op_blockers(cn->s);
>> +}
>> +
>>   /* Context: QEMU global mutex held */
>>   void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
>>                                     VirtIOBlockDataPlane **dataplane,
>> @@ -194,22 +251,17 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
>>       s->ctx = iothread_get_aio_context(s->iothread);
>>       s->bh = aio_bh_new(s->ctx, notify_guest_bh, s);
>>   
>> -    error_setg(&s->blocker, "block device is in use by data plane");
>> -    blk_op_block_all(conf->conf.blk, s->blocker);
>> -    blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_RESIZE, s->blocker);
>> -    blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_DRIVE_DEL, s->blocker);
>> -    blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_BACKUP_SOURCE, s->blocker);
>> -    blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_CHANGE, s->blocker);
>> -    blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_COMMIT_SOURCE, s->blocker);
>> -    blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_COMMIT_TARGET, s->blocker);
>> -    blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_EJECT, s->blocker);
>> -    blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, s->blocker);
>> -    blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, s->blocker);
>> -    blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE,
>> -                   s->blocker);
>> -    blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_MIRROR, s->blocker);
>> -    blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_STREAM, s->blocker);
>> -    blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_REPLACE, s->blocker);
>> +    s->insert_notifier = g_new0(DataPlaneBlkChangeNotifier, 1);
>> +    s->insert_notifier->n.notify = data_plane_blk_insert_notifier;
>> +    s->insert_notifier->s = s;
>> +    blk_add_insert_bs_notifier(conf->conf.blk, &s->insert_notifier->n);
>> +
>> +    s->remove_notifier = g_new0(DataPlaneBlkChangeNotifier, 1);
>> +    s->remove_notifier->n.notify = data_plane_blk_remove_notifier;
>> +    s->remove_notifier->s = s;
>> +    blk_add_remove_bs_notifier(conf->conf.blk, &s->remove_notifier->n);
>> +
>> +    data_plane_set_up_op_blockers(s);
>>   
>>       *dataplane = s;
>>   }
>> @@ -222,10 +274,15 @@ void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s)
>>       }
>>   
>>       virtio_blk_data_plane_stop(s);
>> -    blk_op_unblock_all(s->conf->conf.blk, s->blocker);
>> -    error_free(s->blocker);
>> +    data_plane_remove_op_blockers(s);
>>       object_unref(OBJECT(s->iothread));
>>       qemu_bh_delete(s->bh);
>> +
>> +    notifier_remove(&s->insert_notifier->n);
>> +    notifier_remove(&s->remove_notifier->n);
>> +    g_free(s->insert_notifier);
>> +    g_free(s->remove_notifier);
>> +
>>       g_free(s);
>>   }
>>   
>> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
>> index 5469bad..0033862 100644
>> --- a/hw/scsi/virtio-scsi.c
>> +++ b/hw/scsi/virtio-scsi.c
>> @@ -755,6 +755,38 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense)
>>       }
>>   }
>>   
>> +static void virtio_scsi_set_up_op_blockers(VirtIOSCSI *s, SCSIDevice *sd)
>> +{
>> +    assert(!s->blocker);
>> +    error_setg(&s->blocker, "block device is in use by data plane");
>> +    blk_op_block_all(sd->conf.blk, s->blocker);
>> +}
>> +
>> +static void virtio_scsi_remove_op_blockers(VirtIOSCSI *s, SCSIDevice *sd)
>> +{
>> +    if (s->blocker) {
>> +        blk_op_unblock_all(sd->conf.blk, s->blocker);
>> +        error_free(s->blocker);
>> +        s->blocker = NULL;
>> +    }
>> +}
>> +
>> +static void virtio_scsi_blk_insert_notifier(Notifier *n, void *data)
>> +{
>> +    VirtIOSCSIBlkChangeNotifier *cn = DO_UPCAST(VirtIOSCSIBlkChangeNotifier,
>> +                                                n, n);
>> +    assert(cn->sd->conf.blk == data);
>> +    virtio_scsi_set_up_op_blockers(cn->s, cn->sd);
>> +}
>> +
>> +static void virtio_scsi_blk_remove_notifier(Notifier *n, void *data)
>> +{
>> +    VirtIOSCSIBlkChangeNotifier *cn = DO_UPCAST(VirtIOSCSIBlkChangeNotifier,
>> +                                                n, n);
>> +    assert(cn->sd->conf.blk == data);
>> +    virtio_scsi_remove_op_blockers(cn->s, cn->sd);
>> +}
>> +
>>   static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
>>                                   Error **errp)
>>   {
>> @@ -763,12 +795,22 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
>>       SCSIDevice *sd = SCSI_DEVICE(dev);
>>   
>>       if (s->ctx && !s->dataplane_disabled) {
>> +        s->insert_notifier = g_new0(VirtIOSCSIBlkChangeNotifier, 1);
> We leak the first s->insert_notifier when a second device is plugged. Probably
> you can just embed these in SCSIDevice.

Won't virtio_scsi_hotunplug() (which frees s->{insert,remove}_notifier) 
be called before a second device is plugged?

>> +        s->insert_notifier->n.notify = virtio_scsi_blk_insert_notifier;
>> +        s->insert_notifier->s = s;
>> +        s->insert_notifier->sd = sd;
>> +        blk_add_insert_bs_notifier(sd->conf.blk, &s->insert_notifier->n);
>> +
>> +        s->remove_notifier = g_new0(VirtIOSCSIBlkChangeNotifier, 1);
>> +        s->remove_notifier->n.notify = virtio_scsi_blk_remove_notifier;
>> +        s->remove_notifier->s = s;
>> +        s->remove_notifier->sd = sd;
>> +        blk_add_remove_bs_notifier(sd->conf.blk, &s->remove_notifier->n);
>> +
>>           if (blk_op_is_blocked(sd->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) {
>>               return;
>>           }
>> -        assert(!s->blocker);
>> -        error_setg(&s->blocker, "block device is in use by data plane");
>> -        blk_op_block_all(sd->conf.blk, s->blocker);
>> +        virtio_scsi_set_up_op_blockers(s, sd);
>>       }
>>   
>>       if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) {
>> @@ -791,10 +833,18 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev,
>>                                  VIRTIO_SCSI_EVT_RESET_REMOVED);
>>       }
>>   
>> -    if (s->ctx && s->blocker) {
>> -        blk_op_unblock_all(sd->conf.blk, s->blocker);
>> -        error_free(s->blocker);
>> -        s->blocker = NULL;
>> +    if (s->ctx) {
>> +        virtio_scsi_remove_op_blockers(s, sd);
>> +    }
>> +    if (s->insert_notifier) {
>> +        notifier_remove(&s->insert_notifier->n);
>> +        g_free(s->insert_notifier);
>> +        s->insert_notifier = NULL;
>> +    }
>> +    if (s->remove_notifier) {
>> +        notifier_remove(&s->remove_notifier->n);
>> +        g_free(s->remove_notifier);
>> +        s->remove_notifier = NULL;
>>       }
>>       qdev_simple_device_unplug_cb(hotplug_dev, dev, errp);
>>   }
>> diff --git a/include/block/block.h b/include/block/block.h
>> index 1422f01..dc94084 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -207,7 +207,6 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
>>   void bdrv_reopen_commit(BDRVReopenState *reopen_state);
>>   void bdrv_reopen_abort(BDRVReopenState *reopen_state);
>>   void bdrv_close(BlockDriverState *bs);
>> -void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify);
>>   int bdrv_read(BlockDriverState *bs, int64_t sector_num,
>>                 uint8_t *buf, int nb_sectors);
>>   int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num,
>> diff --git a/include/block/block_int.h b/include/block/block_int.h
>> index 8fcfc29..b2c1d87 100644
>> --- a/include/block/block_int.h
>> +++ b/include/block/block_int.h
>> @@ -367,8 +367,6 @@ struct BlockDriverState {
>>       BlockDriverState *backing_hd;
>>       BlockDriverState *file;
>>   
>> -    NotifierList close_notifiers;
>> -
>>       /* Callback before write request is processed */
>>       NotifierWithReturnList before_write_notifiers;
>>   
>> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
>> index bf17cc9..723cc42 100644
>> --- a/include/hw/virtio/virtio-scsi.h
>> +++ b/include/hw/virtio/virtio-scsi.h
>> @@ -176,6 +176,12 @@ typedef struct VirtIOSCSICommon {
>>       VirtQueue **cmd_vqs;
>>   } VirtIOSCSICommon;
>>   
>> +typedef struct VirtIOSCSIBlkChangeNotifier {
>> +    Notifier n;
>> +    struct VirtIOSCSI *s;
>> +    SCSIDevice *sd;
>> +} VirtIOSCSIBlkChangeNotifier;
>> +
>>   typedef struct VirtIOSCSI {
>>       VirtIOSCSICommon parent_obj;
>>   
>> @@ -186,6 +192,8 @@ typedef struct VirtIOSCSI {
>>       /* Fields for dataplane below */
>>       AioContext *ctx; /* one iothread per virtio-scsi-pci for now */
>>   
>> +    VirtIOSCSIBlkChangeNotifier *insert_notifier, *remove_notifier;
>> +
>>       /* Vring is used instead of vq in dataplane code, because of the underlying
>>        * memory layer thread safety */
>>       VirtIOSCSIVring *ctrl_vring;
>> diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
>> index 3aad010..e0a2749 100644
>> --- a/include/sysemu/block-backend.h
>> +++ b/include/sysemu/block-backend.h
>> @@ -158,7 +158,8 @@ void blk_remove_aio_context_notifier(BlockBackend *blk,
>>                                                                     void *),
>>                                        void (*detach_aio_context)(void *),
>>                                        void *opaque);
>> -void blk_add_close_notifier(BlockBackend *blk, Notifier *notify);
>> +void blk_add_remove_bs_notifier(BlockBackend *blk, Notifier *notify);
>> +void blk_add_insert_bs_notifier(BlockBackend *blk, Notifier *notify);
>>   void blk_io_plug(BlockBackend *blk);
>>   void blk_io_unplug(BlockBackend *blk);
>>   BlockAcctStats *blk_get_stats(BlockBackend *blk);
>> diff --git a/nbd.c b/nbd.c
>> index 71159af..5d50f22 100644
>> --- a/nbd.c
>> +++ b/nbd.c
>> @@ -964,9 +964,25 @@ static void blk_aio_detach(void *opaque)
>>       exp->ctx = NULL;
>>   }
>>   
>> +typedef struct NBDEjectNotifier {
>> +    Notifier n;
>> +    NBDExport *exp;
>> +    QTAILQ_ENTRY(NBDEjectNotifier) next;
>> +} NBDEjectNotifier;
> The same question here: will embedding the Notifier into NBDExport simplify
> memory management?

Maybe, but I don't think it works.

Max

> Fam
>
>> +
>> +static QTAILQ_HEAD(, NBDEjectNotifier) eject_notifiers =
>> +    QTAILQ_HEAD_INITIALIZER(eject_notifiers);
>> +
>> +static void nbd_eject_notifier(Notifier *n, void *data)
>> +{
>> +    NBDEjectNotifier *cn = DO_UPCAST(NBDEjectNotifier, n, n);
>> +    nbd_export_close(cn->exp);
>> +}
>> +
>>   NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size,
>>                             uint32_t nbdflags, void (*close)(NBDExport *))
>>   {
>> +    NBDEjectNotifier *n = g_new0(NBDEjectNotifier, 1);
>>       NBDExport *exp = g_malloc0(sizeof(NBDExport));
>>       exp->refcount = 1;
>>       QTAILQ_INIT(&exp->clients);
>> @@ -978,6 +994,13 @@ NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size,
>>       exp->ctx = blk_get_aio_context(blk);
>>       blk_ref(blk);
>>       blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp);
>> +
>> +    n->n.notify = nbd_eject_notifier;
>> +    n->exp = exp;
>> +    QTAILQ_INSERT_TAIL(&eject_notifiers, n, next);
>> +
>> +    blk_add_remove_bs_notifier(blk, &n->n);
>> +
>>       /*
>>        * NBD exports are used for non-shared storage migration.  Make sure
>>        * that BDRV_O_INCOMING is cleared and the image is ready for write
>> @@ -1022,6 +1045,7 @@ void nbd_export_set_name(NBDExport *exp, const char *name)
>>   
>>   void nbd_export_close(NBDExport *exp)
>>   {
>> +    NBDEjectNotifier *n;
>>       NBDClient *client, *next;
>>   
>>       nbd_export_get(exp);
>> @@ -1031,6 +1055,17 @@ void nbd_export_close(NBDExport *exp)
>>       nbd_export_set_name(exp, NULL);
>>       nbd_export_put(exp);
>>       if (exp->blk) {
>> +        QTAILQ_FOREACH(n, &eject_notifiers, next) {
>> +            if (n->exp == exp) {
>> +                break;
>> +            }
>> +        }
>> +        assert(n);
>> +
>> +        notifier_remove(&n->n);
>> +        QTAILQ_REMOVE(&eject_notifiers, n, next);
>> +        g_free(n);
>> +
>>           blk_remove_aio_context_notifier(exp->blk, blk_aio_attached,
>>                                           blk_aio_detach, exp);
>>           blk_unref(exp->blk);
>> -- 
>> 2.1.0
>>
>>