qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Qemu-block] [PATCH 16/19] block: Allow graph changes i


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH 16/19] block: Allow graph changes in subtree drained section
Date: Wed, 20 Dec 2017 12:31:22 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0

On 20/12/2017 12:18, Kevin Wolf wrote:
> Am 20.12.2017 um 11:51 hat Paolo Bonzini geschrieben:
>> On 20/12/2017 11:34, Kevin Wolf wrote:
>>>      .inherit_options = bdrv_inherited_options,
>>>      .drained_begin   = bdrv_child_cb_drained_begin,
>>>      .drained_end     = bdrv_child_cb_drained_end,
>>> +    .attach          = bdrv_child_cb_attach,
>>> +    .detach          = bdrv_child_cb_detach,
>>>      .inactivate      = bdrv_child_cb_inactivate,
>>>  };
>>>  
>>> @@ -911,6 +933,8 @@ const BdrvChildRole child_format = {
>>>      .inherit_options = bdrv_inherited_fmt_options,
>>>      .drained_begin   = bdrv_child_cb_drained_begin,
>>>      .drained_end     = bdrv_child_cb_drained_end,
>>> +    .attach          = bdrv_child_cb_attach,
>>> +    .detach          = bdrv_child_cb_detach,
>>>      .inactivate      = bdrv_child_cb_inactivate,
>>
>> Is there any case of a BdrvChildRole that doesn't want these callbacks?
>> Maybe the functions should be called after ->attach and before ->detach
>> (e.g. bdrv_{,un}apply_subtree_drain), rather than modifying the
>> BdrvChildRole implementations.
> 
> At first I intended to implement it directly in
> bdrv_replace_child_noperm(), but the thing is that you need the
> bs->recursive_quiesce_counter of the parent BDS - but not all parents of
> a BdrvChild are even a BDS. It could also be a BB root child or a block
> job child. This is why we only have a void *opaque rather than a BDS
> pointer for the parent.
> 
> The other option would be an additional BdrvChildRole callback like
> .get_recursive_quiesce_counter, but compared to that, I like some code
> in .attach/.detach better.

I see.  What about keeping the callbacks, but exporting

void bdrv_apply_subtree_drain(BlockDriverState *child,
                              BlockDriverState *new_parent);
void bdrv_unapply_subtree_drain(BlockDriverState *child,
                                BlockDriverState *old_parent);

instead of bdrv_do_drained_{begin,end}?

Thanks,

Paolo

>> Then they can be put in block/io.c, and bdrv_do_drained_* can remain
>> static.  (I would also consider extracting block/drain.c, but it is
>> painful to do it now that you have this nice series---so let's do it after).
> 
> I can keep that in mind for part 3 (or 4, whatever it may become).
> 
> Kevin
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]