From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51571) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YQchT-0002Kp-Ve for qemu-devel@nongnu.org; Wed, 25 Feb 2015 09:12:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YQchO-0000gx-SV for qemu-devel@nongnu.org; Wed, 25 Feb 2015 09:12:31 -0500 Message-ID: <54EDD846.5030509@redhat.com> Date: Wed, 25 Feb 2015 09:12:22 -0500 From: Max Reitz MIME-Version: 1.0 References: <1424792164-2130-1-git-send-email-mreitz@redhat.com> <1424792164-2130-6-git-send-email-mreitz@redhat.com> <20150225075232.GH5293@ad.nay.redhat.com> In-Reply-To: <20150225075232.GH5293@ad.nay.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 05/11] block: Move BDS close notifiers into BB List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Kevin Wolf , qemu-block@nongnu.org, qemu-devel@nongnu.org, Markus Armbruster , Stefan Hajnoczi , Paolo Bonzini 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 >> --- >> 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 >> >>