qapi: Top-level type reference for command definitions

The code that interprets the read JSON expression and appends types to the respective global variables was duplicated. We can avoid that by splitting off the part that reads from the file.

Signed-off-by Kevin Wolf <[email protected]>

---
 scripts/qapi.py | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

scripts/qapi.py +9 -10

index 02ad668..3139994 100644
@@ -78,10 +78,8 @@ def parse(tokens):
 def evaluate(string):
     return parse(map(lambda x: x, tokenize(string)))[0]
 
-def parse_schema(fp):
-    exprs = []
+def get_expr(fp):
     expr = ''
-    expr_eval = None
 
     for line in fp:
         if line.startswith('#') or line == '\n':

@@ -90,18 +88,20 @@ def parse_schema(fp):
         if line.startswith(' '):
             expr += line
         elif expr:
-            expr_eval = evaluate(expr)
-            if expr_eval.has_key('enum'):
-                add_enum(expr_eval['enum'])
-            elif expr_eval.has_key('union'):
-                add_enum('%sKind' % expr_eval['union'])
-            exprs.append(expr_eval)
+            yield expr
             expr = line
         else:
             expr += line
 
     if expr:
+        yield expr
+
+def parse_schema(fp):
+    exprs = []
+
+    for expr in get_expr(fp):
         expr_eval = evaluate(expr)
+
         if expr_eval.has_key('enum'):
             add_enum(expr_eval['enum'])
         elif expr_eval.has_key('union'):
--

1.8.1.4

If 'data' for a command definition isn't a dict, but a string, it is taken as a (struct) type name and the fields of this struct are directly used as parameters.

This is useful for transactionable commands that can use the same type definition for both the transaction action and the arguments of the standalone command.

Signed-off-by Kevin Wolf <[email protected]>

---
 scripts/qapi.py | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

scripts/qapi.py +19 -1

index 3139994..baf1321 100644
@@ -106,11 +106,18 @@ def parse_schema(fp):
             add_enum(expr_eval['enum'])
         elif expr_eval.has_key('union'):
             add_enum('%sKind' % expr_eval['union'])
+        elif expr_eval.has_key('type'):
+            add_struct(expr_eval)
         exprs.append(expr_eval)
 
     return exprs
 
 def parse_args(typeinfo):
+    if isinstance(typeinfo, basestring):
+        struct = find_struct(typeinfo)
+        assert struct != None
+        typeinfo = struct['data']
+
     for member in typeinfo:
         argname = member
         argentry = typeinfo[member]

@@ -180,6 +187,18 @@ def type_name(name):
     return name
 
 enum_types = []
+struct_types = []
+
+def add_struct(definition):
+    global struct_types
+    struct_types.append(definition)
+
+def find_struct(name):
+    global struct_types
+    for struct in struct_types:
+        if struct['type'] == name:
+            return struct
+    return None
 
 def add_enum(name):
     global enum_types
--

1.8.1.4

We don't have to duplicate the definition any more now that we may refer to a type instead.

Signed-off-by Kevin Wolf <[email protected]>

---
 qapi-schema.json | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

qapi-schema.json +2 -13

index 5c32528..a90aeb1 100644
@@ -1709,16 +1709,7 @@
 #
 # Generates a synchronous snapshot of a block device.
 #
-# @device:  the name of the device to generate the snapshot from.
-#
-# @snapshot-file: the target of the new image. If the file exists, or if it
-#                 is a device, the snapshot will be created in the existing
-#                 file/device. If does not exist, a new file will be created.
-#
-# @format: #optional the format of the snapshot image, default is 'qcow2'.
-#
-# @mode: #optional whether and how QEMU should create a new image, default is
-#        'absolute-paths'.
+# For the arguments, see the documentation of BlockdevSnapshot.
 #
 # Returns: nothing on success
 #          If @device is not a valid block device, DeviceNotFound

@@ -1726,8 +1717,7 @@
 # Since 0.14.0
 ##
 { 'command': 'blockdev-snapshot-sync',
-  'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
-            '*mode': 'NewImageMode'} }
+  'data': 'BlockdevSnapshot' }
 
 ##
 # @human-monitor-command:
--

1.8.1.4

We don't have to duplicate the definition any more now that we may refer to a type instead.

Signed-off-by Kevin Wolf <[email protected]>

---
 qapi-schema.json | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

v2:
- Updated documentation in patch 3

v3:
- Refactored differently (introduced get_expr()) as suggested by Michael

Kevin Wolf (3):
qapi.py: Avoid code duplication
qapi.py: Allow top-level type reference for command definitions
qapi-schema: Use BlockdevSnapshot type for blockdev-snapshot-sync

 qapi-schema.json | 14 ++------------
 scripts/qapi.py  | 37 ++++++++++++++++++++++++++++---------
 2 files changed, 30 insertions(+), 21 deletions(-)

Quoted 10 lines · On Mon, 1 Jul 2013 16:31:49 +0200 Kevin Wolf <[email protected]> wrote:

v2:
- Updated documentation in patch 3

v3:
- Refactored differently (introduced get_expr()) as suggested by Michael

Kevin Wolf (3):
qapi.py: Avoid code duplication
qapi.py: Allow top-level type reference for command definitions
qapi-schema: Use BlockdevSnapshot type for blockdev-snapshot-sync

Applied to the qmp branch, thanks.

Can you post an incremental patch to cover the drive-backup command as suggested by Eric?