Am 09.11.2015 um 17:50 hat Max Reitz geschrieben:
> On 09.11.2015 17:04, Kevin Wolf wrote:
> > 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
>
> I'll do my best.
>
> >> 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.
>
> You mean blk_delete()? I can do that, yes.Yes, sorry, that's what I meant. Kevin
pgpQfChJXdxsh.pgp
Description: PGP signature
