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

[Hanna Reitz]

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