diff mbox series

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

Message ID 20221116122241.2856527-10-eesposit@redhat.com (mailing list archive)
State New, archived
Headers show
Series Still more coroutine and various fixes in block layer | expand

Commit Message

Emanuele Giuseppe Esposito Nov. 16, 2022, 12:22 p.m. UTC 
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(-)

Comments

Kevin Wolf Nov. 22, 2022, 8:58 a.m. UTC | #1 
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.

>      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()?

>  {
>      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
Emanuele Giuseppe Esposito Nov. 22, 2022, 9:04 a.m. UTC | #2 
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
>
diff mbox series

Patch

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());
     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)
 {
     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);
diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h
index 00e0cf8aea..6f35ed99e3 100644
--- a/include/block/block-global-state.h
+++ b/include/block/block-global-state.h
@@ -57,7 +57,8 @@  BlockDriver *bdrv_find_protocol(const char *filename,
 BlockDriver *bdrv_find_format(const char *format_name);
 int bdrv_create(BlockDriver *drv, const char* filename,
                 QemuOpts *opts, Error **errp);
-int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);
+int coroutine_fn bdrv_create_file(const char *filename, QemuOpts *opts,
+                                  Error **errp);
 
 BlockDriverState *bdrv_new(void);
 int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,