diff mbox series

[v4,04/11] block-coroutine-wrapper.py: introduce generated_co_wrapper_simple

Message ID 20221116122241.2856527-5-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
This new annotation creates just a function wrapper that creates
a new coroutine. It assumes the caller is not a coroutine.

This is much better as g_c_w, because it is clear if the caller
is a coroutine or not, and provides the advantage of automating
the code creation. In the future all g_c_w functions will be
substituted on g_c_w_simple.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 include/block/block-common.h       |  1 +
 scripts/block-coroutine-wrapper.py | 87 ++++++++++++++++++++++--------
 2 files changed, 66 insertions(+), 22 deletions(-)

Comments

Kevin Wolf Nov. 21, 2022, 12:21 p.m. UTC | #1
Am 16.11.2022 um 13:22 hat Emanuele Giuseppe Esposito geschrieben:
> This new annotation creates just a function wrapper that creates
> a new coroutine. It assumes the caller is not a coroutine.
> 
> This is much better as g_c_w, because it is clear if the caller
> is a coroutine or not, and provides the advantage of automating
> the code creation. In the future all g_c_w functions will be
> substituted on g_c_w_simple.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

$ mypy --strict scripts/block-coroutine-wrapper.py
scripts/block-coroutine-wrapper.py:31: error: Function is missing a return type annotation
scripts/block-coroutine-wrapper.py:90: error: Missing type parameters for generic type "Iterator"
scripts/block-coroutine-wrapper.py:110: error: "FuncDecl" has no attribute "co_name"
scripts/block-coroutine-wrapper.py:111: error: "FuncDecl" has no attribute "struct_name"
scripts/block-coroutine-wrapper.py:112: error: "FuncDecl" has no attribute "bs"
scripts/block-coroutine-wrapper.py:135: error: "FuncDecl" has no attribute "co_name"
scripts/block-coroutine-wrapper.py:136: error: "FuncDecl" has no attribute "struct_name"
scripts/block-coroutine-wrapper.py:137: error: "FuncDecl" has no attribute "bs"
scripts/block-coroutine-wrapper.py:160: error: "FuncDecl" has no attribute "co_name"
scripts/block-coroutine-wrapper.py:161: error: "FuncDecl" has no attribute "co_name"
scripts/block-coroutine-wrapper.py:172: error: "FuncDecl" has no attribute "bs"
scripts/block-coroutine-wrapper.py:173: error: "FuncDecl" has no attribute "struct_name"
scripts/block-coroutine-wrapper.py:174: error: "FuncDecl" has no attribute "struct_name"
scripts/block-coroutine-wrapper.py:218: error: Call to untyped function "gen_header" in typed context
Found 14 errors in 1 file (checked 1 source file)

The first two and the last one isn't the fault of this patch, but the
others are. When you add new attributes, they should be declared in
FuncDecl.__init__(). And actually, this even seems like a better place
to initialise them already with the right value than gen_wrapper().

>  include/block/block-common.h       |  1 +
>  scripts/block-coroutine-wrapper.py | 87 ++++++++++++++++++++++--------
>  2 files changed, 66 insertions(+), 22 deletions(-)
> 
> diff --git a/include/block/block-common.h b/include/block/block-common.h
> index 297704c1e9..8ae750c7cf 100644
> --- a/include/block/block-common.h
> +++ b/include/block/block-common.h
> @@ -43,6 +43,7 @@
>   * Read more in docs/devel/block-coroutine-wrapper.rst
>   */
>  #define generated_co_wrapper
> +#define generated_co_wrapper_simple
>  
>  /* block.c */
>  typedef struct BlockDriver BlockDriver;
> diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py
> index 08be813407..f88ef53964 100644
> --- a/scripts/block-coroutine-wrapper.py
> +++ b/scripts/block-coroutine-wrapper.py
> @@ -62,10 +62,15 @@ def __init__(self, param_decl: str) -> None:
>  
>  
>  class FuncDecl:
> -    def __init__(self, return_type: str, name: str, args: str) -> None:
> +    def __init__(self, return_type: str, name: str, args: str,
> +                 variant: str) -> None:
>          self.return_type = return_type.strip()
>          self.name = name.strip()
>          self.args = [ParamDecl(arg.strip()) for arg in args.split(',')]
> +        self.create_only_co = False
> +
> +        if variant == '_simple':
> +            self.create_only_co = True
>  
>      def gen_list(self, format: str) -> str:
>          return ', '.join(format.format_map(arg.__dict__) for arg in self.args)
> @@ -75,7 +80,8 @@ def gen_block(self, format: str) -> str:
>  
>  
>  # Match wrappers declared with a generated_co_wrapper mark
> -func_decl_re = re.compile(r'^int\s*generated_co_wrapper\s*'
> +func_decl_re = re.compile(r'^int\s*generated_co_wrapper'
> +                          r'(?P<variant>(_[a-z][a-z0-9_]*)?)\s*'
>                            r'(?P<wrapper_name>[a-z][a-z0-9_]*)'
>                            r'\((?P<args>[^)]*)\);$', re.MULTILINE)
>  
> @@ -84,7 +90,8 @@ def func_decl_iter(text: str) -> Iterator:
>      for m in func_decl_re.finditer(text):
>          yield FuncDecl(return_type='int',
>                         name=m.group('wrapper_name'),
> -                       args=m.group('args'))
> +                       args=m.group('args'),
> +                       variant=m.group('variant'))
>  
>  
>  def snake_to_camel(func_name: str) -> str:
> @@ -97,6 +104,51 @@ def snake_to_camel(func_name: str) -> str:
>      return ''.join(words)
>  
>  
> +# Checks if we are already in coroutine
> +def create_g_c_w(func: FuncDecl) -> str:
> +    name = func.co_name
> +    struct_name = func.struct_name
> +    return f"""\
> +int {func.name}({ func.gen_list('{decl}') })
> +{{
> +    if (qemu_in_coroutine()) {{
> +        return {name}({ func.gen_list('{name}') });
> +    }} else {{
> +        {struct_name} s = {{
> +            .poll_state.bs = {func.bs},
> +            .poll_state.in_progress = true,
> +
> +{ func.gen_block('            .{name} = {name},') }
> +        }};
> +
> +        s.poll_state.co = qemu_coroutine_create({name}_entry, &s);
> +
> +        return bdrv_poll_co(&s.poll_state);
> +    }}
> +}}"""
> +
> +
> +# Assumes we are not in coroutine, and creates one
> +def create_coroutine_only(func: FuncDecl) -> str:
> +    name = func.co_name
> +    struct_name = func.struct_name
> +    return f"""\
> +int {func.name}({ func.gen_list('{decl}') })
> +{{
> +    assert(!qemu_in_coroutine());
> +    {struct_name} s = {{
> +        .poll_state.bs = {func.bs},
> +        .poll_state.in_progress = true,
> +
> +{ func.gen_block('        .{name} = {name},') }
> +    }};

Not sure how strict we are about this in generated code, but generally
the QEMU coding style requires declarations to come first, so the
assertion should be below it.

> +
> +    s.poll_state.co = qemu_coroutine_create({name}_entry, &s);
> +
> +    return bdrv_poll_co(&s.poll_state);
> +}}"""

Kevin
diff mbox series

Patch

diff --git a/include/block/block-common.h b/include/block/block-common.h
index 297704c1e9..8ae750c7cf 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -43,6 +43,7 @@ 
  * Read more in docs/devel/block-coroutine-wrapper.rst
  */
 #define generated_co_wrapper
+#define generated_co_wrapper_simple
 
 /* block.c */
 typedef struct BlockDriver BlockDriver;
diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py
index 08be813407..f88ef53964 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -62,10 +62,15 @@  def __init__(self, param_decl: str) -> None:
 
 
 class FuncDecl:
-    def __init__(self, return_type: str, name: str, args: str) -> None:
+    def __init__(self, return_type: str, name: str, args: str,
+                 variant: str) -> None:
         self.return_type = return_type.strip()
         self.name = name.strip()
         self.args = [ParamDecl(arg.strip()) for arg in args.split(',')]
+        self.create_only_co = False
+
+        if variant == '_simple':
+            self.create_only_co = True
 
     def gen_list(self, format: str) -> str:
         return ', '.join(format.format_map(arg.__dict__) for arg in self.args)
@@ -75,7 +80,8 @@  def gen_block(self, format: str) -> str:
 
 
 # Match wrappers declared with a generated_co_wrapper mark
-func_decl_re = re.compile(r'^int\s*generated_co_wrapper\s*'
+func_decl_re = re.compile(r'^int\s*generated_co_wrapper'
+                          r'(?P<variant>(_[a-z][a-z0-9_]*)?)\s*'
                           r'(?P<wrapper_name>[a-z][a-z0-9_]*)'
                           r'\((?P<args>[^)]*)\);$', re.MULTILINE)
 
@@ -84,7 +90,8 @@  def func_decl_iter(text: str) -> Iterator:
     for m in func_decl_re.finditer(text):
         yield FuncDecl(return_type='int',
                        name=m.group('wrapper_name'),
-                       args=m.group('args'))
+                       args=m.group('args'),
+                       variant=m.group('variant'))
 
 
 def snake_to_camel(func_name: str) -> str:
@@ -97,6 +104,51 @@  def snake_to_camel(func_name: str) -> str:
     return ''.join(words)
 
 
+# Checks if we are already in coroutine
+def create_g_c_w(func: FuncDecl) -> str:
+    name = func.co_name
+    struct_name = func.struct_name
+    return f"""\
+int {func.name}({ func.gen_list('{decl}') })
+{{
+    if (qemu_in_coroutine()) {{
+        return {name}({ func.gen_list('{name}') });
+    }} else {{
+        {struct_name} s = {{
+            .poll_state.bs = {func.bs},
+            .poll_state.in_progress = true,
+
+{ func.gen_block('            .{name} = {name},') }
+        }};
+
+        s.poll_state.co = qemu_coroutine_create({name}_entry, &s);
+
+        return bdrv_poll_co(&s.poll_state);
+    }}
+}}"""
+
+
+# Assumes we are not in coroutine, and creates one
+def create_coroutine_only(func: FuncDecl) -> str:
+    name = func.co_name
+    struct_name = func.struct_name
+    return f"""\
+int {func.name}({ func.gen_list('{decl}') })
+{{
+    assert(!qemu_in_coroutine());
+    {struct_name} s = {{
+        .poll_state.bs = {func.bs},
+        .poll_state.in_progress = true,
+
+{ func.gen_block('        .{name} = {name},') }
+    }};
+
+    s.poll_state.co = qemu_coroutine_create({name}_entry, &s);
+
+    return bdrv_poll_co(&s.poll_state);
+}}"""
+
+
 def gen_wrapper(func: FuncDecl) -> str:
     assert not '_co_' in func.name
     assert func.return_type == 'int'
@@ -105,7 +157,8 @@  def gen_wrapper(func: FuncDecl) -> str:
 
     subsystem, subname = func.name.split('_', 1)
 
-    name = f'{subsystem}_co_{subname}'
+    func.co_name = f'{subsystem}_co_{subname}'
+    name = func.co_name
 
     t = func.args[0].type
     if t == 'BlockDriverState *':
@@ -114,7 +167,13 @@  def gen_wrapper(func: FuncDecl) -> str:
         bs = 'child->bs'
     else:
         bs = 'blk_bs(blk)'
-    struct_name = snake_to_camel(name)
+    func.bs = bs
+    func.struct_name = snake_to_camel(func.name)
+    struct_name = func.struct_name
+
+    creation_function = create_g_c_w
+    if func.create_only_co:
+        creation_function = create_coroutine_only
 
     return f"""\
 /*
@@ -136,23 +195,7 @@  def gen_wrapper(func: FuncDecl) -> str:
     aio_wait_kick();
 }}
 
-int {func.name}({ func.gen_list('{decl}') })
-{{
-    if (qemu_in_coroutine()) {{
-        return {name}({ func.gen_list('{name}') });
-    }} else {{
-        {struct_name} s = {{
-            .poll_state.bs = {bs},
-            .poll_state.in_progress = true,
-
-{ func.gen_block('            .{name} = {name},') }
-        }};
-
-        s.poll_state.co = qemu_coroutine_create({name}_entry, &s);
-
-        return bdrv_poll_co(&s.poll_state);
-    }}
-}}"""
+{creation_function(func)}"""
 
 
 def gen_wrappers(input_code: str) -> str: