Am 04.11.2015 um 19:57 hat Max Reitz geschrieben:
> 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 <mre...@redhat.com>
I think this needs to be split into smaller patches:
1. Add the new BlockBackend notifiers
2. Use them in virtio-blk in order to fix... removable virtio-blk
devices, or what is it?
3. Convert NBD
4. Remove old close notifiers
> block.c | 7 ----
> block/block-backend.c | 19 +++++++---
> blockdev-nbd.c | 37 +-------------------
> hw/block/dataplane/virtio-blk.c | 77
> +++++++++++++++++++++++++++++++----------
> hw/scsi/virtio-scsi.c | 59 +++++++++++++++++++++++++++++++
> include/block/block.h | 1 -
> include/block/block_int.h | 2 --
> include/hw/virtio/virtio-scsi.h | 10 ++++++
> include/sysemu/block-backend.h | 3 +-
> nbd.c | 13 +++++++
> 10 files changed, 159 insertions(+), 69 deletions(-)
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 6f9309f..38580f7 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -48,6 +48,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 {
> @@ -98,6 +100,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;
> }
> @@ -343,6 +347,8 @@ void blk_hide_on_behalf_of_hmp_drive_del(BlockBackend
> *blk)
> */
> void blk_remove_bs(BlockBackend *blk)
> {
> + notifier_list_notify(&blk->remove_bs_notifiers, blk);
> +
> blk_update_root_state(blk);
>
> blk->bs->blk = NULL;
> @@ -359,6 +365,8 @@ void blk_insert_bs(BlockBackend *blk, BlockDriverState
> *bs)
> bdrv_ref(bs);
> blk->bs = bs;
> bs->blk = blk;
> +
> + notifier_list_notify(&blk->insert_bs_notifiers, blk);
> }
Do we want to notify on BB deletion, too? It's also some kind of removal
of a connection between BB and BDS. In other words, should blk_delete()
call blk_remove_bs() rather than bdrv_unref()?
[ Edit: I see that's what the next patch does. Good. ]
Should blk_unref() also assert that the notifier list is empty?
Otherwise we would be leaking notifiers.
Kevin
