Re: [PATCH v4 09/11] block: bdrv_create_file is a coroutine_fn

[Emanuele Giuseppe Esposito <eesposit@redhat.com>]

Am 22/11/2022 um 09:58 schrieb Kevin Wolf:
> Am 16.11.2022 um 13:22 hat Emanuele Giuseppe Esposito geschrieben:
>> It is always called in coroutine_fn callbacks, therefore
>> it can directly call bdrv_co_create().
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>> ---
>>  block.c                            | 6 ++++--
>>  include/block/block-global-state.h | 3 ++-
>>  2 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index c610a32e77..7a4c3eb540 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -534,6 +534,7 @@ static int coroutine_fn bdrv_co_create(BlockDriver *drv, const char *filename,
>>      int ret;
>>      char *filename_copy;
>>      GLOBAL_STATE_CODE();
>> +    assert(qemu_in_coroutine());
> 
> We don't generally assert this, otherwise it would have to be in every
> coroutine_fn.

That was my plan for the serie "Protect the block layer with a rwlock:
part 3", where I convert BlockDriver callbacks in coroutine, and thus
assert there because I know all the callers are coroutine_fn.

> 
>>      assert(*errp == NULL);
>>      assert(drv);
>>  
>> @@ -725,7 +726,8 @@ out:
>>      return ret;
>>  }
>>  
>> -int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
>> +int coroutine_fn bdrv_create_file(const char *filename, QemuOpts *opts,
>> +                                  Error **errp)
> 
> Should it be renamed as bdrv_co_create_file()?
> 
Ok (when I don't answer just assume that I agree).

Thank you,
Emanuele
>>  {
>>      QemuOpts *protocol_opts;
>>      BlockDriver *drv;
>> @@ -766,7 +768,7 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
>>          goto out;
>>      }
>>  
>> -    ret = bdrv_create(drv, filename, protocol_opts, errp);
>> +    ret = bdrv_co_create(drv, filename, protocol_opts, errp);
>>  out:
>>      qemu_opts_del(protocol_opts);
>>      qobject_unref(qdict);
> 
> Kevin
>