Re: [Qemu-devel] [PATCH 2/5] block: Add AioContextNotifier functions to BB

[Max Reitz <mreitz@redhat.com>]

On 2014-11-17 at 18:26, Paolo Bonzini wrote:
> On 17/11/2014 16:30, Max Reitz wrote:
>> Because all BlockDriverStates behind a single BlockBackend reside in a
>> single AioContext, it is fine to just pass these functions
>> (blk_add_aio_context_notifier() and blk_remove_aio_context_notifier())
>> through to the root BlockDriverState.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
> The logical question then is: are there any function in BlockDriverState
> that do not make sense as a BlockBackend API?

Well, surely bdrv_swap(), or bdrv_drop_intermediate(). These are 
functions which work on multiple BDSs (in the same tree, that is, behind 
the same BB) so they don't make sense on the BB.

Others could simply be passed through to the root BDS but it somehow 
doesn't make sense to execute them on the BB. For instance, 
bdrv_set_key(); this is something for an individual BDS, in contrast to 
other operations like reading and writing which will probably be passed 
through the BDS tree; or bdrv_get_info().

The functions added in this patch do make sense on a BB level: Since all 
BDSs behind one BB are always in the same AioContext, it makes sense to 
consider that the BB's AioContext.

The next patch is more difficult to justify. Closing a BDS is somehow 
passed through, but at first glance it doesn't make a whole lot of sense 
on the BB level. However, when you consider (as far as I looked into it) 
that a BDS is only closed when there are either no references to it 
(which will not happen as long as it has a BB) or when it is ejected, it 
suddenly does make sense: "Ejecting" really is something for the BB, so 
it makes sense to wait for that event (even though the name "close 
notifier" doesn't sound much like it...). Maybe I should sometimes take 
a deeper look into when a BDS belonging to a BB may be closed and if 
it's really only due to ejection, rename the "close notifiers" to 
something like "eject notifiers" (on the BB level).

Max