Hi all!
We have a very frequent pattern of creating coroutine from function
with several arguments:
- create structure to pack parameters
- create _entry function to call original function taking parameters
from struct
- do different magic to handle completion: set ret to NOT_DONE or
EINPROGRESS, use separate bool for void functions
- fill the struct and create coroutine from _entry function and this
struct as a parameter
Here is a template code + example how it can be used to drop a lot of
similar code.
TODO: make coroutine-wrapper template file to be header itself, or
generate header from it
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
---
v2: - don't generate poll-loop wrappers
- drop jinja
Makefile | 5 ++
Makefile.objs | 2 +-
include/block/block.h | 3 ++
include/block/block_int.h | 8 +++
block.c | 76 +++++++---------------------
coroutine-wrapper | 2 +
scripts/coroutine-wrapper.py | 98 ++++++++++++++++++++++++++++++++++++
7 files changed, 136 insertions(+), 58 deletions(-)
create mode 100644 coroutine-wrapper
create mode 100755 scripts/coroutine-wrapper.py
diff --git a/Makefile b/Makefile
index 1278a3eb52..8d19b06cf1 100644
--- a/Makefile
+++ b/Makefile
@@ -122,6 +122,8 @@ endif
GENERATED_FILES += module_block.h
+GENERATED_FILES += block-gen.c
+
TRACE_HEADERS = trace-root.h $(trace-events-subdirs:%=%/trace.h)
TRACE_SOURCES = trace-root.c $(trace-events-subdirs:%=%/trace.c)
TRACE_DTRACE =
@@ -138,6 +140,9 @@ GENERATED_FILES += $(TRACE_SOURCES)
GENERATED_FILES += $(BUILD_DIR)/trace-events-all
GENERATED_FILES += .git-submodule-status
+block-gen.c: coroutine-wrapper $(SRC_PATH)/scripts/coroutine-wrapper.py
+ $(call quiet-command, python $(SRC_PATH)/scripts/coroutine-wrapper.py <
$< > $@,"GEN","$(TARGET_DIR)$@")
+
trace-group-name = $(shell dirname $1 | sed -e 's/[^a-zA-Z0-9]/_/g')
tracetool-y = $(SRC_PATH)/scripts/tracetool.py
diff --git a/Makefile.objs b/Makefile.objs
index 67a054b08a..16159d3e0f 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -23,7 +23,7 @@ slirp-obj-$(CONFIG_SLIRP) = slirp/
# block-obj-y is code used by both qemu system emulation and qemu-img
block-obj-y += nbd/
-block-obj-y += block.o blockjob.o job.o
+block-obj-y += block.o blockjob.o job.o block-gen.o
block-obj-y += block/ scsi/
block-obj-y += qemu-io-cmds.o
block-obj-$(CONFIG_REPLICATION) += replication.o
diff --git a/include/block/block.h b/include/block/block.h
index 57233cf2c0..61b2ca6fe5 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -371,6 +371,8 @@ typedef enum {
} BdrvCheckMode;
int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix);
+int coroutine_fn bdrv_co_check(BlockDriverState *bs,
+ BdrvCheckResult *res, BdrvCheckMode fix);
/* The units of offset and total_work_size may be chosen arbitrarily by the
* block driver; total_work_size may change during the course of the amendment
@@ -399,6 +401,7 @@ int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf);
/* Invalidate any cached metadata used by image formats */
void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp);
+void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp);
void bdrv_invalidate_cache_all(Error **errp);
int bdrv_inactivate_all(void);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index f605622216..9af6507be7 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1175,4 +1175,12 @@ int coroutine_fn bdrv_co_copy_range_to(BdrvChild *src,
uint64_t src_offset,
int refresh_total_sectors(BlockDriverState *bs, int64_t hint);
+
+Coroutine *bdrv_co_invalidate_cache__create_co(bool *_in_progress,
+ BlockDriverState *bs,
+ Error **errp);
+Coroutine *bdrv_co_check__create_co(bool *_in_progress, int *_ret,
+ BlockDriverState *bs, BdrvCheckResult *res,
+ BdrvCheckMode fix);
+
#endif /* BLOCK_INT_H */
diff --git a/block.c b/block.c
index b67d9b7b65..94999bc0fc 100644
--- a/block.c
+++ b/block.c
@@ -3706,8 +3706,8 @@ static void bdrv_delete(BlockDriverState *bs)
* free of errors) or -errno when an internal error occurred. The results of
the
* check are stored in res.
*/
-static int coroutine_fn bdrv_co_check(BlockDriverState *bs,
- BdrvCheckResult *res, BdrvCheckMode fix)
+int coroutine_fn bdrv_co_check(BlockDriverState *bs,
+ BdrvCheckResult *res, BdrvCheckMode fix)
{
if (bs->drv == NULL) {
return -ENOMEDIUM;
@@ -3720,44 +3720,25 @@ static int coroutine_fn bdrv_co_check(BlockDriverState
*bs,
return bs->drv->bdrv_co_check(bs, res, fix);
}
-typedef struct CheckCo {
- BlockDriverState *bs;
- BdrvCheckResult *res;
- BdrvCheckMode fix;
- int ret;
-} CheckCo;
-
-static void bdrv_check_co_entry(void *opaque)
-{
- CheckCo *cco = opaque;
- cco->ret = bdrv_co_check(cco->bs, cco->res, cco->fix);
- aio_wait_kick();
-}
-
int bdrv_check(BlockDriverState *bs,
BdrvCheckResult *res, BdrvCheckMode fix)
{
+ int ret;
+ bool in_progress;
Coroutine *co;
- CheckCo cco = {
- .bs = bs,
- .res = res,
- .ret = -EINPROGRESS,
- .fix = fix,
- };
if (qemu_in_coroutine()) {
- /* Fast-path if already in coroutine context */
- bdrv_check_co_entry(&cco);
- } else {
- co = qemu_coroutine_create(bdrv_check_co_entry, &cco);
- bdrv_coroutine_enter(bs, co);
- BDRV_POLL_WHILE(bs, cco.ret == -EINPROGRESS);
+ return bdrv_co_check(bs, res, fix);
}
- return cco.ret;
-}
+ co = bdrv_co_check__create_co(&in_progress, &ret, bs, res, fix);
+ bdrv_coroutine_enter(bs, co);
+ BDRV_POLL_WHILE(bs, in_progress);
+ return ret;
+}
/*
+ *
* Return values:
* 0 - success
* -EINVAL - backing format specified, but no file
@@ -4623,8 +4604,7 @@ void bdrv_init_with_whitelist(void)
bdrv_init();
}
-static void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs,
- Error **errp)
+void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp)
{
BdrvChild *child, *parent;
uint64_t perm, shared_perm;
@@ -4705,37 +4685,19 @@ static void coroutine_fn
bdrv_co_invalidate_cache(BlockDriverState *bs,
}
}
-typedef struct InvalidateCacheCo {
- BlockDriverState *bs;
- Error **errp;
- bool done;
-} InvalidateCacheCo;
-
-static void coroutine_fn bdrv_invalidate_cache_co_entry(void *opaque)
-{
- InvalidateCacheCo *ico = opaque;
- bdrv_co_invalidate_cache(ico->bs, ico->errp);
- ico->done = true;
- aio_wait_kick();
-}
-
void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
{
+ bool in_progress;
Coroutine *co;
- InvalidateCacheCo ico = {
- .bs = bs,
- .done = false,
- .errp = errp
- };
if (qemu_in_coroutine()) {
- /* Fast-path if already in coroutine context */
- bdrv_invalidate_cache_co_entry(&ico);
- } else {
- co = qemu_coroutine_create(bdrv_invalidate_cache_co_entry, &ico);
- bdrv_coroutine_enter(bs, co);
- BDRV_POLL_WHILE(bs, !ico.done);
+ bdrv_co_invalidate_cache(bs, errp);
+ return;
}
+
+ co = bdrv_co_invalidate_cache__create_co(&in_progress, bs, errp);
+ bdrv_coroutine_enter(bs, co);
+ BDRV_POLL_WHILE(bs, in_progress);
}
void bdrv_invalidate_cache_all(Error **errp)
diff --git a/coroutine-wrapper b/coroutine-wrapper
new file mode 100644
index 0000000000..47bcd2e6a0
--- /dev/null
+++ b/coroutine-wrapper
@@ -0,0 +1,2 @@
+int bdrv_co_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode
fix)
+void bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp)
diff --git a/scripts/coroutine-wrapper.py b/scripts/coroutine-wrapper.py
new file mode 100755
index 0000000000..35b0ba8c59
--- /dev/null
+++ b/scripts/coroutine-wrapper.py
@@ -0,0 +1,98 @@
+#!/usr/bin/env python
+
+import re
+
+header = """/*
+ * File is generated by scripts/coroutine-wrapper.py
+ */
+
+#include "qemu/osdep.h"
+#include "block/block_int.h"
+"""
+
+template = """
+/*
+ * Wrappers for $name$
+ */
+
+typedef struct $name$__ArgumentsPack {
+ $fields$
+} $name$__ArgumentsPack;
+
+static void $name$__entry(void *opaque)
+{
+ $name$__ArgumentsPack *pack = opaque;
+
+ $call$;
+
+ *pack->_in_progress = false;
+ aio_wait_kick();
+ g_free(opaque);
+}
+
+Coroutine *$name$__create_co($args_def$)
+{
+ $name$__ArgumentsPack *pack = g_new($name$__ArgumentsPack, 1);
+
+ *pack = ($name$__ArgumentsPack) {
+ $initializers$
+ };
+
+ *_in_progress = true;
+
+ return qemu_coroutine_create($name$__entry, pack);
+}
+"""
+
+# We want to use python string.format() formatter, which uses curly brackets
+# as separators. But it's not comfortable with C. So, we used dollars instead,
+# and now is the time to escape curly brackets and convert dollars.
+template = template.replace('{', '{{').replace('}', '}}')
+template = re.sub(r'\$(\w+)\$', r'{\1}', template)
+
+func_decl_re = re.compile(r'^([^(]+) ([a-z][a-z0-9_]*)\((.*)\)$')
+param_re = re.compile(r'(?P<def>.*[ *](?P<name>[a-z][a-z0-9_]*))')
+
+
+def format_args(args, format, separator):
+ return separator.join(format.format(**arg) for arg in args)
+
+
+def make_wrapper(function_declaration):
+ try:
+ m = func_decl_re.match(function_declaration)
+ ret_type = m.group(1).strip()
+ name = m.group(2)
+ raw_args = m.group(3).split(', ')
+ args = [param_re.match(arg).groupdict() for arg in raw_args]
+ except AttributeError:
+ raise ValueError('Failed to parse function declaration')
+
+ xargs = [{'def': 'bool *_in_progress', 'name': '_in_progress'}] + args
+
+ has_ret = ret_type != 'void'
+ if has_ret:
+ xargs.insert(1, {'def': ret_type + ' * _ret', 'name': '_ret'})
+
+ params = {
+ 'name': name,
+ 'args_def': format_args(xargs, '{def}', ', '),
+ 'fields': format_args(xargs, '{def};', '\n '),
+ 'initializers': format_args(xargs, '.{name} = {name},', '\n '),
+ 'call': '{}{}({})'.format('*pack->_ret = ' if has_ret else '',
+ name,
+ format_args(args, 'pack->{name}', ', '))
+ }
+
+ return template.format(**params)
+
+
+if __name__ == '__main__':
+ import sys
+
+ print(header)
+ try:
+ for ind, line in enumerate(sys.stdin):
+ print(make_wrapper(line))
+ except ValueError as e:
+ sys.exit(('ERROR: {} at line {}:\n{}').format(e, ind + 1, line))
--
2.18.0
