Re: [PATCH v5 09/31] block: introduce assert_bdrv_graph_writable

[Hanna Reitz <hreitz@redhat.com>]

On 14.12.21 20:48, Emanuele Giuseppe Esposito wrote:
>
>
> On 10/12/2021 18:43, Hanna Reitz wrote:
>> On 24.11.21 07:43, Emanuele Giuseppe Esposito wrote:
>>> We want to be sure that the functions that write the child and
>>> parent list of a bs are under BQL and drain.
>>>
>>> BQL prevents from concurrent writings from the GS API, while
>>> drains protect from I/O.
>>>
>>> TODO: drains are missing in some functions using this assert.
>>> Therefore a proper assertion will fail. Because adding drains
>>> requires additional discussions, they will be added in future
>>> series.
>>>
>>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>>> ---
>>>   include/block/block_int-global-state.h | 10 +++++++++-
>>>   block.c                                |  4 ++++
>>>   block/io.c                             | 11 +++++++++++
>>>   3 files changed, 24 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/block/block_int-global-state.h 
>>> b/include/block/block_int-global-state.h
>>> index a1b7d0579d..fa96e8b449 100644
>>> --- a/include/block/block_int-global-state.h
>>> +++ b/include/block/block_int-global-state.h
>>> @@ -312,4 +312,12 @@ void 
>>> bdrv_remove_aio_context_notifier(BlockDriverState *bs,
>>>    */
>>>   void bdrv_drain_all_end_quiesce(BlockDriverState *bs);
>>> -#endif /* BLOCK_INT_GLOBAL_STATE*/
>>
>> This looks like it should be squashed into patch 7, sorry I missed 
>> this in v4...
>>
>> (Rest of this patch looks good to me, for the record – and while I’m 
>> at it, for patches I didn’t reply to so far, I planned to send an R-b 
>> later.  But then there’s things like patches 2/3 looking good to me, 
>> but it turned out in my review for patch 4 that bdrv_lock_medium() is 
>> used in an I/O path, so I can’t really send an R-b now anymore...)
>>
> Sorry I don't understand this, what should be squashed into patch 7? 
> The assertion? If so, why?

No, the

-#endif /* BLOCK_INT_GLOBAL_STATE*/
+#endif /* BLOCK_INT_GLOBAL_STATE */

part of the hunk.

Hanna