[PATCH v3 0/3] coroutines: generate wrapper code

[PATCH v3 0/3] coroutines: generate wrapper code

[Vladimir Sementsov-Ogievskiy]

Hi all!

After a long delay (~year) here is a v3.

The aim of the series is to reduce code-duplication and writing
parameters structure-packing by hand around coroutine function wrappers.

It's an alternative to "[PATCH v3] block: Factor out bdrv_run_co()"
patch.

Benefits:
 - no code duplication
 - less indirection

Vladimir Sementsov-Ogievskiy (3):
  block/io: refactor coroutine wrappers
  block: declare some coroutine functions in block/coroutines.h
  block: generate coroutine-wrapper code

 Makefile                             |   6 +
 block/block-gen.h                    |  30 +++
 block/coroutines.h                   |  44 ++++
 include/block/block.h                |  17 +-
 include/block/generated-co-wrapper.h |  11 +
 block.c                              |  78 +------
 block/io.c                           | 295 ++-------------------------
 block/Makefile.objs                  |   1 +
 scripts/coroutine-wrapper.py         | 169 +++++++++++++++
 9 files changed, 296 insertions(+), 355 deletions(-)
 create mode 100644 block/block-gen.h
 create mode 100644 block/coroutines.h
 create mode 100644 include/block/generated-co-wrapper.h
 create mode 100755 scripts/coroutine-wrapper.py

-- 
2.21.0

[PATCH v3 1/3] block/io: refactor coroutine wrappers

[Vladimir Sementsov-Ogievskiy]

Most of coroutine wrappers already follow this notation:

We have coroutine_fn bdrv_co_<something>(<normal argument list>), which
is the core functions, and wrapper, which does polling loope is called
bdrv_<something>(<same argument list>).

The only outsiders are bdrv_prwv_co and bdrv_common_block_status_above
wrappers. Let's refactor the to behave as the others, it simplifies
further conversion of coroutine wrappers.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/io.c | 61 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/block/io.c b/block/io.c
index 121ce17a49..bd00a70b47 100644
--- a/block/io.c
+++ b/block/io.c
@@ -900,28 +900,32 @@ typedef struct RwCo {
     BdrvRequestFlags flags;
 } RwCo;
 
+static int coroutine_fn bdrv_co_prwv(BdrvChild *child, int64_t offset,
+                                     QEMUIOVector *qiov, bool is_write,
+                                     BdrvRequestFlags flags)
+{
+    if (is_write) {
+        return bdrv_co_pwritev(child, offset, qiov->size, qiov, flags);
+    } else {
+        return bdrv_co_preadv(child, offset, qiov->size, qiov, flags);
+    }
+}
+
 static void coroutine_fn bdrv_rw_co_entry(void *opaque)
 {
     RwCo *rwco = opaque;
 
-    if (!rwco->is_write) {
-        rwco->ret = bdrv_co_preadv(rwco->child, rwco->offset,
-                                   rwco->qiov->size, rwco->qiov,
-                                   rwco->flags);
-    } else {
-        rwco->ret = bdrv_co_pwritev(rwco->child, rwco->offset,
-                                    rwco->qiov->size, rwco->qiov,
-                                    rwco->flags);
-    }
+    rwco->ret = bdrv_co_prwv(rwco->child, rwco->offset, rwco->qiov,
+                             rwco->is_write, rwco->flags);
     aio_wait_kick();
 }
 
 /*
  * Process a vectored synchronous request using coroutines
  */
-static int bdrv_prwv_co(BdrvChild *child, int64_t offset,
-                        QEMUIOVector *qiov, bool is_write,
-                        BdrvRequestFlags flags)
+static int bdrv_prwv(BdrvChild *child, int64_t offset,
+                     QEMUIOVector *qiov, bool is_write,
+                     BdrvRequestFlags flags)
 {
     Coroutine *co;
     RwCo rwco = {
@@ -949,8 +953,7 @@ int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset,
 {
     QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, bytes);
 
-    return bdrv_prwv_co(child, offset, &qiov, true,
-                        BDRV_REQ_ZERO_WRITE | flags);
+    return bdrv_prwv(child, offset, &qiov, true, BDRV_REQ_ZERO_WRITE | flags);
 }
 
 /*
@@ -999,7 +1002,7 @@ int bdrv_preadv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov)
 {
     int ret;
 
-    ret = bdrv_prwv_co(child, offset, qiov, false, 0);
+    ret = bdrv_prwv(child, offset, qiov, false, 0);
     if (ret < 0) {
         return ret;
     }
@@ -1023,7 +1026,7 @@ int bdrv_pwritev(BdrvChild *child, int64_t offset, QEMUIOVector *qiov)
 {
     int ret;
 
-    ret = bdrv_prwv_co(child, offset, qiov, true, 0);
+    ret = bdrv_prwv(child, offset, qiov, true, 0);
     if (ret < 0) {
         return ret;
     }
@@ -2443,14 +2446,15 @@ early_out:
     return ret;
 }
 
-static int coroutine_fn bdrv_co_block_status_above(BlockDriverState *bs,
-                                                   BlockDriverState *base,
-                                                   bool want_zero,
-                                                   int64_t offset,
-                                                   int64_t bytes,
-                                                   int64_t *pnum,
-                                                   int64_t *map,
-                                                   BlockDriverState **file)
+static int coroutine_fn
+bdrv_co_common_block_status_above(BlockDriverState *bs,
+                                  BlockDriverState *base,
+                                  bool want_zero,
+                                  int64_t offset,
+                                  int64_t bytes,
+                                  int64_t *pnum,
+                                  int64_t *map,
+                                  BlockDriverState **file)
 {
     BlockDriverState *p;
     int ret = 0;
@@ -2488,10 +2492,11 @@ static void coroutine_fn bdrv_block_status_above_co_entry(void *opaque)
 {
     BdrvCoBlockStatusData *data = opaque;
 
-    data->ret = bdrv_co_block_status_above(data->bs, data->base,
-                                           data->want_zero,
-                                           data->offset, data->bytes,
-                                           data->pnum, data->map, data->file);
+    data->ret = bdrv_co_common_block_status_above(data->bs, data->base,
+                                                  data->want_zero,
+                                                  data->offset, data->bytes,
+                                                  data->pnum, data->map,
+                                                  data->file);
     data->done = true;
     aio_wait_kick();
 }
-- 
2.21.0

[PATCH v3 2/3] block: declare some coroutine functions in block/coroutines.h

[Vladimir Sementsov-Ogievskiy]

We are going to keep coroutine-wrappers code (structure-packing
parameters, BDRV_POLL wrapper functions) in a separate auto-generated
files. So, we'll need a header with declaration of original _co_
functions, for those which are static now. As well, we'll need
declarations for wrapper functions. Do these declarations now, as a
preparation step.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/coroutines.h | 43 +++++++++++++++++++++++++++++++++++++++++++
 block.c            |  8 ++++----
 block/io.c         | 34 +++++++++++++++++-----------------
 3 files changed, 64 insertions(+), 21 deletions(-)
 create mode 100644 block/coroutines.h

diff --git a/block/coroutines.h b/block/coroutines.h
new file mode 100644
index 0000000000..23ea6fd5b3
--- /dev/null
+++ b/block/coroutines.h
@@ -0,0 +1,43 @@
+#ifndef BLOCK_COROUTINES_INT_H
+#define BLOCK_COROUTINES_INT_H
+
+#include "block/block_int.h"
+
+int coroutine_fn bdrv_co_check(BlockDriverState *bs,
+                               BdrvCheckResult *res, BdrvCheckMode fix);
+void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp);
+
+int coroutine_fn
+bdrv_co_prwv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov,
+             bool is_write, BdrvRequestFlags flags);
+int
+bdrv_prwv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov,
+          bool is_write, BdrvRequestFlags flags);
+
+int coroutine_fn
+bdrv_co_common_block_status_above(BlockDriverState *bs,
+                                  BlockDriverState *base,
+                                  bool want_zero,
+                                  int64_t offset,
+                                  int64_t bytes,
+                                  int64_t *pnum,
+                                  int64_t *map,
+                                  BlockDriverState **file);
+int
+bdrv_common_block_status_above(BlockDriverState *bs,
+                               BlockDriverState *base,
+                               bool want_zero,
+                               int64_t offset,
+                               int64_t bytes,
+                               int64_t *pnum,
+                               int64_t *map,
+                               BlockDriverState **file);
+
+int coroutine_fn
+bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
+                   bool is_read);
+int
+bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
+                bool is_read);
+
+#endif /* BLOCK_COROUTINES_INT_H */
diff --git a/block.c b/block.c
index 8416376c9b..7f06e82880 100644
--- a/block.c
+++ b/block.c
@@ -48,6 +48,7 @@
 #include "qemu/timer.h"
 #include "qemu/cutils.h"
 #include "qemu/id.h"
+#include "block/coroutines.h"
 
 #ifdef CONFIG_BSD
 #include <sys/ioctl.h>
@@ -4625,8 +4626,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;
@@ -5643,8 +5644,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;
diff --git a/block/io.c b/block/io.c
index bd00a70b47..f5b6ce3bf6 100644
--- a/block/io.c
+++ b/block/io.c
@@ -29,6 +29,7 @@
 #include "block/blockjob.h"
 #include "block/blockjob_int.h"
 #include "block/block_int.h"
+#include "block/coroutines.h"
 #include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
@@ -900,9 +901,9 @@ typedef struct RwCo {
     BdrvRequestFlags flags;
 } RwCo;
 
-static int coroutine_fn bdrv_co_prwv(BdrvChild *child, int64_t offset,
-                                     QEMUIOVector *qiov, bool is_write,
-                                     BdrvRequestFlags flags)
+int coroutine_fn bdrv_co_prwv(BdrvChild *child, int64_t offset,
+                              QEMUIOVector *qiov, bool is_write,
+                              BdrvRequestFlags flags)
 {
     if (is_write) {
         return bdrv_co_pwritev(child, offset, qiov->size, qiov, flags);
@@ -923,9 +924,9 @@ static void coroutine_fn bdrv_rw_co_entry(void *opaque)
 /*
  * Process a vectored synchronous request using coroutines
  */
-static int bdrv_prwv(BdrvChild *child, int64_t offset,
-                     QEMUIOVector *qiov, bool is_write,
-                     BdrvRequestFlags flags)
+int bdrv_prwv(BdrvChild *child, int64_t offset,
+              QEMUIOVector *qiov, bool is_write,
+              BdrvRequestFlags flags)
 {
     Coroutine *co;
     RwCo rwco = {
@@ -2446,7 +2447,7 @@ early_out:
     return ret;
 }
 
-static int coroutine_fn
+int coroutine_fn
 bdrv_co_common_block_status_above(BlockDriverState *bs,
                                   BlockDriverState *base,
                                   bool want_zero,
@@ -2506,12 +2507,12 @@ static void coroutine_fn bdrv_block_status_above_co_entry(void *opaque)
  *
  * See bdrv_co_block_status_above() for details.
  */
-static int bdrv_common_block_status_above(BlockDriverState *bs,
-                                          BlockDriverState *base,
-                                          bool want_zero, int64_t offset,
-                                          int64_t bytes, int64_t *pnum,
-                                          int64_t *map,
-                                          BlockDriverState **file)
+int bdrv_common_block_status_above(BlockDriverState *bs,
+                                   BlockDriverState *base,
+                                   bool want_zero, int64_t offset,
+                                   int64_t bytes, int64_t *pnum,
+                                   int64_t *map,
+                                   BlockDriverState **file)
 {
     Coroutine *co;
     BdrvCoBlockStatusData data = {
@@ -2638,7 +2639,7 @@ typedef struct BdrvVmstateCo {
     int                 ret;
 } BdrvVmstateCo;
 
-static int coroutine_fn
+int coroutine_fn
 bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
                    bool is_read)
 {
@@ -2670,9 +2671,8 @@ static void coroutine_fn bdrv_co_rw_vmstate_entry(void *opaque)
     aio_wait_kick();
 }
 
-static inline int
-bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
-                bool is_read)
+int bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
+                    bool is_read)
 {
     if (qemu_in_coroutine()) {
         return bdrv_co_rw_vmstate(bs, qiov, pos, is_read);
-- 
2.21.0

[PATCH v3 3/3] block: generate coroutine-wrapper code

[Vladimir Sementsov-Ogievskiy]

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
  - do coroutine enter and BDRV_POLL_WHILE loop

Let's reduce code duplication. Here:

Functional part (BDRV_POLL_WHILE loop, aio_wait_kick()) moved to
(non-generated) block/block-gen.h

Mechanical part (arguments packing, different kind of needed wrappers)
are generated from template by scripts/coroutine-wrapper.py to
resulting file block/block-gen.c

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 Makefile                             |   6 +
 block/block-gen.h                    |  30 ++++
 block/coroutines.h                   |   7 +-
 include/block/block.h                |  17 +-
 include/block/generated-co-wrapper.h |  11 ++
 block.c                              |  70 --------
 block/io.c                           | 260 ---------------------------
 block/Makefile.objs                  |   1 +
 scripts/coroutine-wrapper.py         | 169 +++++++++++++++++
 9 files changed, 232 insertions(+), 339 deletions(-)
 create mode 100644 block/block-gen.h
 create mode 100644 include/block/generated-co-wrapper.h
 create mode 100755 scripts/coroutine-wrapper.py

diff --git a/Makefile b/Makefile
index 40e4f7677b..67b1c7852f 100644
--- a/Makefile
+++ b/Makefile
@@ -159,6 +159,8 @@ generated-files-$(CONFIG_TRACE_UST) += trace-ust-all.c
 
 generated-files-y += module_block.h
 
+GENERATED_FILES += block/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 =
@@ -175,6 +177,10 @@ generated-files-y += $(TRACE_SOURCES)
 generated-files-y += $(BUILD_DIR)/trace-events-all
 generated-files-y += .git-submodule-status
 
+COROUTINE_HEADERS = include/block/block.h block/coroutines.h
+block/block-gen.c: $(COROUTINE_HEADERS) $(SRC_PATH)/scripts/coroutine-wrapper.py
+	$(call quiet-command, cat $(COROUTINE_HEADERS) | $(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/block/block-gen.h b/block/block-gen.h
new file mode 100644
index 0000000000..79762cdda9
--- /dev/null
+++ b/block/block-gen.h
@@ -0,0 +1,30 @@
+#ifndef BLOCK_BLOCK_GEN_H
+#define BLOCK_BLOCK_GEN_H
+
+#include "block/block_int.h"
+
+/* This function is called at the end of generated coroutine entries. */
+static inline void bdrv_poll_co__on_exit(void)
+{
+    aio_wait_kick();
+}
+
+/* Base structure for argument packing structures */
+typedef struct BdrvPollCo {
+    BlockDriverState *bs;
+    bool in_progress;
+    int ret;
+    Coroutine *co; /* Keep pointer here for debugging */
+} BdrvPollCo;
+
+static inline int bdrv_poll_co(BdrvPollCo *s)
+{
+    assert(!qemu_in_coroutine());
+
+    bdrv_coroutine_enter(s->bs, s->co);
+    BDRV_POLL_WHILE(s->bs, s->in_progress);
+
+    return s->ret;
+}
+
+#endif /* BLOCK_BLOCK_GEN_H */
diff --git a/block/coroutines.h b/block/coroutines.h
index 23ea6fd5b3..6eb32ac387 100644
--- a/block/coroutines.h
+++ b/block/coroutines.h
@@ -2,6 +2,7 @@
 #define BLOCK_COROUTINES_INT_H
 
 #include "block/block_int.h"
+#include "block/generated-co-wrapper.h"
 
 int coroutine_fn bdrv_co_check(BlockDriverState *bs,
                                BdrvCheckResult *res, BdrvCheckMode fix);
@@ -10,7 +11,7 @@ void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp);
 int coroutine_fn
 bdrv_co_prwv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov,
              bool is_write, BdrvRequestFlags flags);
-int
+int generated_co_wrapper
 bdrv_prwv(BdrvChild *child, int64_t offset, QEMUIOVector *qiov,
           bool is_write, BdrvRequestFlags flags);
 
@@ -23,7 +24,7 @@ bdrv_co_common_block_status_above(BlockDriverState *bs,
                                   int64_t *pnum,
                                   int64_t *map,
                                   BlockDriverState **file);
-int
+int generated_co_wrapper
 bdrv_common_block_status_above(BlockDriverState *bs,
                                BlockDriverState *base,
                                bool want_zero,
@@ -36,7 +37,7 @@ bdrv_common_block_status_above(BlockDriverState *bs,
 int coroutine_fn
 bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
                    bool is_read);
-int
+int generated_co_wrapper
 bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
                 bool is_read);
 
diff --git a/include/block/block.h b/include/block/block.h
index 25e299605e..aed6ffcc4f 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -9,6 +9,7 @@
 #include "block/dirty-bitmap.h"
 #include "block/blockjob.h"
 #include "qemu/hbitmap.h"
+#include "block/generated-co-wrapper.h"
 
 /* block.c */
 typedef struct BlockDriver BlockDriver;
@@ -398,8 +399,9 @@ void bdrv_refresh_filename(BlockDriverState *bs);
 int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
                                   PreallocMode prealloc, BdrvRequestFlags flags,
                                   Error **errp);
-int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact,
-                  PreallocMode prealloc, BdrvRequestFlags flags, Error **errp);
+int generated_co_wrapper
+bdrv_truncate(BdrvChild *child, int64_t offset, bool exact,
+              PreallocMode prealloc, BdrvRequestFlags flags, Error **errp);
 
 int64_t bdrv_nb_sectors(BlockDriverState *bs);
 int64_t bdrv_getlength(BlockDriverState *bs);
@@ -441,7 +443,8 @@ typedef enum {
     BDRV_FIX_ERRORS   = 2,
 } BdrvCheckMode;
 
-int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix);
+int generated_co_wrapper bdrv_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
@@ -464,12 +467,13 @@ void bdrv_aio_cancel_async(BlockAIOCB *acb);
 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 generated_co_wrapper bdrv_invalidate_cache(BlockDriverState *bs,
+                                                Error **errp);
 void bdrv_invalidate_cache_all(Error **errp);
 int bdrv_inactivate_all(void);
 
 /* Ensure contents are flushed to disk.  */
-int bdrv_flush(BlockDriverState *bs);
+int generated_co_wrapper bdrv_flush(BlockDriverState *bs);
 int coroutine_fn bdrv_co_flush(BlockDriverState *bs);
 int bdrv_flush_all(void);
 void bdrv_close_all(void);
@@ -484,7 +488,8 @@ void bdrv_drain_all(void);
     AIO_WAIT_WHILE(bdrv_get_aio_context(bs_),              \
                    cond); })
 
-int bdrv_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes);
+int generated_co_wrapper bdrv_pdiscard(BdrvChild *child, int64_t offset,
+                                       int64_t bytes);
 int bdrv_co_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes);
 int bdrv_has_zero_init_1(BlockDriverState *bs);
 int bdrv_has_zero_init(BlockDriverState *bs);
diff --git a/include/block/generated-co-wrapper.h b/include/block/generated-co-wrapper.h
new file mode 100644
index 0000000000..80a5433af4
--- /dev/null
+++ b/include/block/generated-co-wrapper.h
@@ -0,0 +1,11 @@
+#ifndef BLOCK_GENERATED_CO_WRAPPER_H
+#define BLOCK_GENERATED_CO_WRAPPER_H
+
+/*
+ * generated_co_wrapper
+ * Function specifier, which does nothing but marking functions to be
+ * generated by scripts/coroutine-wrapper.py
+ */
+#define generated_co_wrapper
+
+#endif /* BLOCK_GENERATED_CO_WRAPPER_H */
diff --git a/block.c b/block.c
index 7f06e82880..c1132ab323 100644
--- a/block.c
+++ b/block.c
@@ -4640,43 +4640,6 @@ 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 coroutine_fn 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)
-{
-    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 cco.ret;
-}
-
 /*
  * Return values:
  * 0        - success
@@ -5721,39 +5684,6 @@ void coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp)
     }
 }
 
-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)
-{
-    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);
-    }
-}
-
 void bdrv_invalidate_cache_all(Error **errp)
 {
     BlockDriverState *bs;
diff --git a/block/io.c b/block/io.c
index f5b6ce3bf6..f9700cc897 100644
--- a/block/io.c
+++ b/block/io.c
@@ -892,15 +892,6 @@ static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
     return 0;
 }
 
-typedef struct RwCo {
-    BdrvChild *child;
-    int64_t offset;
-    QEMUIOVector *qiov;
-    bool is_write;
-    int ret;
-    BdrvRequestFlags flags;
-} RwCo;
-
 int coroutine_fn bdrv_co_prwv(BdrvChild *child, int64_t offset,
                               QEMUIOVector *qiov, bool is_write,
                               BdrvRequestFlags flags)
@@ -912,43 +903,6 @@ int coroutine_fn bdrv_co_prwv(BdrvChild *child, int64_t offset,
     }
 }
 
-static void coroutine_fn bdrv_rw_co_entry(void *opaque)
-{
-    RwCo *rwco = opaque;
-
-    rwco->ret = bdrv_co_prwv(rwco->child, rwco->offset, rwco->qiov,
-                             rwco->is_write, rwco->flags);
-    aio_wait_kick();
-}
-
-/*
- * Process a vectored synchronous request using coroutines
- */
-int bdrv_prwv(BdrvChild *child, int64_t offset,
-              QEMUIOVector *qiov, bool is_write,
-              BdrvRequestFlags flags)
-{
-    Coroutine *co;
-    RwCo rwco = {
-        .child = child,
-        .offset = offset,
-        .qiov = qiov,
-        .is_write = is_write,
-        .ret = NOT_DONE,
-        .flags = flags,
-    };
-
-    if (qemu_in_coroutine()) {
-        /* Fast-path if already in coroutine context */
-        bdrv_rw_co_entry(&rwco);
-    } else {
-        co = qemu_coroutine_create(bdrv_rw_co_entry, &rwco);
-        bdrv_coroutine_enter(child->bs, co);
-        BDRV_POLL_WHILE(child->bs, rwco.ret == NOT_DONE);
-    }
-    return rwco.ret;
-}
-
 int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset,
                        int bytes, BdrvRequestFlags flags)
 {
@@ -2223,20 +2177,6 @@ int bdrv_flush_all(void)
     return result;
 }
 
-
-typedef struct BdrvCoBlockStatusData {
-    BlockDriverState *bs;
-    BlockDriverState *base;
-    bool want_zero;
-    int64_t offset;
-    int64_t bytes;
-    int64_t *pnum;
-    int64_t *map;
-    BlockDriverState **file;
-    int ret;
-    bool done;
-} BdrvCoBlockStatusData;
-
 int coroutine_fn bdrv_co_block_status_from_file(BlockDriverState *bs,
                                                 bool want_zero,
                                                 int64_t offset,
@@ -2488,56 +2428,6 @@ bdrv_co_common_block_status_above(BlockDriverState *bs,
     return ret;
 }
 
-/* Coroutine wrapper for bdrv_block_status_above() */
-static void coroutine_fn bdrv_block_status_above_co_entry(void *opaque)
-{
-    BdrvCoBlockStatusData *data = opaque;
-
-    data->ret = bdrv_co_common_block_status_above(data->bs, data->base,
-                                                  data->want_zero,
-                                                  data->offset, data->bytes,
-                                                  data->pnum, data->map,
-                                                  data->file);
-    data->done = true;
-    aio_wait_kick();
-}
-
-/*
- * Synchronous wrapper around bdrv_co_block_status_above().
- *
- * See bdrv_co_block_status_above() for details.
- */
-int bdrv_common_block_status_above(BlockDriverState *bs,
-                                   BlockDriverState *base,
-                                   bool want_zero, int64_t offset,
-                                   int64_t bytes, int64_t *pnum,
-                                   int64_t *map,
-                                   BlockDriverState **file)
-{
-    Coroutine *co;
-    BdrvCoBlockStatusData data = {
-        .bs = bs,
-        .base = base,
-        .want_zero = want_zero,
-        .offset = offset,
-        .bytes = bytes,
-        .pnum = pnum,
-        .map = map,
-        .file = file,
-        .done = false,
-    };
-
-    if (qemu_in_coroutine()) {
-        /* Fast-path if already in coroutine context */
-        bdrv_block_status_above_co_entry(&data);
-    } else {
-        co = qemu_coroutine_create(bdrv_block_status_above_co_entry, &data);
-        bdrv_coroutine_enter(bs, co);
-        BDRV_POLL_WHILE(bs, !data.done);
-    }
-    return data.ret;
-}
-
 int bdrv_block_status_above(BlockDriverState *bs, BlockDriverState *base,
                             int64_t offset, int64_t bytes, int64_t *pnum,
                             int64_t *map, BlockDriverState **file)
@@ -2631,14 +2521,6 @@ int bdrv_is_allocated_above(BlockDriverState *top,
     return 0;
 }
 
-typedef struct BdrvVmstateCo {
-    BlockDriverState    *bs;
-    QEMUIOVector        *qiov;
-    int64_t             pos;
-    bool                is_read;
-    int                 ret;
-} BdrvVmstateCo;
-
 int coroutine_fn
 bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
                    bool is_read)
@@ -2664,34 +2546,6 @@ bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
     return ret;
 }
 
-static void coroutine_fn bdrv_co_rw_vmstate_entry(void *opaque)
-{
-    BdrvVmstateCo *co = opaque;
-    co->ret = bdrv_co_rw_vmstate(co->bs, co->qiov, co->pos, co->is_read);
-    aio_wait_kick();
-}
-
-int bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
-                    bool is_read)
-{
-    if (qemu_in_coroutine()) {
-        return bdrv_co_rw_vmstate(bs, qiov, pos, is_read);
-    } else {
-        BdrvVmstateCo data = {
-            .bs         = bs,
-            .qiov       = qiov,
-            .pos        = pos,
-            .is_read    = is_read,
-            .ret        = -EINPROGRESS,
-        };
-        Coroutine *co = qemu_coroutine_create(bdrv_co_rw_vmstate_entry, &data);
-
-        bdrv_coroutine_enter(bs, co);
-        BDRV_POLL_WHILE(bs, data.ret == -EINPROGRESS);
-        return data.ret;
-    }
-}
-
 int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
                       int64_t pos, int size)
 {
@@ -2767,20 +2621,6 @@ void bdrv_aio_cancel_async(BlockAIOCB *acb)
 /**************************************************************/
 /* Coroutine block device emulation */
 
-typedef struct FlushCo {
-    BlockDriverState *bs;
-    int ret;
-} FlushCo;
-
-
-static void coroutine_fn bdrv_flush_co_entry(void *opaque)
-{
-    FlushCo *rwco = opaque;
-
-    rwco->ret = bdrv_co_flush(rwco->bs);
-    aio_wait_kick();
-}
-
 int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
 {
     int current_gen;
@@ -2893,40 +2733,6 @@ early_exit:
     return ret;
 }
 
-int bdrv_flush(BlockDriverState *bs)
-{
-    Coroutine *co;
-    FlushCo flush_co = {
-        .bs = bs,
-        .ret = NOT_DONE,
-    };
-
-    if (qemu_in_coroutine()) {
-        /* Fast-path if already in coroutine context */
-        bdrv_flush_co_entry(&flush_co);
-    } else {
-        co = qemu_coroutine_create(bdrv_flush_co_entry, &flush_co);
-        bdrv_coroutine_enter(bs, co);
-        BDRV_POLL_WHILE(bs, flush_co.ret == NOT_DONE);
-    }
-
-    return flush_co.ret;
-}
-
-typedef struct DiscardCo {
-    BdrvChild *child;
-    int64_t offset;
-    int64_t bytes;
-    int ret;
-} DiscardCo;
-static void coroutine_fn bdrv_pdiscard_co_entry(void *opaque)
-{
-    DiscardCo *rwco = opaque;
-
-    rwco->ret = bdrv_co_pdiscard(rwco->child, rwco->offset, rwco->bytes);
-    aio_wait_kick();
-}
-
 int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset,
                                   int64_t bytes)
 {
@@ -3041,28 +2847,6 @@ out:
     return ret;
 }
 
-int bdrv_pdiscard(BdrvChild *child, int64_t offset, int64_t bytes)
-{
-    Coroutine *co;
-    DiscardCo rwco = {
-        .child = child,
-        .offset = offset,
-        .bytes = bytes,
-        .ret = NOT_DONE,
-    };
-
-    if (qemu_in_coroutine()) {
-        /* Fast-path if already in coroutine context */
-        bdrv_pdiscard_co_entry(&rwco);
-    } else {
-        co = qemu_coroutine_create(bdrv_pdiscard_co_entry, &rwco);
-        bdrv_coroutine_enter(child->bs, co);
-        BDRV_POLL_WHILE(child->bs, rwco.ret == NOT_DONE);
-    }
-
-    return rwco.ret;
-}
-
 int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf)
 {
     BlockDriver *drv = bs->drv;
@@ -3460,47 +3244,3 @@ out:
 
     return ret;
 }
-
-typedef struct TruncateCo {
-    BdrvChild *child;
-    int64_t offset;
-    bool exact;
-    PreallocMode prealloc;
-    BdrvRequestFlags flags;
-    Error **errp;
-    int ret;
-} TruncateCo;
-
-static void coroutine_fn bdrv_truncate_co_entry(void *opaque)
-{
-    TruncateCo *tco = opaque;
-    tco->ret = bdrv_co_truncate(tco->child, tco->offset, tco->exact,
-                                tco->prealloc, tco->flags, tco->errp);
-    aio_wait_kick();
-}
-
-int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact,
-                  PreallocMode prealloc, BdrvRequestFlags flags, Error **errp)
-{
-    Coroutine *co;
-    TruncateCo tco = {
-        .child      = child,
-        .offset     = offset,
-        .exact      = exact,
-        .prealloc   = prealloc,
-        .flags      = flags,
-        .errp       = errp,
-        .ret        = NOT_DONE,
-    };
-
-    if (qemu_in_coroutine()) {
-        /* Fast-path if already in coroutine context */
-        bdrv_truncate_co_entry(&tco);
-    } else {
-        co = qemu_coroutine_create(bdrv_truncate_co_entry, &tco);
-        bdrv_coroutine_enter(child->bs, co);
-        BDRV_POLL_WHILE(child->bs, tco.ret == NOT_DONE);
-    }
-
-    return tco.ret;
-}
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 3635b6b4c1..05e4d033c1 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -45,6 +45,7 @@ block-obj-y += crypto.o
 block-obj-y += aio_task.o
 block-obj-y += backup-top.o
 block-obj-y += filter-compress.o
+block-obj-y += block-gen.o
 common-obj-y += monitor/
 
 block-obj-y += stream.o
diff --git a/scripts/coroutine-wrapper.py b/scripts/coroutine-wrapper.py
new file mode 100755
index 0000000000..cd9eefbe28
--- /dev/null
+++ b/scripts/coroutine-wrapper.py
@@ -0,0 +1,169 @@
+#!/usr/bin/env python3
+#
+# Generate coroutine wrappers for block subsystem.
+#
+# Copyright (c) 2020 Virtuozzo International GmbH.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import re
+from typing import List, Iterator
+
+header = """/*
+ * File is generated by scripts/coroutine-wrapper.py
+ */
+
+#include "qemu/osdep.h"
+#include "block/coroutines.h"
+#include "block/block-gen.h"
+"""
+
+template = """
+/*
+ * Wrappers for $name$
+ */
+
+typedef struct $struct_name$ {
+    BdrvPollCo poll_state;
+    $fields$
+} $struct_name$;
+
+static void coroutine_fn $name$_entry(void *opaque)
+{
+    $struct_name$ *s = opaque;
+
+    $assign_ret$$name$($args_from_s$);
+
+    s->poll_state.in_progress = false;
+
+    bdrv_poll_co__on_exit();
+}
+
+$ret_type$ $wrapper_name$($args_def$)
+{
+    if (qemu_in_coroutine()) {
+        $do_return$$name$($arg_names$);
+    } else {
+        $struct_name$ s = {
+            .poll_state.bs = $bs$,
+            .poll_state.in_progress = true,
+
+            $initializers$
+        };
+
+        s.poll_state.co = qemu_coroutine_create($name$_entry, &s);
+
+        $do_return$bdrv_poll_co(&s.poll_state);
+    }
+}
+"""
+
+# 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)
+
+
+class ParamDecl:
+    param_re = re.compile(r'(?P<decl>'
+                          r'(?P<type>.*[ *])'
+                          r'(?P<name>[a-z][a-z0-9_]*)'
+                          r')')
+
+    def __init__(self, param_decl: str) -> None:
+        m = self.param_re.match(param_decl.strip())
+        self.decl = m.group('decl')
+        self.type = m.group('type')
+        self.name = m.group('name')
+
+
+class FuncDecl:
+    def __init__(self, return_type: str, name: str, args: str) -> None:
+        self.return_type = return_type.strip()
+        self.name = name.strip()
+        self.args: List[ParamDecl] = []
+        self.args = [ParamDecl(arg) for arg in args.split(',')]
+
+    def get_args_decl(self) -> str:
+        return ', '.join(arg.decl for arg in self.args)
+
+    def get_arg_names(self) -> str:
+        return ', '.join(arg.name for arg in self.args)
+
+    def gen_struct_fields(self) -> str:
+        return '\n    '.join(f'{arg.decl};' for arg in self.args)
+
+    def gen_struct_initializers(self, indent: int) -> str:
+        sep = '\n' + ' ' * indent
+        return sep.join(f'.{a.name} = {a.name},' for a in self.args)
+
+
+# Match wrappers declaration, with generated_co_wrapper mark
+func_decl_re = re.compile(r'^(?P<return_type>(int|void))'
+                          r'\s*generated_co_wrapper\s*'
+                          r'(?P<wrapper_name>[a-z][a-z0-9_]*)'
+                          r'\((?P<args>[^)]*)\);$', re.MULTILINE)
+
+
+def func_decl_iter(text: str) -> Iterator:
+    for m in func_decl_re.finditer(text):
+        yield FuncDecl(return_type=m.group('return_type'),
+                       name=m.group('wrapper_name'),
+                       args=m.group('args'))
+
+
+def struct_name(func_name: str) -> str:
+    """some_function_name -> SomeFunctionName"""
+    words = func_name.split('_')
+    words = [w[0].upper() + w[1:] for w in words]
+    return ''.join(words)
+
+
+def make_wrapper(func: FuncDecl) -> str:
+    assert func.name.startswith('bdrv_')
+    co_name = 'bdrv_co_' + func.name[5:]
+
+    has_ret = func.return_type != 'void'
+
+    params = {
+        'name': co_name,
+        'do_return': 'return ' if has_ret else '',
+        'assign_ret': 's->poll_state.ret = ' if has_ret else '',
+        'struct_name': struct_name(co_name),
+        'wrapper_name': func.name,
+        'ret_type': func.return_type,
+        'args_def': func.get_args_decl(),
+        'arg_names': func.get_arg_names(),
+        'fields': func.gen_struct_fields(),
+        'initializers': func.gen_struct_initializers(12),
+        'args_from_s': ', '.join(f's->{a.name}' for a in func.args),
+    }
+
+    if func.args[0].type == 'BlockDriverState *':
+        params['bs'] = 'bs'
+    else:
+        assert func.args[0].type == 'BdrvChild *'
+        params['bs'] = 'child->bs'
+
+    return template.format(**params)
+
+
+if __name__ == '__main__':
+    import sys
+
+    print(header)
+    for func in func_decl_iter(sys.stdin.read()):
+        print(make_wrapper(func))
-- 
2.21.0

Re: [PATCH v3 0/3] coroutines: generate wrapper code

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200522161950.2839-1-vsementsov@virtuozzo.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

block/vhdx-log.o: In function `vhdx_log_write_and_flush':
/tmp/qemu-test/src/block/vhdx-log.c:1049: undefined reference to `bdrv_flush'
/tmp/qemu-test/src/block/vhdx-log.c:1061: undefined reference to `bdrv_flush'
collect2: error: ld returned 1 exit status
make: *** [qemu-nbd] Error 1
make: *** Waiting for unfinished jobs....
block.o: In function `bdrv_invalidate_cache_all':
/tmp/qemu-test/src/block.c:5697: undefined reference to `bdrv_invalidate_cache'
---
block/vhdx-log.o: In function `vhdx_log_write_and_flush':
/tmp/qemu-test/src/block/vhdx-log.c:1049: undefined reference to `bdrv_flush'
/tmp/qemu-test/src/block/vhdx-log.c:1061: undefined reference to `bdrv_flush'
collect2: error: ld returned 1 exit status
make: *** [qemu-storage-daemon] Error 1
block.o: In function `bdrv_invalidate_cache_all':
/tmp/qemu-test/src/block.c:5697: undefined reference to `bdrv_invalidate_cache'
block.o: In function `bdrv_close':
---
block/vhdx-log.o: In function `vhdx_log_write_and_flush':
/tmp/qemu-test/src/block/vhdx-log.c:1049: undefined reference to `bdrv_flush'
/tmp/qemu-test/src/block/vhdx-log.c:1061: undefined reference to `bdrv_flush'
collect2: error: ld returned 1 exit status
make: *** [qemu-io] Error 1
  GEN     x86_64-softmmu/config-target.h
  GEN     x86_64-softmmu/hmp-commands.h
  GEN     x86_64-softmmu/hmp-commands-info.h
---
../block/vhdx-log.o: In function `vhdx_log_write_and_flush':
/tmp/qemu-test/src/block/vhdx-log.c:1049: undefined reference to `bdrv_flush'
/tmp/qemu-test/src/block/vhdx-log.c:1061: undefined reference to `bdrv_flush'
collect2: error: ld returned 1 exit status
make[1]: *** [qemu-system-x86_64] Error 1
make: *** [x86_64-softmmu/all] Error 2
../blockdev.o: In function `external_snapshot_prepare':
/tmp/qemu-test/src/blockdev.c:1480: undefined reference to `bdrv_flush'
../block.o: In function `bdrv_invalidate_cache_all':
---
../block/vhdx-log.o: In function `vhdx_log_write_and_flush':
/tmp/qemu-test/src/block/vhdx-log.c:1049: undefined reference to `bdrv_flush'
/tmp/qemu-test/src/block/vhdx-log.c:1061: undefined reference to `bdrv_flush'
collect2: error: ld returned 1 exit status
make[1]: *** [qemu-system-aarch64] Error 1
make: *** [aarch64-softmmu/all] Error 2
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
    sys.exit(main())
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=4d6dd8af9e3d41618b3eefc6134b03c2', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-fojaep43/src/docker-src.2020-05-22-17.15.05.18871:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=4d6dd8af9e3d41618b3eefc6134b03c2
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-fojaep43/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    3m17.180s
user    0m8.686s


The full log is available at
http://patchew.org/logs/20200522161950.2839-1-vsementsov@virtuozzo.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v3 0/3] coroutines: generate wrapper code

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200522161950.2839-1-vsementsov@virtuozzo.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

/tmp/qemu-test/src/nbd/server.c:1504: undefined reference to `bdrv_invalidate_cache'
/usr/bin/ld: qemu-img.o: in function `collect_image_check':
/tmp/qemu-test/src/qemu-img.c:695: undefined reference to `bdrv_check'
clang-8: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [/tmp/qemu-test/src/rules.mak:124: qemu-io] Error 1
make: *** Waiting for unfinished jobs....
/usr/bin/ld:   CC      x86_64-softmmu/accel/stubs/hax-stub.o
block.o: in function `bdrv_reopen_prepare':
---
/usr/bin/ld: block/vhdx-log.o: in function `vhdx_log_write_and_flush':
/tmp/qemu-test/src/block/vhdx-log.c:1049: undefined reference to `bdrv_flush'
/usr/bin/ld: /tmp/qemu-test/src/block/vhdx-log.c:1061: undefined reference to `bdrv_flush'
clang-8: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [/tmp/qemu-test/src/rules.mak:124: qemu-storage-daemon] Error 1
/usr/bin/ld: block/parallels.o: in function `parallels_close':
/tmp/qemu-test/src/block/parallels.c:898: undefined reference to `bdrv_truncate'
/usr/bin/ld: block/parallels.o: in function `parallels_co_check':
---
/tmp/qemu-test/src/block/io.c:2584: undefined reference to `bdrv_rw_vmstate'
/usr/bin/ld: nbd/server.o: in function `nbd_export_new':
/tmp/qemu-test/src/nbd/server.c:1504: undefined reference to `bdrv_invalidate_cache'
clang-8: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [/tmp/qemu-test/src/rules.mak:124: qemu-nbd] Error 1
  CC      x86_64-softmmu/accel/tcg/tcg-runtime-gvec.o
  CC      x86_64-softmmu/accel/tcg/cpu-exec.o
clang-8: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [/tmp/qemu-test/src/rules.mak:124: qemu-img] Error 1
  CC      x86_64-softmmu/accel/tcg/cpu-exec-common.o
  CC      x86_64-softmmu/accel/tcg/translate-all.o
  CC      x86_64-softmmu/accel/tcg/translator.o
---
/tmp/qemu-test/src/block/io.c:2584: undefined reference to `bdrv_rw_vmstate'
/usr/bin/ld: ../nbd/server.o: in function `nbd_export_new':
/tmp/qemu-test/src/nbd/server.c:1504: undefined reference to `bdrv_invalidate_cache'
clang-8: error: linker command failed with exit code 1 (use -v to see invocation)
make[1]: *** [Makefile:208: qemu-system-x86_64] Error 1
make: *** [Makefile:533: x86_64-softmmu/all] Error 2
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
    sys.exit(main())
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=0ae44f45ff214aacb698f382c823422b', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-dwg_9cv4/src/docker-src.2020-05-22-17.19.04.31012:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=0ae44f45ff214aacb698f382c823422b
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-dwg_9cv4/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    4m51.712s
user    0m9.298s


The full log is available at
http://patchew.org/logs/20200522161950.2839-1-vsementsov@virtuozzo.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v3 0/3] coroutines: generate wrapper code

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200522161950.2839-1-vsementsov@virtuozzo.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

/tmp/qemu-test/src/block/io.c:1025: undefined reference to `bdrv_flush'
block/io.o: In function `bdrv_flush_all':
/tmp/qemu-test/src/block/io.c:2170: undefined reference to `bdrv_flush'
block/collect2: error: ld returned 1 exit status
io.o: In function `bdrv_block_status_above':
/tmp/qemu-test/src/block/io.c:2435: undefined reference to `bdrv_common_block_status_above'
/tmp/qemu-test/src/block/io.c:2435: undefined reference to `bdrv_common_block_status_above'
---
/tmp/qemu-test/src/block/io.c:2584: undefined reference to `bdrv_rw_vmstate'
nbd/server.o: In function `nbd_export_new':
/tmp/qemu-test/src/nbd/server.c:1504: undefined reference to `bdrv_invalidate_cache'
collect2: error: ld returned 1 exit status
make: *** [/tmp/qemu-test/src/rules.mak:124: qemu-io.exe] Error 1
make: *** Waiting for unfinished jobs....
make: *** [/tmp/qemu-test/src/rules.mak:124: qemu-img.exe] Error 1
  GEN     x86_64-softmmu/hmp-commands.h
  GEN     x86_64-softmmu/hmp-commands-info.h
  GEN     x86_64-softmmu/config-devices.h
---
/tmp/qemu-test/src/block/io.c:2584: undefined reference to `bdrv_rw_vmstate'
../nbd/server.o: In function `nbd_export_new':
/tmp/qemu-test/src/nbd/server.c:1504: undefined reference to `bdrv_invalidate_cache'
collect2: error: ld returned 1 exit status
make[1]: *** [Makefile:208: qemu-system-x86_64w.exe] Error 1
make: *** [Makefile:533: x86_64-softmmu/all] Error 2
  LINK    aarch64-softmmu/qemu-system-aarch64w.exe
../blockdev.o: In function `external_snapshot_prepare':
/tmp/qemu-test/src/blockdev.c:1480: undefined reference to `bdrv_flush'
---
/tmp/qemu-test/src/block/io.c:2584: undefined reference to `bdrv_rw_vmstate'
../nbd/server.o: In function `nbd_export_new':
/tmp/qemu-test/src/nbd/server.c:1504: undefined reference to `bdrv_invalidate_cache'
collect2: error: ld returned 1 exit status
make[1]: *** [Makefile:208: qemu-system-aarch64w.exe] Error 1
make: *** [Makefile:533: aarch64-softmmu/all] Error 2
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
    sys.exit(main())
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=5a1f0334f43e44cea93b4f1fa85d7a08', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-y0bd7no0/src/docker-src.2020-05-22-17.25.42.8055:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=5a1f0334f43e44cea93b4f1fa85d7a08
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-y0bd7no0/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    4m15.085s
user    0m8.269s


The full log is available at
http://patchew.org/logs/20200522161950.2839-1-vsementsov@virtuozzo.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v3 1/3] block/io: refactor coroutine wrappers

[Eric Blake]

On 5/22/20 11:19 AM, Vladimir Sementsov-Ogievskiy wrote:
> Most of coroutine wrappers already follow this notation:

s/of/of our/
s/notation/convention/

> 
> We have coroutine_fn bdrv_co_<something>(<normal argument list>), which
> is the core functions, and wrapper, which does polling loope is called
> bdrv_<something>(<same argument list>).

We have 'coroutine_fn bdrv_co_<something>(<normal argument list>)' as 
the core function, and a wrapper 'bdrv_<something>(<same argument 
list>)' which does a polling loop.

> 
> The only outsiders are bdrv_prwv_co and bdrv_common_block_status_above

s/are/are the/

> wrappers. Let's refactor the to behave as the others, it simplifies

s/the/them/

> further conversion of coroutine wrappers.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/io.c | 61 +++++++++++++++++++++++++++++-------------------------
>   1 file changed, 33 insertions(+), 28 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 121ce17a49..bd00a70b47 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -900,28 +900,32 @@ typedef struct RwCo {
>       BdrvRequestFlags flags;
>   } RwCo;
>   
> +static int coroutine_fn bdrv_co_prwv(BdrvChild *child, int64_t offset,
> +                                     QEMUIOVector *qiov, bool is_write,
> +                                     BdrvRequestFlags flags)
> +{
> +    if (is_write) {
> +        return bdrv_co_pwritev(child, offset, qiov->size, qiov, flags);
> +    } else {
> +        return bdrv_co_preadv(child, offset, qiov->size, qiov, flags);
> +    }
> +}
> +

If we're trying to avoid needless indirection, wouldn't it be simpler to 
quit trying to slam reads and writes through a single prwv function that 
then has to split back out, and instead make two separate coroutine 
wrappers, one for just reads, and the other for just writes, without 
having to mess with a 'bool is_write' parameter?

>   static void coroutine_fn bdrv_rw_co_entry(void *opaque)
>   {

That is, should we have bdrv_co_preadv_entry and bdrv_co_pwritev_entry 
instead of just one bdrv_rw_co_entry?

At any rate, the renames done here are mechanical enough that if we make 
further changes, it could be a separate commit.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v3 2/3] block: declare some coroutine functions in block/coroutines.h

[Eric Blake]

On 5/22/20 11:19 AM, Vladimir Sementsov-Ogievskiy wrote:
> We are going to keep coroutine-wrappers code (structure-packing
> parameters, BDRV_POLL wrapper functions) in a separate auto-generated
> files. So, we'll need a header with declaration of original _co_
> functions, for those which are static now. As well, we'll need
> declarations for wrapper functions. Do these declarations now, as a
> preparation step.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/coroutines.h | 43 +++++++++++++++++++++++++++++++++++++++++++
>   block.c            |  8 ++++----
>   block/io.c         | 34 +++++++++++++++++-----------------
>   3 files changed, 64 insertions(+), 21 deletions(-)
>   create mode 100644 block/coroutines.h
> 
> diff --git a/block/coroutines.h b/block/coroutines.h
> new file mode 100644
> index 0000000000..23ea6fd5b3
> --- /dev/null
> +++ b/block/coroutines.h
> @@ -0,0 +1,43 @@
> +#ifndef BLOCK_COROUTINES_INT_H
> +#define BLOCK_COROUTINES_INT_H

Should have a copyright header.

Otherwise makes sense.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v3 1/3] block/io: refactor coroutine wrappers

[Vladimir Sementsov-Ogievskiy]

23.05.2020 00:33, Eric Blake wrote:
> On 5/22/20 11:19 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Most of coroutine wrappers already follow this notation:
> 
> s/of/of our/
> s/notation/convention/
> 
>>
>> We have coroutine_fn bdrv_co_<something>(<normal argument list>), which
>> is the core functions, and wrapper, which does polling loope is called
>> bdrv_<something>(<same argument list>).
> 
> We have 'coroutine_fn bdrv_co_<something>(<normal argument list>)' as the core function, and a wrapper 'bdrv_<something>(<same argument list>)' which does a polling loop.
> 
>>
>> The only outsiders are bdrv_prwv_co and bdrv_common_block_status_above
> 
> s/are/are the/
> 
>> wrappers. Let's refactor the to behave as the others, it simplifies
> 
> s/the/them/
> 
>> further conversion of coroutine wrappers.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   block/io.c | 61 +++++++++++++++++++++++++++++-------------------------
>>   1 file changed, 33 insertions(+), 28 deletions(-)
>>
>> diff --git a/block/io.c b/block/io.c
>> index 121ce17a49..bd00a70b47 100644
>> --- a/block/io.c
>> +++ b/block/io.c
>> @@ -900,28 +900,32 @@ typedef struct RwCo {
>>       BdrvRequestFlags flags;
>>   } RwCo;
>> +static int coroutine_fn bdrv_co_prwv(BdrvChild *child, int64_t offset,
>> +                                     QEMUIOVector *qiov, bool is_write,
>> +                                     BdrvRequestFlags flags)
>> +{
>> +    if (is_write) {
>> +        return bdrv_co_pwritev(child, offset, qiov->size, qiov, flags);
>> +    } else {
>> +        return bdrv_co_preadv(child, offset, qiov->size, qiov, flags);
>> +    }
>> +}
>> +
> 
> If we're trying to avoid needless indirection, wouldn't it be simpler to quit trying to slam reads and writes through a single prwv function that then has to split back out, and instead make two separate coroutine wrappers, one for just reads, and the other for just writes, without having to mess with a 'bool is_write' parameter?

Yes, and it's simpler after the transformation than before. I even wanted to do it but forget.. Will do as a follow-up, or with next version.

> 
>>   static void coroutine_fn bdrv_rw_co_entry(void *opaque)
>>   {
> 
> That is, should we have bdrv_co_preadv_entry and bdrv_co_pwritev_entry instead of just one bdrv_rw_co_entry?
> 
> At any rate, the renames done here are mechanical enough that if we make further changes, it could be a separate commit.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 


-- 
Best regards,
Vladimir