Re: [PATCH v5 13/31] block.c: add assertions to static functions

[Hanna Reitz]

On 24.11.21 07:44, Emanuele Giuseppe Esposito wrote:
> Following the assertion derived from the API split,
> propagate the assertion also in the static functions.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   block.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 44 insertions(+), 1 deletion(-)

Looks good to me, just one small question:

> diff --git a/block.c b/block.c
> index 5516c84ec4..b77ab0a104 100644
> --- a/block.c
> +++ b/block.c

[...]

> @@ -1241,6 +1252,7 @@ static void bdrv_temp_snapshot_options(int *child_flags, 
> QDict *child_options,
>                                          int parent_flags, QDict 
> *parent_options)
>   {
>       *child_flags = (parent_flags & ~BDRV_O_SNAPSHOT) | BDRV_O_TEMPORARY;
> +    assert(qemu_in_main_thread());

Stylistically, it’s a bit strange that in other places, this assertion 
comes right after all local variable declarations, or after some 
assertions that are already present in that place; but here, it follows 
a normal statement.  Is that on purpose?

>   
>       /* For temporary files, unconditional cache=unsafe is fine */
>       qdict_set_default_str(child_options, BDRV_OPT_CACHE_DIRECT, "off");