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

[Emanuele Giuseppe Esposito]

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?

Thank you,
Emanuele