Re: [RFC PATCH v2 11/25] include/block/blockjob.h: global state API

[Stefan Hajnoczi]

On Tue, Oct 05, 2021 at 10:32:01AM -0400, Emanuele Giuseppe Esposito wrote:
> blockjob functions run always under the BQL lock.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>  include/block/blockjob.h | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
> index d200f33c10..3bf384f8bf 100644
> --- a/include/block/blockjob.h
> +++ b/include/block/blockjob.h
> @@ -77,6 +77,27 @@ typedef struct BlockJob {
>      GSList *nodes;
>  } BlockJob;
>  
> +/*
> + * 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.
   */

signature.asc
Description: PGP signature