[Qemu-devel] [PATCH v3] Let the C generated bits of QAPI be generated only once when identical includes are done

[Qemu-devel] [PATCH v3] Let the C generated bits of QAPI be generated only once when identical includes are done

[Benoît Canet]

in V3:
        change commit title [Eric]
        s/spitted/emitted/
        change tests names [Eric]

Benoît Canet (1):
  qapi: skip redundant includes

 scripts/qapi.py                               | 14 +++++++++++---
 tests/Makefile                                |  3 ++-
 tests/qapi-schema/include-repetition-sub.json |  2 ++
 tests/qapi-schema/include-repetition.err      |  0
 tests/qapi-schema/include-repetition.exit     |  1 +
 tests/qapi-schema/include-repetition.json     |  3 +++
 tests/qapi-schema/include-repetition.out      |  3 +++
 7 files changed, 22 insertions(+), 4 deletions(-)
 create mode 100644 tests/qapi-schema/include-repetition-sub.json
 create mode 100644 tests/qapi-schema/include-repetition.err
 create mode 100644 tests/qapi-schema/include-repetition.exit
 create mode 100644 tests/qapi-schema/include-repetition.json
 create mode 100644 tests/qapi-schema/include-repetition.out

-- 
2.0.0.rc2

[Qemu-devel] [PATCH v3] qapi: skip redundant includes

[Benoît Canet]

The purpose of this change is to help create a json file containing
common definitions; each bit of generated C code must be emitted
only one time.

A second history global to all QAPISchema instances has been added
to detect when a file is included more than one time and skip these
includes.
It does not act as a stack and the changes made to it by the
__init__ function are propagated back to the caller so it's really
a global state.

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 scripts/qapi.py                               | 14 +++++++++++---
 tests/Makefile                                |  3 ++-
 tests/qapi-schema/include-repetition-sub.json |  2 ++
 tests/qapi-schema/include-repetition.err      |  0
 tests/qapi-schema/include-repetition.exit     |  1 +
 tests/qapi-schema/include-repetition.json     |  3 +++
 tests/qapi-schema/include-repetition.out      |  3 +++
 7 files changed, 22 insertions(+), 4 deletions(-)
 create mode 100644 tests/qapi-schema/include-repetition-sub.json
 create mode 100644 tests/qapi-schema/include-repetition.err
 create mode 100644 tests/qapi-schema/include-repetition.exit
 create mode 100644 tests/qapi-schema/include-repetition.json
 create mode 100644 tests/qapi-schema/include-repetition.out

diff --git a/scripts/qapi.py b/scripts/qapi.py
index ec806aa..0265b40 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -73,13 +73,18 @@ class QAPIExprError(Exception):
 
 class QAPISchema:
 
-    def __init__(self, fp, input_relname=None, include_hist=[], parent_info=None):
+    def __init__(self, fp, input_relname=None, include_hist=[],
+                 previously_included=[], parent_info=None):
+        """ include_hist is a stack used to detect inclusion cycles
+            previously_included is a global state used to avoid multiple
+                                inclusions of the same file"""
         input_fname = os.path.abspath(fp.name)
         if input_relname is None:
             input_relname = fp.name
         self.input_dir = os.path.dirname(input_fname)
         self.input_file = input_relname
         self.include_hist = include_hist + [(input_relname, input_fname)]
+        previously_included.append(input_fname)
         self.parent_info = parent_info
         self.src = fp.read()
         if self.src == '' or self.src[-1] != '\n':
@@ -106,13 +111,16 @@ class QAPISchema:
                        for elem in self.include_hist):
                     raise QAPIExprError(expr_info, "Inclusion loop for %s"
                                         % include)
+                # skip multiple include of the same file
+                if include_path in previously_included:
+                    continue
                 try:
                     fobj = open(include_path, 'r')
                 except IOError as e:
                     raise QAPIExprError(expr_info,
                                         '%s: %s' % (e.strerror, include))
-                exprs_include = QAPISchema(fobj, include,
-                                           self.include_hist, expr_info)
+                exprs_include = QAPISchema(fobj, include, self.include_hist,
+                                           previously_included, expr_info)
                 self.exprs.extend(exprs_include.exprs)
             else:
                 expr_elem = {'expr': expr,
diff --git a/tests/Makefile b/tests/Makefile
index a8d45be..24c3d05 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -182,7 +182,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
         flat-union-string-discriminator.json \
         include-simple.json include-relpath.json include-format-err.json \
         include-non-file.json include-no-file.json include-before-err.json \
-        include-nested-err.json include-self-cycle.json include-cycle.json)
+        include-nested-err.json include-self-cycle.json include-cycle.json \
+        include-repetition.json)
 
 GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
 
diff --git a/tests/qapi-schema/include-repetition-sub.json b/tests/qapi-schema/include-repetition-sub.json
new file mode 100644
index 0000000..6bfffdf
--- /dev/null
+++ b/tests/qapi-schema/include-repetition-sub.json
@@ -0,0 +1,2 @@
+{ 'include': 'comments.json' }
+{ 'include': 'comments.json' }
diff --git a/tests/qapi-schema/include-repetition.err b/tests/qapi-schema/include-repetition.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/include-repetition.exit b/tests/qapi-schema/include-repetition.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/include-repetition.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/include-repetition.json b/tests/qapi-schema/include-repetition.json
new file mode 100644
index 0000000..ec329dd
--- /dev/null
+++ b/tests/qapi-schema/include-repetition.json
@@ -0,0 +1,3 @@
+{ 'include': 'comments.json' }
+{ 'include': 'include-repetition-sub.json' }
+{ 'include': 'comments.json' }
diff --git a/tests/qapi-schema/include-repetition.out b/tests/qapi-schema/include-repetition.out
new file mode 100644
index 0000000..4ce3dcf
--- /dev/null
+++ b/tests/qapi-schema/include-repetition.out
@@ -0,0 +1,3 @@
+[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
+[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
+[]
-- 
2.0.0.rc2

Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes

[Markus Armbruster]

Benoît Canet <benoit.canet@irqsave.net> writes:

> The purpose of this change is to help create a json file containing
> common definitions; each bit of generated C code must be emitted
> only one time.
>
> A second history global to all QAPISchema instances has been added
> to detect when a file is included more than one time and skip these
> includes.
> It does not act as a stack and the changes made to it by the
> __init__ function are propagated back to the caller so it's really
> a global state.
>
> Signed-off-by: Benoit Canet <benoit@irqsave.net>

Committed a couple of weeks ago as 24fd848.

Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes

[Benoît Canet]

The Friday 27 Jun 2014 à 17:14:33 (+0200), Markus Armbruster wrote :
> Benoît Canet <benoit.canet@irqsave.net> writes:
> 
> > The purpose of this change is to help create a json file containing
> > common definitions; each bit of generated C code must be emitted
> > only one time.
> >
> > A second history global to all QAPISchema instances has been added
> > to detect when a file is included more than one time and skip these
> > includes.
> > It does not act as a stack and the changes made to it by the
> > __init__ function are propagated back to the caller so it's really
> > a global state.
> >
> > Signed-off-by: Benoit Canet <benoit@irqsave.net>
> 
> Committed a couple of weeks ago as 24fd848.

Yes I took a git send-email line in my bash history and forgot to remove a directory to send
while adding one another.

Best regards

Benoît

[Qemu-devel] [PATCH v1 0/2] Quorum arbitrary node replacement patches

[Benoît Canet]

in v1:
        no blank line before EOF [Kevin]
        Add call to bdrv_is_first_non_filter in check_to_replace_node to
           avoid backing blockers issues and data corruption by the user [Kevin, Benoît]
        Propagate error returned by check_to_replace_node [Kevin ]
        Remove uneeded line break [Kevin]
        remove uneeded empty line [Kevin]

Failed to run 041 tests since they are broken before this series.

Benoît Canet (2):
  block: Add replaces argument to drive-mirror
  qemu-iotests: Add TestRepairQuorum to 041 to test drive-mirror
    node-name mode.

 block.c                    |  25 ++++++
 block/mirror.c             |  60 +++++++++++---
 blockdev.c                 |  31 ++++++-
 hmp.c                      |   2 +-
 include/block/block.h      |   4 +
 include/block/block_int.h  |   3 +
 qapi/block-core.json       |   6 +-
 qmp-commands.hx            |   4 +-
 tests/qemu-iotests/041     | 196 ++++++++++++++++++++++++++++++++++++++++++++-
 tests/qemu-iotests/041.out |   4 +-
 10 files changed, 312 insertions(+), 23 deletions(-)

-- 
2.0.0

[Qemu-devel] [PATCH v1 1/2] block: Add replaces argument to drive-mirror

[Benoît Canet]

drive-mirror will bdrv_swap the new BDS named node-name with the one
pointed by replaces when the mirroring is finished.

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 block.c                   | 25 ++++++++++++++++++++
 block/mirror.c            | 60 +++++++++++++++++++++++++++++++++++++----------
 blockdev.c                | 31 +++++++++++++++++++++++-
 hmp.c                     |  2 +-
 include/block/block.h     |  4 ++++
 include/block/block_int.h |  3 +++
 qapi/block-core.json      |  6 ++++-
 qmp-commands.hx           |  4 +++-
 8 files changed, 118 insertions(+), 17 deletions(-)

diff --git a/block.c b/block.c
index 106238d..9ecadf0 100644
--- a/block.c
+++ b/block.c
@@ -5766,3 +5766,28 @@ bool bdrv_is_first_non_filter(BlockDriverState *candidate)
 
     return false;
 }
+
+BlockDriverState *check_to_replace_node(const char *node_name, Error **errp)
+{
+    BlockDriverState *to_replace_bs = bdrv_find_node(node_name);
+    if (!to_replace_bs) {
+        error_setg(errp, "Node name '%s' not found", node_name);
+        return NULL;
+    }
+
+    if (bdrv_op_is_blocked(to_replace_bs, BLOCK_OP_TYPE_REPLACE, errp)) {
+        return NULL;
+    }
+
+    /* We don't want arbitrary node of the BDS chain to be replaced only the top
+     * most non filter in order to prevent data corruption.
+     * Another benefit is that this tests exclude backing files which are
+     * blocked by the backing blockers.
+     */
+    if (!bdrv_is_first_non_filter(to_replace_bs)) {
+        error_setg(errp, "Only top most non filter can be replaced");
+        return NULL;
+    }
+
+    return to_replace_bs;
+}
diff --git a/block/mirror.c b/block/mirror.c
index 7c9f898..6c3ee70 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -32,6 +32,12 @@ typedef struct MirrorBlockJob {
     RateLimit limit;
     BlockDriverState *target;
     BlockDriverState *base;
+    /* The name of the graph node to replace */
+    char *replaces;
+    /* The BDS to replace */
+    BlockDriverState *to_replace;
+    /* Used to block operations on the drive-mirror-replace target */
+    Error *replace_blocker;
     bool is_none_mode;
     BlockdevOnError on_source_error, on_target_error;
     bool synced;
@@ -500,10 +506,14 @@ immediate_exit:
     bdrv_release_dirty_bitmap(bs, s->dirty_bitmap);
     bdrv_iostatus_disable(s->target);
     if (s->should_complete && ret == 0) {
-        if (bdrv_get_flags(s->target) != bdrv_get_flags(s->common.bs)) {
-            bdrv_reopen(s->target, bdrv_get_flags(s->common.bs), NULL);
+        BlockDriverState *to_replace = s->common.bs;
+        if (s->to_replace) {
+            to_replace = s->to_replace;
         }
-        bdrv_swap(s->target, s->common.bs);
+        if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) {
+            bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
+        }
+        bdrv_swap(s->target, to_replace);
         if (s->common.driver->job_type == BLOCK_JOB_TYPE_COMMIT) {
             /* drop the bs loop chain formed by the swap: break the loop then
              * trigger the unref from the top one */
@@ -512,6 +522,12 @@ immediate_exit:
             bdrv_unref(p);
         }
     }
+    if (s->to_replace) {
+        bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
+        error_free(s->replace_blocker);
+        bdrv_unref(s->to_replace);
+    }
+    g_free(s->replaces);
     bdrv_unref(s->target);
     block_job_completed(&s->common, ret);
 }
@@ -550,6 +566,20 @@ static void mirror_complete(BlockJob *job, Error **errp)
         return;
     }
 
+    /* check the target bs is not blocked and block all operations on it */
+    if (s->replaces) {
+        s->to_replace = check_to_replace_node(s->replaces, &local_err);
+        if (!s->to_replace) {
+            error_propagate(errp, local_err);
+            return;
+        }
+
+        error_setg(&s->replace_blocker,
+                   "block device is in use by block-job-complete");
+        bdrv_op_block_all(s->to_replace, s->replace_blocker);
+        bdrv_ref(s->to_replace);
+    }
+
     s->should_complete = true;
     block_job_resume(job);
 }
@@ -572,14 +602,15 @@ static const BlockJobDriver commit_active_job_driver = {
 };
 
 static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
-                            int64_t speed, int64_t granularity,
-                            int64_t buf_size,
-                            BlockdevOnError on_source_error,
-                            BlockdevOnError on_target_error,
-                            BlockDriverCompletionFunc *cb,
-                            void *opaque, Error **errp,
-                            const BlockJobDriver *driver,
-                            bool is_none_mode, BlockDriverState *base)
+                             const char *replaces,
+                             int64_t speed, int64_t granularity,
+                             int64_t buf_size,
+                             BlockdevOnError on_source_error,
+                             BlockdevOnError on_target_error,
+                             BlockDriverCompletionFunc *cb,
+                             void *opaque, Error **errp,
+                             const BlockJobDriver *driver,
+                             bool is_none_mode, BlockDriverState *base)
 {
     MirrorBlockJob *s;
 
@@ -610,6 +641,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
         return;
     }
 
+    s->replaces = g_strdup(replaces);
     s->on_source_error = on_source_error;
     s->on_target_error = on_target_error;
     s->target = target;
@@ -631,6 +663,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
 }
 
 void mirror_start(BlockDriverState *bs, BlockDriverState *target,
+                  const char *replaces,
                   int64_t speed, int64_t granularity, int64_t buf_size,
                   MirrorSyncMode mode, BlockdevOnError on_source_error,
                   BlockdevOnError on_target_error,
@@ -642,7 +675,8 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
 
     is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
     base = mode == MIRROR_SYNC_MODE_TOP ? bs->backing_hd : NULL;
-    mirror_start_job(bs, target, speed, granularity, buf_size,
+    mirror_start_job(bs, target, replaces,
+                     speed, granularity, buf_size,
                      on_source_error, on_target_error, cb, opaque, errp,
                      &mirror_job_driver, is_none_mode, base);
 }
@@ -690,7 +724,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
     }
 
     bdrv_ref(base);
-    mirror_start_job(bs, base, speed, 0, 0,
+    mirror_start_job(bs, base, NULL, speed, 0, 0,
                      on_error, on_error, cb, opaque, &local_err,
                      &commit_active_job_driver, false, base);
     if (local_err) {
diff --git a/blockdev.c b/blockdev.c
index 9433012..69b7c2a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2100,6 +2100,7 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
 void qmp_drive_mirror(const char *device, const char *target,
                       bool has_format, const char *format,
                       bool has_node_name, const char *node_name,
+                      bool has_replaces, const char *replaces,
                       enum MirrorSyncMode sync,
                       bool has_mode, enum NewImageMode mode,
                       bool has_speed, int64_t speed,
@@ -2187,6 +2188,29 @@ void qmp_drive_mirror(const char *device, const char *target,
         return;
     }
 
+    if (has_replaces) {
+        BlockDriverState *to_replace_bs;
+
+        if (!has_node_name) {
+            error_setg(errp, "a node-name must be provided when replacing a"
+                             " named node of the graph");
+            return;
+        }
+
+        to_replace_bs = check_to_replace_node(replaces, &local_err);
+
+        if (!to_replace_bs) {
+            error_propagate(errp, local_err);
+            return;
+        }
+
+        if (size != bdrv_getlength(to_replace_bs)) {
+            error_setg(errp, "cannot replace image with a mirror image of "
+                             "different size");
+            return;
+        }
+    }
+
     if ((sync == MIRROR_SYNC_MODE_FULL || !source)
         && mode != NEW_IMAGE_MODE_EXISTING)
     {
@@ -2231,7 +2255,12 @@ void qmp_drive_mirror(const char *device, const char *target,
         return;
     }
 
-    mirror_start(bs, target_bs, speed, granularity, buf_size, sync,
+    /* pass the node name to replace to mirror start since it's loose coupling
+     * and will allow to check whether the node still exist at mirror completion
+     */
+    mirror_start(bs, target_bs,
+                 has_replaces ? replaces : NULL,
+                 speed, granularity, buf_size, sync,
                  on_source_error, on_target_error,
                  block_job_cb, bs, &local_err);
     if (local_err != NULL) {
diff --git a/hmp.c b/hmp.c
index 73acc32..6429e6b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -933,7 +933,7 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
     }
 
     qmp_drive_mirror(device, filename, !!format, format,
-                     false, NULL,
+                     false, NULL, false, NULL,
                      full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
                      true, mode, false, 0, false, 0, false, 0,
                      false, 0, false, 0, &err);
diff --git a/include/block/block.h b/include/block/block.h
index d0baf4f..b53833d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -175,6 +175,7 @@ typedef enum BlockOpType {
     BLOCK_OP_TYPE_MIRROR,
     BLOCK_OP_TYPE_RESIZE,
     BLOCK_OP_TYPE_STREAM,
+    BLOCK_OP_TYPE_REPLACE,
     BLOCK_OP_TYPE_MAX,
 } BlockOpType;
 
@@ -314,6 +315,9 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs,
                                       BlockDriverState *candidate);
 bool bdrv_is_first_non_filter(BlockDriverState *candidate);
 
+/* check if a named node can be replaced when doing drive-mirror */
+BlockDriverState *check_to_replace_node(const char *node_name, Error **errp);
+
 /* async block I/O */
 typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector,
                                      int sector_num);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 135c5dc..53e77cf 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -489,6 +489,8 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
  * mirror_start:
  * @bs: Block device to operate on.
  * @target: Block device to write to.
+ * @replaces: Block graph node name to replace once the mirror is done. Can
+ *            only be used when full mirroring is selected.
  * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
  * @granularity: The chosen granularity for the dirty bitmap.
  * @buf_size: The amount of data that can be in flight at one time.
@@ -505,6 +507,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
  * @bs will be switched to read from @target.
  */
 void mirror_start(BlockDriverState *bs, BlockDriverState *target,
+                  const char *replaces,
                   int64_t speed, int64_t granularity, int64_t buf_size,
                   MirrorSyncMode mode, BlockdevOnError on_source_error,
                   BlockdevOnError on_target_error,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index a46cdbe..b362364 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -768,6 +768,10 @@
 # @node-name: #optional the new block driver state node name in the graph
 #             (Since 2.1)
 #
+# @replaces: #optional with sync=full graph node name to be replaced by the new
+#            image when a whole image copy is done. This can be used to repair
+#            broken Quorum files. (Since 2.1)
+#
 # @mode: #optional whether and how QEMU should create a new image, default is
 #        'absolute-paths'.
 #
@@ -800,7 +804,7 @@
 ##
 { 'command': 'drive-mirror',
   'data': { 'device': 'str', 'target': 'str', '*format': 'str',
-            '*node-name': 'str',
+            '*node-name': 'str', '*replaces': 'str',
             'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
             '*speed': 'int', '*granularity': 'uint32',
             '*buf-size': 'int', '*on-source-error': 'BlockdevOnError',
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 5254938..d342b8a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1293,7 +1293,7 @@ EQMP
     {
         .name       = "drive-mirror",
         .args_type  = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
-                      "node-name:s?,"
+                      "node-name:s?,replaces:s?,"
                       "on-source-error:s?,on-target-error:s?,"
                       "granularity:i?,buf-size:i?",
         .mhandler.cmd_new = qmp_marshal_input_drive_mirror,
@@ -1317,6 +1317,8 @@ Arguments:
 - "format": format of new image (json-string, optional)
 - "node-name": the name of the new block driver state in the node graph
                (json-string, optional)
+- "replaces": the block driver node name to replace when finished
+              (json-string, optional)
 - "mode": how an image file should be created into the target
   file/device (NewImageMode, optional, default 'absolute-paths')
 - "speed": maximum speed of the streaming job, in bytes per second
-- 
2.0.0

[Qemu-devel] [PATCH v1 2/2] qemu-iotests: Add TestRepairQuorum to 041 to test drive-mirror node-name mode.

[Benoît Canet]

The to-replace-node-name is designed to allow repairing a broken Quorum file.
This patch introduces a new class TestRepairQuorum testing that the feature
works.
Some further work will be done on QEMU to improve the robustness of the tests.

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 tests/qemu-iotests/041     | 196 ++++++++++++++++++++++++++++++++++++++++++++-
 tests/qemu-iotests/041.out |   4 +-
 2 files changed, 194 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index ef4f465..0815e19 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -28,6 +28,12 @@ target_backing_img = os.path.join(iotests.test_dir, 'target-backing.img')
 test_img = os.path.join(iotests.test_dir, 'test.img')
 target_img = os.path.join(iotests.test_dir, 'target.img')
 
+quorum_img1 = os.path.join(iotests.test_dir, 'quorum1.img')
+quorum_img2 = os.path.join(iotests.test_dir, 'quorum2.img')
+quorum_img3 = os.path.join(iotests.test_dir, 'quorum3.img')
+quorum_repair_img = os.path.join(iotests.test_dir, 'quorum_repair.img')
+quorum_snapshot_file = os.path.join(iotests.test_dir, 'quorum_snapshot.img')
+
 class ImageMirroringTestCase(iotests.QMPTestCase):
     '''Abstract base class for image mirroring test cases'''
 
@@ -42,8 +48,8 @@ class ImageMirroringTestCase(iotests.QMPTestCase):
                     ready = True
 
     def wait_ready_and_cancel(self, drive='drive0'):
-        self.wait_ready(drive)
-        event = self.cancel_and_wait()
+        self.wait_ready(drive=drive)
+        event = self.cancel_and_wait(drive=drive)
         self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED')
         self.assert_qmp(event, 'data/type', 'mirror')
         self.assert_qmp(event, 'data/offset', self.image_len)
@@ -52,12 +58,12 @@ class ImageMirroringTestCase(iotests.QMPTestCase):
     def complete_and_wait(self, drive='drive0', wait_ready=True):
         '''Complete a block job and wait for it to finish'''
         if wait_ready:
-            self.wait_ready()
+            self.wait_ready(drive=drive)
 
         result = self.vm.qmp('block-job-complete', device=drive)
         self.assert_qmp(result, 'return', {})
 
-        event = self.wait_until_completed()
+        event = self.wait_until_completed(drive=drive)
         self.assert_qmp(event, 'data/type', 'mirror')
 
 class TestSingleDrive(ImageMirroringTestCase):
@@ -723,5 +729,187 @@ class TestUnbackedSource(ImageMirroringTestCase):
         self.complete_and_wait()
         self.assert_no_active_block_jobs()
 
+class TestRepairQuorum(ImageMirroringTestCase):
+    """ This class test quorum file repair using drive-mirror.
+        It's mostly a fork of TestSingleDrive """
+    image_len = 1 * 1024 * 1024 # MB
+    IMAGES = [ quorum_img1, quorum_img2, quorum_img3 ]
+
+    def setUp(self):
+        self.vm = iotests.VM()
+
+        # Add each individual quorum images
+        for i in self.IMAGES:
+            qemu_img('create', '-f', iotests.imgfmt, i,
+                     str(TestSingleDrive.image_len))
+            # Assign a node name to each quorum image in order to manipulate
+            # them
+            opts = "node-name=img%i" % self.IMAGES.index(i)
+            self.vm = self.vm.add_drive(i, opts)
+
+        self.vm.launch()
+
+        #assemble the quorum block device from the individual files
+        args = { "options" : { "driver": "quorum", "id": "quorum0",
+                 "vote-threshold": 2, "children": [ "img0", "img1", "img2" ] } }
+        result = self.vm.qmp("blockdev-add", **args)
+        self.assert_qmp(result, 'return', {})
+
+
+    def tearDown(self):
+        self.vm.shutdown()
+        for i in self.IMAGES + [ quorum_repair_img ]:
+            # Do a try/except because the test may have deleted some images
+            try:
+                os.remove(i)
+            except OSError:
+                pass
+
+    def test_complete(self):
+        self.assert_no_active_block_jobs()
+
+        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+                             node_name="repair0",
+                             replaces="img1",
+                             target=quorum_repair_img, format=iotests.imgfmt)
+        self.assert_qmp(result, 'return', {})
+
+        self.complete_and_wait(drive="quorum0")
+        result = self.vm.qmp('query-named-block-nodes')
+        self.assert_qmp(result, 'return[0]/file', quorum_repair_img)
+        # TODO: a better test requiring some QEMU infrastructure will be added
+        #       to check that this file is really driven by quorum
+        self.vm.shutdown()
+        self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img),
+                        'target image does not match source after mirroring')
+
+    def test_cancel(self):
+        self.assert_no_active_block_jobs()
+
+        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+                             node_name="repair0",
+                             replaces="img1",
+                             target=quorum_repair_img, format=iotests.imgfmt)
+        self.assert_qmp(result, 'return', {})
+
+        self.cancel_and_wait(drive="quorum0", force=True)
+        # here we check that the last registered quorum file has not been
+        # swapped out and unref
+        result = self.vm.qmp('query-named-block-nodes')
+        self.assert_qmp(result, 'return[0]/file', quorum_img3)
+        self.vm.shutdown()
+
+    def test_cancel_after_ready(self):
+        self.assert_no_active_block_jobs()
+
+        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+                             node_name="repair0",
+                             replaces="img1",
+                             target=quorum_repair_img, format=iotests.imgfmt)
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_ready_and_cancel(drive="quorum0")
+        result = self.vm.qmp('query-named-block-nodes')
+        # here we check that the last registered quorum file has not been
+        # swapped out and unref
+        self.assert_qmp(result, 'return[0]/file', quorum_img3)
+        self.vm.shutdown()
+        self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img),
+                        'target image does not match source after mirroring')
+
+    def test_pause(self):
+        self.assert_no_active_block_jobs()
+
+        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+                             node_name="repair0",
+                             replaces="img1",
+                             target=quorum_repair_img, format=iotests.imgfmt)
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('block-job-pause', device='quorum0')
+        self.assert_qmp(result, 'return', {})
+
+        time.sleep(1)
+        result = self.vm.qmp('query-block-jobs')
+        offset = self.dictpath(result, 'return[0]/offset')
+
+        time.sleep(1)
+        result = self.vm.qmp('query-block-jobs')
+        self.assert_qmp(result, 'return[0]/offset', offset)
+
+        result = self.vm.qmp('block-job-resume', device='quorum0')
+        self.assert_qmp(result, 'return', {})
+
+        self.complete_and_wait(drive="quorum0")
+        self.vm.shutdown()
+        self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img),
+                        'target image does not match source after mirroring')
+
+    def test_medium_not_found(self):
+        result = self.vm.qmp('drive-mirror', device='ide1-cd0', sync='full',
+                             node_name='repair0',
+                             replaces='img1',
+                             target=quorum_repair_img, format=iotests.imgfmt)
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+    def test_image_not_found(self):
+        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+                             node_name='repair0',
+                             replaces='img1',
+                             mode='existing',
+                             target=quorum_repair_img, format=iotests.imgfmt)
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+    def test_device_not_found(self):
+        result = self.vm.qmp('drive-mirror', device='nonexistent', sync='full',
+                             node_name='repair0',
+                             replaces='img1',
+                             target=quorum_repair_img, format=iotests.imgfmt)
+        self.assert_qmp(result, 'error/class', 'DeviceNotFound')
+
+    def test_wrong_sync_mode(self):
+        result = self.vm.qmp('drive-mirror', device='quorum0',
+                             node_name='repair0',
+                             replaces='img1',
+                             target=quorum_repair_img, format=iotests.imgfmt)
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+    def test_no_node_name(self):
+        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+                             replaces='img1',
+                             target=quorum_repair_img, format=iotests.imgfmt)
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+    def test_unexistant_replaces(self):
+        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+                             node_name='repair0',
+                             replaces='img77',
+                             target=quorum_repair_img, format=iotests.imgfmt)
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+    def test_after_a_quorum_snapshot(self):
+        result = self.vm.qmp('blockdev-snapshot-sync', node_name='img1',
+                             snapshot_file=quorum_snapshot_file,
+                             snapshot_node_name="snap1");
+
+        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+                             node_name='repair0',
+                             replaces="img1",
+                             target=quorum_repair_img, format=iotests.imgfmt)
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+        result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+                             node_name='repair0',
+                             replaces="snap1",
+                             target=quorum_repair_img, format=iotests.imgfmt)
+        self.assert_qmp(result, 'return', {})
+
+        self.complete_and_wait(drive="quorum0")
+        result = self.vm.qmp('query-named-block-nodes')
+        self.assert_qmp(result, 'return[0]/file', quorum_repair_img)
+        # TODO: a better test requiring some QEMU infrastructure will be added
+        #       to check that this file is really driven by quorum
+        self.vm.shutdown()
+
 if __name__ == '__main__':
     iotests.main(supported_fmts=['qcow2', 'qed'])
diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
index cfa5c0d..73e375a 100644
--- a/tests/qemu-iotests/041.out
+++ b/tests/qemu-iotests/041.out
@@ -1,5 +1,5 @@
-...................................
+......................................
 ----------------------------------------------------------------------
-Ran 35 tests
+Ran 38 tests
 
 OK
-- 
2.0.0

Re: [Qemu-devel] [PATCH v3] Let the C generated bits of QAPI be generated only once when identical includes are done

[Benoît Canet]

The Friday 27 Jun 2014 à 15:41:50 (+0200), Benoît Canet wrote :
> in V3:
>         change commit title [Eric]
>         s/spitted/emitted/
>         change tests names [Eric]
> 
> Benoît Canet (1):
>   qapi: skip redundant includes
> 
>  scripts/qapi.py                               | 14 +++++++++++---
>  tests/Makefile                                |  3 ++-
>  tests/qapi-schema/include-repetition-sub.json |  2 ++
>  tests/qapi-schema/include-repetition.err      |  0
>  tests/qapi-schema/include-repetition.exit     |  1 +
>  tests/qapi-schema/include-repetition.json     |  3 +++
>  tests/qapi-schema/include-repetition.out      |  3 +++
>  7 files changed, 22 insertions(+), 4 deletions(-)
>  create mode 100644 tests/qapi-schema/include-repetition-sub.json
>  create mode 100644 tests/qapi-schema/include-repetition.err
>  create mode 100644 tests/qapi-schema/include-repetition.exit
>  create mode 100644 tests/qapi-schema/include-repetition.json
>  create mode 100644 tests/qapi-schema/include-repetition.out

Please forgot this one is send two separate directories at once.

> 
> -- 
> 2.0.0.rc2
> 

[Qemu-devel] [PATCH v3] Let the C generated bits of QAPI be generated only once when identical includes are done

[Benoît Canet]

in V3:
        change commit title [Eric]
        s/spitted/emitted/
        change tests names [Eric]

Benoît Canet (1):
  qapi: skip redundant includes

 scripts/qapi.py                               | 14 +++++++++++---
 tests/Makefile                                |  3 ++-
 tests/qapi-schema/include-repetition-sub.json |  2 ++
 tests/qapi-schema/include-repetition.err      |  0
 tests/qapi-schema/include-repetition.exit     |  1 +
 tests/qapi-schema/include-repetition.json     |  3 +++
 tests/qapi-schema/include-repetition.out      |  3 +++
 7 files changed, 22 insertions(+), 4 deletions(-)
 create mode 100644 tests/qapi-schema/include-repetition-sub.json
 create mode 100644 tests/qapi-schema/include-repetition.err
 create mode 100644 tests/qapi-schema/include-repetition.exit
 create mode 100644 tests/qapi-schema/include-repetition.json
 create mode 100644 tests/qapi-schema/include-repetition.out

-- 
2.0.0.rc2

[Qemu-devel] [PATCH v3] qapi: skip redundant includes

[Benoît Canet]

The purpose of this change is to help create a json file containing
common definitions; each bit of generated C code must be emitted
only one time.

A second history global to all QAPISchema instances has been added
to detect when a file is included more than one time and skip these
includes.
It does not act as a stack and the changes made to it by the
__init__ function are propagated back to the caller so it's really
a global state.

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 scripts/qapi.py                               | 14 +++++++++++---
 tests/Makefile                                |  3 ++-
 tests/qapi-schema/include-repetition-sub.json |  2 ++
 tests/qapi-schema/include-repetition.err      |  0
 tests/qapi-schema/include-repetition.exit     |  1 +
 tests/qapi-schema/include-repetition.json     |  3 +++
 tests/qapi-schema/include-repetition.out      |  3 +++
 7 files changed, 22 insertions(+), 4 deletions(-)
 create mode 100644 tests/qapi-schema/include-repetition-sub.json
 create mode 100644 tests/qapi-schema/include-repetition.err
 create mode 100644 tests/qapi-schema/include-repetition.exit
 create mode 100644 tests/qapi-schema/include-repetition.json
 create mode 100644 tests/qapi-schema/include-repetition.out

diff --git a/scripts/qapi.py b/scripts/qapi.py
index ec806aa..0265b40 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -73,13 +73,18 @@ class QAPIExprError(Exception):
 
 class QAPISchema:
 
-    def __init__(self, fp, input_relname=None, include_hist=[], parent_info=None):
+    def __init__(self, fp, input_relname=None, include_hist=[],
+                 previously_included=[], parent_info=None):
+        """ include_hist is a stack used to detect inclusion cycles
+            previously_included is a global state used to avoid multiple
+                                inclusions of the same file"""
         input_fname = os.path.abspath(fp.name)
         if input_relname is None:
             input_relname = fp.name
         self.input_dir = os.path.dirname(input_fname)
         self.input_file = input_relname
         self.include_hist = include_hist + [(input_relname, input_fname)]
+        previously_included.append(input_fname)
         self.parent_info = parent_info
         self.src = fp.read()
         if self.src == '' or self.src[-1] != '\n':
@@ -106,13 +111,16 @@ class QAPISchema:
                        for elem in self.include_hist):
                     raise QAPIExprError(expr_info, "Inclusion loop for %s"
                                         % include)
+                # skip multiple include of the same file
+                if include_path in previously_included:
+                    continue
                 try:
                     fobj = open(include_path, 'r')
                 except IOError as e:
                     raise QAPIExprError(expr_info,
                                         '%s: %s' % (e.strerror, include))
-                exprs_include = QAPISchema(fobj, include,
-                                           self.include_hist, expr_info)
+                exprs_include = QAPISchema(fobj, include, self.include_hist,
+                                           previously_included, expr_info)
                 self.exprs.extend(exprs_include.exprs)
             else:
                 expr_elem = {'expr': expr,
diff --git a/tests/Makefile b/tests/Makefile
index a8d45be..24c3d05 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -182,7 +182,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
         flat-union-string-discriminator.json \
         include-simple.json include-relpath.json include-format-err.json \
         include-non-file.json include-no-file.json include-before-err.json \
-        include-nested-err.json include-self-cycle.json include-cycle.json)
+        include-nested-err.json include-self-cycle.json include-cycle.json \
+        include-repetition.json)
 
 GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
 
diff --git a/tests/qapi-schema/include-repetition-sub.json b/tests/qapi-schema/include-repetition-sub.json
new file mode 100644
index 0000000..6bfffdf
--- /dev/null
+++ b/tests/qapi-schema/include-repetition-sub.json
@@ -0,0 +1,2 @@
+{ 'include': 'comments.json' }
+{ 'include': 'comments.json' }
diff --git a/tests/qapi-schema/include-repetition.err b/tests/qapi-schema/include-repetition.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/include-repetition.exit b/tests/qapi-schema/include-repetition.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/include-repetition.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/include-repetition.json b/tests/qapi-schema/include-repetition.json
new file mode 100644
index 0000000..ec329dd
--- /dev/null
+++ b/tests/qapi-schema/include-repetition.json
@@ -0,0 +1,3 @@
+{ 'include': 'comments.json' }
+{ 'include': 'include-repetition-sub.json' }
+{ 'include': 'comments.json' }
diff --git a/tests/qapi-schema/include-repetition.out b/tests/qapi-schema/include-repetition.out
new file mode 100644
index 0000000..4ce3dcf
--- /dev/null
+++ b/tests/qapi-schema/include-repetition.out
@@ -0,0 +1,3 @@
+[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
+[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
+[]
-- 
2.0.0.rc2

Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

On 05/14/2014 08:07 AM, Benoît Canet wrote:
> The purpose of this change is to help create a json file containing
> common definitions; each bit of generated C code must be emitted
> only one time.
> 
> A second history global to all QAPISchema instances has been added
> to detect when a file is included more than one time and skip these
> includes.
> It does not act as a stack and the changes made to it by the
> __init__ function are propagated back to the caller so it's really
> a global state.
> 
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
>  scripts/qapi.py                               | 14 +++++++++++---
>  tests/Makefile                                |  3 ++-
>  tests/qapi-schema/include-repetition-sub.json |  2 ++
>  tests/qapi-schema/include-repetition.err      |  0
>  tests/qapi-schema/include-repetition.exit     |  1 +
>  tests/qapi-schema/include-repetition.json     |  3 +++
>  tests/qapi-schema/include-repetition.out      |  3 +++
>  7 files changed, 22 insertions(+), 4 deletions(-)
>  create mode 100644 tests/qapi-schema/include-repetition-sub.json
>  create mode 100644 tests/qapi-schema/include-repetition.err
>  create mode 100644 tests/qapi-schema/include-repetition.exit
>  create mode 100644 tests/qapi-schema/include-repetition.json
>  create mode 100644 tests/qapi-schema/include-repetition.out
> 

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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes

[Luiz Capitulino]

On Wed, 14 May 2014 16:07:49 +0200
Benoît Canet <benoit.canet@irqsave.net> wrote:

> The purpose of this change is to help create a json file containing
> common definitions; each bit of generated C code must be emitted
> only one time.
> 
> A second history global to all QAPISchema instances has been added
> to detect when a file is included more than one time and skip these
> includes.
> It does not act as a stack and the changes made to it by the
> __init__ function are propagated back to the caller so it's really
> a global state.

Shouldn't this behavior be noted somewhere? Eg. in writing-qmp-commands.txt
or qapi-code-gen.txt?

> 
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
>  scripts/qapi.py                               | 14 +++++++++++---
>  tests/Makefile                                |  3 ++-
>  tests/qapi-schema/include-repetition-sub.json |  2 ++
>  tests/qapi-schema/include-repetition.err      |  0
>  tests/qapi-schema/include-repetition.exit     |  1 +
>  tests/qapi-schema/include-repetition.json     |  3 +++
>  tests/qapi-schema/include-repetition.out      |  3 +++
>  7 files changed, 22 insertions(+), 4 deletions(-)
>  create mode 100644 tests/qapi-schema/include-repetition-sub.json
>  create mode 100644 tests/qapi-schema/include-repetition.err
>  create mode 100644 tests/qapi-schema/include-repetition.exit
>  create mode 100644 tests/qapi-schema/include-repetition.json
>  create mode 100644 tests/qapi-schema/include-repetition.out
> 
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index ec806aa..0265b40 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -73,13 +73,18 @@ class QAPIExprError(Exception):
>  
>  class QAPISchema:
>  
> -    def __init__(self, fp, input_relname=None, include_hist=[], parent_info=None):
> +    def __init__(self, fp, input_relname=None, include_hist=[],
> +                 previously_included=[], parent_info=None):
> +        """ include_hist is a stack used to detect inclusion cycles
> +            previously_included is a global state used to avoid multiple
> +                                inclusions of the same file"""
>          input_fname = os.path.abspath(fp.name)
>          if input_relname is None:
>              input_relname = fp.name
>          self.input_dir = os.path.dirname(input_fname)
>          self.input_file = input_relname
>          self.include_hist = include_hist + [(input_relname, input_fname)]
> +        previously_included.append(input_fname)
>          self.parent_info = parent_info
>          self.src = fp.read()
>          if self.src == '' or self.src[-1] != '\n':
> @@ -106,13 +111,16 @@ class QAPISchema:
>                         for elem in self.include_hist):
>                      raise QAPIExprError(expr_info, "Inclusion loop for %s"
>                                          % include)
> +                # skip multiple include of the same file
> +                if include_path in previously_included:
> +                    continue
>                  try:
>                      fobj = open(include_path, 'r')
>                  except IOError as e:
>                      raise QAPIExprError(expr_info,
>                                          '%s: %s' % (e.strerror, include))
> -                exprs_include = QAPISchema(fobj, include,
> -                                           self.include_hist, expr_info)
> +                exprs_include = QAPISchema(fobj, include, self.include_hist,
> +                                           previously_included, expr_info)
>                  self.exprs.extend(exprs_include.exprs)
>              else:
>                  expr_elem = {'expr': expr,
> diff --git a/tests/Makefile b/tests/Makefile
> index a8d45be..24c3d05 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -182,7 +182,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
>          flat-union-string-discriminator.json \
>          include-simple.json include-relpath.json include-format-err.json \
>          include-non-file.json include-no-file.json include-before-err.json \
> -        include-nested-err.json include-self-cycle.json include-cycle.json)
> +        include-nested-err.json include-self-cycle.json include-cycle.json \
> +        include-repetition.json)
>  
>  GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
>  
> diff --git a/tests/qapi-schema/include-repetition-sub.json b/tests/qapi-schema/include-repetition-sub.json
> new file mode 100644
> index 0000000..6bfffdf
> --- /dev/null
> +++ b/tests/qapi-schema/include-repetition-sub.json
> @@ -0,0 +1,2 @@
> +{ 'include': 'comments.json' }
> +{ 'include': 'comments.json' }
> diff --git a/tests/qapi-schema/include-repetition.err b/tests/qapi-schema/include-repetition.err
> new file mode 100644
> index 0000000..e69de29
> diff --git a/tests/qapi-schema/include-repetition.exit b/tests/qapi-schema/include-repetition.exit
> new file mode 100644
> index 0000000..573541a
> --- /dev/null
> +++ b/tests/qapi-schema/include-repetition.exit
> @@ -0,0 +1 @@
> +0
> diff --git a/tests/qapi-schema/include-repetition.json b/tests/qapi-schema/include-repetition.json
> new file mode 100644
> index 0000000..ec329dd
> --- /dev/null
> +++ b/tests/qapi-schema/include-repetition.json
> @@ -0,0 +1,3 @@
> +{ 'include': 'comments.json' }
> +{ 'include': 'include-repetition-sub.json' }
> +{ 'include': 'comments.json' }
> diff --git a/tests/qapi-schema/include-repetition.out b/tests/qapi-schema/include-repetition.out
> new file mode 100644
> index 0000000..4ce3dcf
> --- /dev/null
> +++ b/tests/qapi-schema/include-repetition.out
> @@ -0,0 +1,3 @@
> +[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
> +[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
> +[]

Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]

On 05/15/2014 01:13 PM, Luiz Capitulino wrote:
> On Wed, 14 May 2014 16:07:49 +0200
> Benoît Canet <benoit.canet@irqsave.net> wrote:
> 
>> The purpose of this change is to help create a json file containing
>> common definitions; each bit of generated C code must be emitted
>> only one time.
>>
>> A second history global to all QAPISchema instances has been added
>> to detect when a file is included more than one time and skip these
>> includes.
>> It does not act as a stack and the changes made to it by the
>> __init__ function are propagated back to the caller so it's really
>> a global state.
> 
> Shouldn't this behavior be noted somewhere? Eg. in writing-qmp-commands.txt
> or qapi-code-gen.txt?

dosc/qapi-code-gen.txt already covers the {'include':'file'} syntax, so
yes, that should be updated to mention that multiple includes of the
same file are now safe.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes

[Benoît Canet]

The Thursday 15 May 2014 à 13:39:02 (-0600), Eric Blake wrote :
> On 05/15/2014 01:13 PM, Luiz Capitulino wrote:
> > On Wed, 14 May 2014 16:07:49 +0200
> > Benoît Canet <benoit.canet@irqsave.net> wrote:
> > 
> >> The purpose of this change is to help create a json file containing
> >> common definitions; each bit of generated C code must be emitted
> >> only one time.
> >>
> >> A second history global to all QAPISchema instances has been added
> >> to detect when a file is included more than one time and skip these
> >> includes.
> >> It does not act as a stack and the changes made to it by the
> >> __init__ function are propagated back to the caller so it's really
> >> a global state.
> > 
> > Shouldn't this behavior be noted somewhere? Eg. in writing-qmp-commands.txt
> > or qapi-code-gen.txt?
> 
> dosc/qapi-code-gen.txt already covers the {'include':'file'} syntax, so
> yes, that should be updated to mention that multiple includes of the
> same file are now safe.

ok, I will do this.

Best regards

Benoît
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>