Re: [PATCH v6 25/33] block_int-common.h: split function pointers in BdrvChildClass

[Hanna Reitz]

On 21.01.22 18:05, Emanuele Giuseppe Esposito wrote:
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   include/block/block_int-common.h | 67 +++++++++++++++++++-------------
>   1 file changed, 40 insertions(+), 27 deletions(-)
>
> diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
> index e007dbf768..cc8c8835ba 100644
> --- a/include/block/block_int-common.h
> +++ b/include/block/block_int-common.h
> @@ -815,12 +815,16 @@ struct BdrvChildClass {
>        */
>       bool parent_is_bds;
>   
> +    /*
> +     * Global state (GS) API. These functions run under the BQL lock.
> +     *
> +     * See include/block/block-global-state.h for more information about
> +     * the GS API.
> +     */
>       void (*inherit_options)(BdrvChildRole role, bool parent_is_format,
>                               int *child_flags, QDict *child_options,
>                               int parent_flags, QDict *parent_options);
> -
>       void (*change_media)(BdrvChild *child, bool load);
> -    void (*resize)(BdrvChild *child);
>   
>       /*
>        * Returns a name that is supposedly more useful for human users than the

The method this comment belongs to is `.get_name()`.  It’s exposed 
through `bdrv_get_parent_name()`, which is called by 
`bdrv_get_device_name()` and `bdrv_get_device_or_node_name()` – so I 
think it should be classified as I/O.

> @@ -837,6 +841,40 @@ struct BdrvChildClass {
>        */
>       char *(*get_parent_desc)(BdrvChild *child);

This function is very similar, so we might also want to reconsider 
classifying it as I/O.  There’s no need, because all of its callers do 
run in the main thread, but at the same time I don’t believe there’s 
anything stopping us (and it starts to sound to me like all functions of 
the “get name” kind perhaps should ideally be I/O, in that they 
shouldn’t require the GS context).

Up to you. O:)

(Rest of this patch looks good!)

Hanna