[PATCH v2 00/11] Validate and test qapi examples

[PATCH v2 00/11] Validate and test qapi examples

[Victor Toso]

Hi,

v1: https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg00853.html

Changes:
- Fixed running the generator with tests (Daniel). Added a flag for the
  generator, set in tests/meson.build (Philippe).
- Moved the script to the end of the series, to avoid git bisect issues
  (Daniel)
- Added a proper assert() to the generator, making it fail if any
  documentation error was found instead of just printing to the stderr
  and carry on. (Daniel)
- Fixed the 3 other warnings the generator was showing, we are 100% free
  of warnings (in my machine)

Cheers,
Victor

Victor Toso (11):
  qapi: fix example of get-win32-socket command
  qapi: fix example of dumpdtb command
  qapi: fix example of cancel-vcpu-dirty-limit command
  qapi: fix example of set-vcpu-dirty-limit command
  qapi: fix example of calc-dirty-rate command
  qapi: fix example of NETDEV_STREAM_CONNECTED event
  qapi: fix example of query-rocker-of-dpa-flows command
  qapi: fix example of query-spice command
  qapi: fix example of query-blockstats command
  qapi: meson: add test flag to allow skip generators
  qapi: scripts: add a generator for qapi's examples

 qapi/block-core.json         |  32 +++---
 qapi/machine.json            |   2 +-
 qapi/migration.json          |   6 +-
 qapi/misc.json               |   2 +-
 qapi/net.json                |   6 +-
 qapi/rocker.json             |   3 +-
 qapi/ui.json                 |   3 +-
 scripts/qapi/dumpexamples.py | 204 +++++++++++++++++++++++++++++++++++
 scripts/qapi/main.py         |   9 +-
 tests/meson.build            |   2 +-
 10 files changed, 239 insertions(+), 30 deletions(-)
 create mode 100644 scripts/qapi/dumpexamples.py

-- 
2.41.0

[PATCH v2 01/11] qapi: fix example of get-win32-socket command

[Victor Toso]

Example output lacks double quotes. Fix it.

Fixes: 4cda177c60 "qmp: add 'get-win32-socket'"
Signed-off-by: Victor Toso <victortoso@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qapi/misc.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/misc.json b/qapi/misc.json
index cda2effa81..be302cadeb 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -290,7 +290,7 @@
 #
 # Example:
 #
-# -> { "execute": "get-win32-socket", "arguments": { "info": "abcd123..", fdname": "skclient" } }
+# -> { "execute": "get-win32-socket", "arguments": { "info": "abcd123..", "fdname": "skclient" } }
 # <- { "return": {} }
 ##
 { 'command': 'get-win32-socket', 'data': {'info': 'str', 'fdname': 'str'}, 'if': 'CONFIG_WIN32' }
-- 
2.41.0

[PATCH v2 02/11] qapi: fix example of dumpdtb command

[Victor Toso]

Example output has extra end curly bracket. Switch with comma.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qapi/machine.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/machine.json b/qapi/machine.json
index a08b6576ca..9eb76193e0 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1684,7 +1684,7 @@
 #
 # Example:
 #
-# -> { "execute": "dumpdtb" }
+# -> { "execute": "dumpdtb",
 #      "arguments": { "filename": "fdt.dtb" } }
 # <- { "return": {} }
 ##
-- 
2.41.0

[PATCH v2 03/11] qapi: fix example of cancel-vcpu-dirty-limit command

[Victor Toso]

Example output has extra end curly bracket. Remove it.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qapi/migration.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index 8843e74b59..9385b9f87c 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -2010,7 +2010,7 @@
 #
 # Example:
 #
-# -> {"execute": "cancel-vcpu-dirty-limit"},
+# -> {"execute": "cancel-vcpu-dirty-limit",
 #     "arguments": { "cpu-index": 1 } }
 # <- { "return": {} }
 ##
-- 
2.41.0

[PATCH v2 04/11] qapi: fix example of set-vcpu-dirty-limit command

[Victor Toso]

Example output has extra end curly bracket. Remove it.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qapi/migration.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index 9385b9f87c..2658cdbcbe 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1986,7 +1986,7 @@
 #
 # Example:
 #
-# -> {"execute": "set-vcpu-dirty-limit"}
+# -> {"execute": "set-vcpu-dirty-limit",
 #     "arguments": { "dirty-rate": 200,
 #                    "cpu-index": 1 } }
 # <- { "return": {} }
-- 
2.41.0

[PATCH v2 05/11] qapi: fix example of calc-dirty-rate command

[Victor Toso]

Example output has property name with single quotes. Fix it.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qapi/migration.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index 2658cdbcbe..45dac41f67 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1922,7 +1922,7 @@
 # Example:
 #
 # -> {"execute": "calc-dirty-rate", "arguments": {"calc-time": 1,
-#                                                 'sample-pages': 512} }
+#                                                 "sample-pages": 512} }
 # <- { "return": {} }
 ##
 { 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64',
-- 
2.41.0

[PATCH v2 06/11] qapi: fix example of NETDEV_STREAM_CONNECTED event

[Victor Toso]

Example output was using single quotes. Fix it.

Signed-off-by: Victor Toso <victortoso@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qapi/net.json | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/qapi/net.json b/qapi/net.json
index 313c8a606e..81988e499a 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -930,9 +930,9 @@
 #
 # Example:
 #
-# <- { 'event': 'NETDEV_STREAM_DISCONNECTED',
-#      'data': {'netdev-id': 'netdev0'},
-#      'timestamp': {'seconds': 1663330937, 'microseconds': 526695} }
+# <- { "event": "NETDEV_STREAM_DISCONNECTED",
+#      "data": {"netdev-id": "netdev0"},
+#      "timestamp": {"seconds": 1663330937, "microseconds": 526695} }
 ##
 { 'event': 'NETDEV_STREAM_DISCONNECTED',
   'data': { 'netdev-id': 'str' } }
-- 
2.41.0

[PATCH v2 07/11] qapi: fix example of query-rocker-of-dpa-flows command

[Victor Toso]

Example output has a comment embedded in the array. Remove it.
The end result is a list of size 1.

Signed-off-by: Victor Toso <victortoso@redhat.com>
---
 qapi/rocker.json | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/qapi/rocker.json b/qapi/rocker.json
index 31ce0b36f6..858e4f4a45 100644
--- a/qapi/rocker.json
+++ b/qapi/rocker.json
@@ -249,8 +249,7 @@
 #                   "cookie": 0,
 #                   "action": {"goto-tbl": 10},
 #                   "mask": {"in-pport": 4294901760}
-#                  },
-#                  {...more...},
+#                  }
 #    ]}
 ##
 { 'command': 'query-rocker-of-dpa-flows',
-- 
2.41.0

[PATCH v2 08/11] qapi: fix example of query-spice command

[Victor Toso]

Example output has a comment embedded in the array. Remove it.
The end result is a list of size 2.

Signed-off-by: Victor Toso <victortoso@redhat.com>
---
 qapi/ui.json | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/qapi/ui.json b/qapi/ui.json
index 006616aa77..6ed36c45ea 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -363,8 +363,7 @@
 #                "host": "127.0.0.1",
 #                "channel-id": 0,
 #                "tls": false
-#             },
-#             [ ... more channels follow ... ]
+#             }
 #          ]
 #       }
 #    }
-- 
2.41.0

[PATCH v2 09/11] qapi: fix example of query-blockstats command

[Victor Toso]

Example output has several missing commas. Add them.

Signed-off-by: Victor Toso <victortoso@redhat.com>
---
 qapi/block-core.json | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 2b1d493d6e..6a81103594 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1167,10 +1167,10 @@
 #                   "wr_bytes":9786368,
 #                   "wr_operations":751,
 #                   "rd_bytes":122567168,
-#                   "rd_operations":36772
-#                   "wr_total_times_ns":313253456
-#                   "rd_total_times_ns":3465673657
-#                   "flush_total_times_ns":49653
+#                   "rd_operations":36772,
+#                   "wr_total_times_ns":313253456,
+#                   "rd_total_times_ns":3465673657,
+#                   "flush_total_times_ns":49653,
 #                   "flush_operations":61,
 #                   "rd_merged":0,
 #                   "wr_merged":0,
@@ -1184,10 +1184,10 @@
 #                "wr_bytes":9786368,
 #                "wr_operations":692,
 #                "rd_bytes":122739200,
-#                "rd_operations":36604
+#                "rd_operations":36604,
 #                "flush_operations":51,
-#                "wr_total_times_ns":313253456
-#                "rd_total_times_ns":3465673657
+#                "wr_total_times_ns":313253456,
+#                "rd_total_times_ns":3465673657,
 #                "flush_total_times_ns":49653,
 #                "rd_merged":0,
 #                "wr_merged":0,
@@ -1204,10 +1204,10 @@
 #                "wr_bytes":0,
 #                "wr_operations":0,
 #                "rd_bytes":0,
-#                "rd_operations":0
+#                "rd_operations":0,
 #                "flush_operations":0,
-#                "wr_total_times_ns":0
-#                "rd_total_times_ns":0
+#                "wr_total_times_ns":0,
+#                "rd_total_times_ns":0,
 #                "flush_total_times_ns":0,
 #                "rd_merged":0,
 #                "wr_merged":0,
@@ -1223,10 +1223,10 @@
 #                "wr_bytes":0,
 #                "wr_operations":0,
 #                "rd_bytes":0,
-#                "rd_operations":0
+#                "rd_operations":0,
 #                "flush_operations":0,
-#                "wr_total_times_ns":0
-#                "rd_total_times_ns":0
+#                "wr_total_times_ns":0,
+#                "rd_total_times_ns":0,
 #                "flush_total_times_ns":0,
 #                "rd_merged":0,
 #                "wr_merged":0,
@@ -1242,10 +1242,10 @@
 #                "wr_bytes":0,
 #                "wr_operations":0,
 #                "rd_bytes":0,
-#                "rd_operations":0
+#                "rd_operations":0,
 #                "flush_operations":0,
-#                "wr_total_times_ns":0
-#                "rd_total_times_ns":0
+#                "wr_total_times_ns":0,
+#                "rd_total_times_ns":0,
 #                "flush_total_times_ns":0,
 #                "rd_merged":0,
 #                "wr_merged":0,
-- 
2.41.0

Re: [PATCH v2 07/11] qapi: fix example of query-rocker-of-dpa-flows command

[Daniel P. Berrangé]

On Mon, Sep 11, 2023 at 12:40:15PM +0200, Victor Toso wrote:
> Example output has a comment embedded in the array. Remove it.
> The end result is a list of size 1.
> 
> Signed-off-by: Victor Toso <victortoso@redhat.com>
> ---
>  qapi/rocker.json | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 08/11] qapi: fix example of query-spice command

[Daniel P. Berrangé]

On Mon, Sep 11, 2023 at 12:40:16PM +0200, Victor Toso wrote:
> Example output has a comment embedded in the array. Remove it.
> The end result is a list of size 2.
> 
> Signed-off-by: Victor Toso <victortoso@redhat.com>
> ---
>  qapi/ui.json | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 09/11] qapi: fix example of query-blockstats command

[Daniel P. Berrangé]

On Mon, Sep 11, 2023 at 12:40:17PM +0200, Victor Toso wrote:
> Example output has several missing commas. Add them.
> 
> Signed-off-by: Victor Toso <victortoso@redhat.com>
> ---
>  qapi/block-core.json | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH v2 10/11] qapi: meson: add test flag to allow skip generators

[Victor Toso]

The next patch adds a generator that also validates qapi
documentation. We don't want to execute it with a test schema.

Signed-off-by: Victor Toso <victortoso@redhat.com>
---
 scripts/qapi/main.py | 4 ++++
 tests/meson.build    | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
index 316736b6a2..7efdcc6b8d 100644
--- a/scripts/qapi/main.py
+++ b/scripts/qapi/main.py
@@ -33,6 +33,7 @@ def generate(schema_file: str,
              prefix: str,
              unmask: bool = False,
              builtins: bool = False,
+             test_schema: bool = False,
              gen_tracing: bool = False) -> None:
     """
     Generate C code for the given schema into the target directory.
@@ -75,6 +76,8 @@ def main() -> int:
     parser.add_argument('-u', '--unmask-non-abi-names', action='store_true',
                         dest='unmask',
                         help="expose non-ABI names in introspection")
+    parser.add_argument('-t', '--tests', action='store_true',
+                        help="flag generator that is running with a test schema")
 
     # Option --suppress-tracing exists so we can avoid solving build system
     # problems.  TODO Drop it when we no longer need it.
@@ -96,6 +99,7 @@ def main() -> int:
                  prefix=args.prefix,
                  unmask=args.unmask,
                  builtins=args.builtins,
+                 test_schema=args.tests,
                  gen_tracing=not args.suppress_tracing)
     except QAPIError as err:
         print(err, file=sys.stderr)
diff --git a/tests/meson.build b/tests/meson.build
index debaa4505e..ed0844a6a0 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -43,7 +43,7 @@ test_qapi_files = custom_target('Test QAPI files',
                                              'qapi-schema/include/sub-module.json',
                                              'qapi-schema/sub-sub-module.json'),
                                 command: [ qapi_gen, '-o', meson.current_build_dir(),
-                                           '-b', '-p', 'test-', '@INPUT0@',
+                                           '-t', '-b', '-p', 'test-', '@INPUT0@',
                                            '--suppress-tracing' ],
                                 depend_files: qapi_gen_depends)
 
-- 
2.41.0

[PATCH v2 11/11] qapi: scripts: add a generator for qapi's examples

[Victor Toso]

This generator has two goals:
 1. Mechanical validation of QAPI examples
 2. Generate the examples in a JSON format to be consumed for extra
    validation.

The generator iterates over every Example section, parsing both server
and client messages. The generator prints any inconsistency found, for
example:

 |  Error: Extra data: line 1 column 39 (char 38)
 |  Location: cancel-vcpu-dirty-limit at qapi/migration.json:2017
 |  Data: {"execute": "cancel-vcpu-dirty-limit"},
 |      "arguments": { "cpu-index": 1 } }

The generator will output other JSON file with all the examples in the
QAPI module that they came from. This can be used to validate the
introspection between QAPI/QMP to language bindings, for example:

 | { "examples": [
 |   {
 |     "id": "ksuxwzfayw",
 |     "client": [
 |     {
 |       "sequence-order": 1
 |       "message-type": "command",
 |       "message":
 |       { "arguments":
 |         { "device": "scratch", "size": 1073741824 },
 |         "execute": "block_resize"
 |       },
 |    } ],
 |    "server": [
 |    {
 |      "sequence-order": 2
 |      "message-type": "return",
 |      "message": { "return": {} },
 |    } ]
 |    }
 |  ] }

Note that the order matters, as read by the Example section and
translated into "sequence-order". A language binding project can then
consume this files to Marshal and Unmarshal, comparing if the results
are what is to be expected.

RFC discussion:
    https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg04641.html

Signed-off-by: Victor Toso <victortoso@redhat.com>
---
 scripts/qapi/dumpexamples.py | 204 +++++++++++++++++++++++++++++++++++
 scripts/qapi/main.py         |   5 +-
 2 files changed, 208 insertions(+), 1 deletion(-)
 create mode 100644 scripts/qapi/dumpexamples.py

diff --git a/scripts/qapi/dumpexamples.py b/scripts/qapi/dumpexamples.py
new file mode 100644
index 0000000000..5a1c275bc8
--- /dev/null
+++ b/scripts/qapi/dumpexamples.py
@@ -0,0 +1,204 @@
+"""
+Dump examples for Developers
+"""
+# Copyright (c) 2023 Red Hat Inc.
+#
+# Authors:
+#  Victor Toso <victortoso@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2.
+# See the COPYING file in the top-level directory.
+
+# Just for type hint on self
+from __future__ import annotations
+
+import os
+import json
+import random
+import string
+
+from typing import Dict, List, Optional
+
+from .schema import (
+    QAPISchema,
+    QAPISchemaType,
+    QAPISchemaVisitor,
+    QAPISchemaEnumMember,
+    QAPISchemaFeature,
+    QAPISchemaIfCond,
+    QAPISchemaObjectType,
+    QAPISchemaObjectTypeMember,
+    QAPISchemaVariants,
+)
+from .source import QAPISourceInfo
+
+
+def gen_examples(schema: QAPISchema,
+                 output_dir: str,
+                 prefix: str) -> None:
+    vis = QAPISchemaGenExamplesVisitor(prefix)
+    schema.visit(vis)
+    vis.write(output_dir)
+
+
+def get_id(random, size: int) -> str:
+    letters = string.ascii_lowercase
+    return ''.join(random.choice(letters) for i in range(size))
+
+
+def next_object(text, start, end, context) -> (Dict, bool):
+    # Start of json object
+    start = text.find("{", start)
+    end = text.rfind("}", start, end+1)
+
+    # try catch, pretty print issues
+    try:
+        ret = json.loads(text[start:end+1])
+    except Exception as e:
+        print("Error: {}\nLocation: {}\nData: {}\n".format(
+              str(e), context, text[start:end+1]))
+        return {}, True
+    else:
+        return ret, False
+
+
+def parse_text_to_dicts(text: str, context: str) -> (List[Dict], bool):
+    examples, clients, servers = [], [], []
+    failed = False
+
+    count = 1
+    c, s = text.find("->"), text.find("<-")
+    while c != -1 or s != -1:
+        if c == -1 or (s != -1 and s < c):
+            start, target = s, servers
+        else:
+            start, target = c, clients
+
+        # Find the client and server, if any
+        if c != -1:
+            c = text.find("->", start + 1)
+        if s != -1:
+            s = text.find("<-", start + 1)
+
+        # Find the limit of current's object.
+        # We first look for the next message, either client or server. If none
+        # is avaible, we set the end of the text as limit.
+        if c == -1 and s != -1:
+            end = s
+        elif c != -1 and s == -1:
+            end = c
+        elif c != -1 and s != -1:
+            end = (c < s) and c or s
+        else:
+            end = len(text) - 1
+
+        message, error = next_object(text, start, end, context)
+        if error:
+            failed = True
+
+        if len(message) > 0:
+            message_type = "return"
+            if "execute" in message:
+                message_type = "command"
+            elif "event" in message:
+                message_type = "event"
+
+            target.append({
+                "sequence-order": count,
+                "message-type": message_type,
+                "message": message
+            })
+            count += 1
+
+    examples.append({"client": clients, "server": servers})
+    return examples, failed
+
+
+def parse_examples_of(self: QAPISchemaGenExamplesVisitor,
+                      name: str):
+
+    assert(name in self.schema._entity_dict)
+    obj = self.schema._entity_dict[name]
+    assert((obj.doc is not None))
+    module_name = obj._module.name
+
+    # We initialize random with the name so that we get consistent example
+    # ids over different generations. The ids of a given example might
+    # change when adding/removing examples, but that's acceptable as the
+    # goal is just to grep $id to find what example failed at a given test
+    # with minimum chorn over regenerating.
+    random.seed(name, version=2)
+
+    for s in obj.doc.sections:
+        if s.name != "Example":
+            continue
+
+        if module_name not in self.target:
+            self.target[module_name] = []
+
+        context = f'''{name} at {obj.info.fname}:{obj.info.line}'''
+        examples, failed = parse_text_to_dicts(s.text, context)
+        if failed:
+            # To warn user that docs needs fixing
+            self.failed = True
+
+        for example in examples:
+            self.target[module_name].append({
+                    "id": get_id(random, 10),
+                    "client": example["client"],
+                    "server": example["server"]
+            })
+
+
+class QAPISchemaGenExamplesVisitor(QAPISchemaVisitor):
+
+    def __init__(self, prefix: str):
+        super().__init__()
+        self.target = {}
+        self.schema = None
+        self.failed = False
+
+    def visit_begin(self, schema):
+        self.schema = schema
+
+    def visit_end(self):
+        self.schema = None
+        assert not self.failed, "Should fix the docs"
+
+    def write(self: QAPISchemaGenExamplesVisitor,
+              output_dir: str) -> None:
+        for filename, content in self.target.items():
+            pathname = os.path.join(output_dir, "examples", filename)
+            odir = os.path.dirname(pathname)
+            os.makedirs(odir, exist_ok=True)
+            result = {"examples": content}
+
+            with open(pathname, "w") as outfile:
+                outfile.write(json.dumps(result, indent=2, sort_keys=True))
+
+    def visit_command(self: QAPISchemaGenExamplesVisitor,
+                      name: str,
+                      info: Optional[QAPISourceInfo],
+                      ifcond: QAPISchemaIfCond,
+                      features: List[QAPISchemaFeature],
+                      arg_type: Optional[QAPISchemaObjectType],
+                      ret_type: Optional[QAPISchemaType],
+                      gen: bool,
+                      success_response: bool,
+                      boxed: bool,
+                      allow_oob: bool,
+                      allow_preconfig: bool,
+                      coroutine: bool) -> None:
+
+        if gen:
+            parse_examples_of(self, name)
+
+    def visit_event(self: QAPISchemaGenExamplesVisitor,
+                    name: str,
+                    info: Optional[QAPISourceInfo],
+                    ifcond: QAPISchemaIfCond,
+                    features: List[QAPISchemaFeature],
+                    arg_type: Optional[QAPISchemaObjectType],
+                    boxed: bool):
+
+        parse_examples_of(self, name)
diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
index 7efdcc6b8d..9d71d78b55 100644
--- a/scripts/qapi/main.py
+++ b/scripts/qapi/main.py
@@ -13,6 +13,7 @@
 
 from .commands import gen_commands
 from .common import must_match
+from .dumpexamples import gen_examples
 from .error import QAPIError
 from .events import gen_events
 from .introspect import gen_introspect
@@ -54,7 +55,9 @@ def generate(schema_file: str,
     gen_commands(schema, output_dir, prefix, gen_tracing)
     gen_events(schema, output_dir, prefix)
     gen_introspect(schema, output_dir, prefix, unmask)
-
+    
+    if not test_schema:
+        gen_examples(schema, output_dir, prefix)
 
 def main() -> int:
     """
-- 
2.41.0

Re: [PATCH v2 07/11] qapi: fix example of query-rocker-of-dpa-flows command

[Markus Armbruster]

Victor Toso <victortoso@redhat.com> writes:

> Example output has a comment embedded in the array. Remove it.
> The end result is a list of size 1.
>
> Signed-off-by: Victor Toso <victortoso@redhat.com>
> ---
>  qapi/rocker.json | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/qapi/rocker.json b/qapi/rocker.json
> index 31ce0b36f6..858e4f4a45 100644
> --- a/qapi/rocker.json
> +++ b/qapi/rocker.json
> @@ -249,8 +249,7 @@
>  #                   "cookie": 0,
>  #                   "action": {"goto-tbl": 10},
>  #                   "mask": {"in-pport": 4294901760}
> -#                  },
> -#                  {...more...},
> +#                  }
>  #    ]}
>  ##
>  { 'command': 'query-rocker-of-dpa-flows',

The schema patches in this series fix typos, except for this patch and
the next one, which drop "more of the same omitted for brevity" text.  I
believe you drop the text because it doesn't parse as JSON.

Fine if the example still make sense afterwards.  Do they?

Shortening examples is a reasonable thing to do.  Perhaps we should
adopt a conventional way to do it, and teach the proposed generator to
cope with it.  What do you think?

Re: [PATCH v2 00/11] Validate and test qapi examples

[Markus Armbruster]

PATCH 01-06,09:
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH v2 07/11] qapi: fix example of query-rocker-of-dpa-flows command

[Victor Toso]

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

Hi,

On Thu, Sep 14, 2023 at 03:50:23PM +0200, Markus Armbruster wrote:
> Victor Toso <victortoso@redhat.com> writes:
> 
> > Example output has a comment embedded in the array. Remove it.
> > The end result is a list of size 1.
> >
> > Signed-off-by: Victor Toso <victortoso@redhat.com>
> > ---
> >  qapi/rocker.json | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/qapi/rocker.json b/qapi/rocker.json
> > index 31ce0b36f6..858e4f4a45 100644
> > --- a/qapi/rocker.json
> > +++ b/qapi/rocker.json
> > @@ -249,8 +249,7 @@
> >  #                   "cookie": 0,
> >  #                   "action": {"goto-tbl": 10},
> >  #                   "mask": {"in-pport": 4294901760}
> > -#                  },
> > -#                  {...more...},
> > +#                  }
> >  #    ]}
> >  ##
> >  { 'command': 'query-rocker-of-dpa-flows',
> 
> The schema patches in this series fix typos, except for this patch and
> the next one, which drop "more of the same omitted for brevity" text.  I
> believe you drop the text because it doesn't parse as JSON.

That's correct.

> Fine if the example still make sense afterwards.  Do they?

It depends what you mean by making sense. I did not setup rocker
to do this query and copied a real example. I think the real
example would have a list of size more than one.

So, if you think about real examples, it might not make sense. If
we talk about clarifying they API, I think it is reasonable.
 
> Shortening examples is a reasonable thing to do.  Perhaps we
> should adopt a conventional way to do it, and teach the
> proposed generator to cope with it.  What do you think?

Yep, I like it. In reality, I did not do this change in v1 but it
was suggested by Daniel that the end result of introducing this
generator would be to have it run without errors, so I shortened
as a simple way to fix it.

So, should we instead move forward with another convention for
comments inside the examples? This way we could still have a list
size 1 with this patch but it would be clear that the expectation
is a bigger list.

Cheers,
Victor

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 07/11] qapi: fix example of query-rocker-of-dpa-flows command

[Daniel P. Berrangé]

On Thu, Sep 14, 2023 at 04:01:55PM +0200, Victor Toso wrote:
> Hi,
> 
> On Thu, Sep 14, 2023 at 03:50:23PM +0200, Markus Armbruster wrote:
> > Victor Toso <victortoso@redhat.com> writes:
> > 
> > > Example output has a comment embedded in the array. Remove it.
> > > The end result is a list of size 1.
> > >
> > > Signed-off-by: Victor Toso <victortoso@redhat.com>
> > > ---
> > >  qapi/rocker.json | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/qapi/rocker.json b/qapi/rocker.json
> > > index 31ce0b36f6..858e4f4a45 100644
> > > --- a/qapi/rocker.json
> > > +++ b/qapi/rocker.json
> > > @@ -249,8 +249,7 @@
> > >  #                   "cookie": 0,
> > >  #                   "action": {"goto-tbl": 10},
> > >  #                   "mask": {"in-pport": 4294901760}
> > > -#                  },
> > > -#                  {...more...},
> > > +#                  }
> > >  #    ]}
> > >  ##
> > >  { 'command': 'query-rocker-of-dpa-flows',
> > 
> > The schema patches in this series fix typos, except for this patch and
> > the next one, which drop "more of the same omitted for brevity" text.  I
> > believe you drop the text because it doesn't parse as JSON.
> 
> That's correct.
> 
> > Fine if the example still make sense afterwards.  Do they?
> 
> It depends what you mean by making sense. I did not setup rocker
> to do this query and copied a real example. I think the real
> example would have a list of size more than one.
> 
> So, if you think about real examples, it might not make sense. If
> we talk about clarifying they API, I think it is reasonable.
>  
> > Shortening examples is a reasonable thing to do.  Perhaps we
> > should adopt a conventional way to do it, and teach the
> > proposed generator to cope with it.  What do you think?
> 
> Yep, I like it. In reality, I did not do this change in v1 but it
> was suggested by Daniel that the end result of introducing this
> generator would be to have it run without errors, so I shortened
> as a simple way to fix it.
> 
> So, should we instead move forward with another convention for
> comments inside the examples? This way we could still have a list
> size 1 with this patch but it would be clear that the expectation
> is a bigger list.

Personally I'd say if a field is a list, then the example should
contain 2 elements, just to make it a little more obvious at a
glance, as opposed to relying on spottnig the []. But that's not
a massively strong argument.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|