[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 07/15] block: Move BDS close notifiers into B
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v6 07/15] block: Move BDS close notifiers into BB |
Date: |
Mon, 9 Nov 2015 17:50:54 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
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 <address@hidden>
>
> 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.
Max
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v6 08/15] block: Use blk_remove_bs() in blk_delete(), Max Reitz, 2015/11/04
[Qemu-devel] [PATCH v6 09/15] blockdev: Use blk_remove_bs() in do_drive_del(), Max Reitz, 2015/11/04
[Qemu-devel] [PATCH v6 10/15] block: Make bdrv_close() static, Max Reitz, 2015/11/04
[Qemu-devel] [PATCH v6 11/15] block: Add list of all BlockDriverStates, Max Reitz, 2015/11/04
[Qemu-devel] [PATCH v6 12/15] blockdev: Keep track of monitor-owned BDS, Max Reitz, 2015/11/04