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

[Fam Zheng]

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 <address@hidden>
> ---
>  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?

>      /* 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.

> +        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?

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
> 
>