[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v2 11/25] include/block/blockjob.h: global state API
From: |
Stefan Hajnoczi |
Subject: |
Re: [RFC PATCH v2 11/25] include/block/blockjob.h: global state API |
Date: |
Mon, 11 Oct 2021 12:10:11 +0100 |
On Fri, Oct 08, 2021 at 09:20:35AM +0200, Emanuele Giuseppe Esposito wrote:
>
> > > +/*
> > > + * Global state (GS) API. These functions run under the BQL lock.
> > > + *
> > > + * If a function modifies the graph, it also uses drain and/or
> > > + * aio_context_acquire/release to be sure it has unique access.
> > > + * aio_context locking is needed together with BQL because of
> > > + * the thread-safe I/O API that concurrently runs and accesses
> > > + * the graph without the BQL.
> > > + *
> > > + * It is important to note that not all of these functions are
> > > + * necessarily limited to running under the BQL, but they would
> > > + * require additional auditing and may small thread-safety changes
> > > + * to move them into the I/O API. Often it's not worth doing that
> > > + * work since the APIs are only used with the BQL held at the
> > > + * moment, so they have been placed in the GS API (for now).
> > > + *
> > > + * All functions below must use this assertion:
> > > + * g_assert(qemu_in_main_thread());
> > > + * to catch when they are accidentally called without the BQL.
> > > + */
> >
> > This is comment is duplicated in many places. I suggest explaining it in
> > one place and using references in the other files:
> >
> > /*
> > * Global state (GS) API. These functions run under the BQL lock.
> > *
> > * See include/block/block.h for more information about the GS API.
> > */
> >
>
> Good idea. Should I also do that for I/O, or it's not worth for very few
> lines?
Up to you, but I think it makes sense to have a minimal comment for both
the GS and I/O API.
Stefan
signature.asc
Description: PGP signature
- Re: [RFC PATCH v2 08/25] block: introduce assert_bdrv_graph_writable, (continued)
[RFC PATCH v2 10/25] assertions for blockjob_int.h, Emanuele Giuseppe Esposito, 2021/10/05
[RFC PATCH v2 12/25] assertions for blockob.h global state API, Emanuele Giuseppe Esposito, 2021/10/05
[RFC PATCH v2 11/25] include/block/blockjob.h: global state API, Emanuele Giuseppe Esposito, 2021/10/05
[RFC PATCH v2 14/25] assertions for blockdev.h global state API, Emanuele Giuseppe Esposito, 2021/10/05
[RFC PATCH v2 15/25] include/block/snapshot: global state API + assertions, Emanuele Giuseppe Esposito, 2021/10/05
[RFC PATCH v2 16/25] block/backup-top.h: global state API + assertions, Emanuele Giuseppe Esposito, 2021/10/05
[RFC PATCH v2 17/25] include/block/transactions: global state API + assertions, Emanuele Giuseppe Esposito, 2021/10/05