[Qemu-devel] [PATCH v3 0/3] qapi: Top-level type reference for command definitions

[Qemu-devel] [PATCH v3 0/3] qapi: Top-level type reference for command definitions

[Kevin Wolf]

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(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH v3 1/3] qapi.py: Avoid code duplication

[Kevin Wolf]

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 <kwolf@redhat.com>
---
 scripts/qapi.py | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 02ad668..3139994 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -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

[Qemu-devel] [PATCH v3 2/3] qapi.py: Allow top-level type reference for command definitions

[Kevin Wolf]

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 <kwolf@redhat.com>
---
 scripts/qapi.py | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 3139994..baf1321 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -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

[Qemu-devel] [PATCH v3 3/3] qapi-schema: Use BlockdevSnapshot type for blockdev-snapshot-sync

[Kevin Wolf]

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

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi-schema.json | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 5c32528..a90aeb1 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -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

Re: [Qemu-devel] [PATCH v3 1/3] qapi.py: Avoid code duplication

[Michael Roth]

On Mon, Jul 1, 2013 at 9:31 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> 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 <kwolf@redhat.com>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  scripts/qapi.py | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 02ad668..3139994 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -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
>

Re: [Qemu-devel] [PATCH v3 2/3] qapi.py: Allow top-level type reference for command definitions

[Michael Roth]

On Mon, Jul 1, 2013 at 9:31 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> 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 <kwolf@redhat.com>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  scripts/qapi.py | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 3139994..baf1321 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -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
>

Re: [Qemu-devel] [PATCH v3 3/3] qapi-schema: Use BlockdevSnapshot type for blockdev-snapshot-sync

[Michael Roth]

On Mon, Jul 1, 2013 at 9:31 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> We don't have to duplicate the definition any more now that we may refer
> to a type instead.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  qapi-schema.json | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 5c32528..a90aeb1 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -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
>

Re: [Qemu-devel] [PATCH v3 3/3] qapi-schema: Use BlockdevSnapshot type for blockdev-snapshot-sync

[Eric Blake]

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

On 07/01/2013 08:31 AM, Kevin Wolf wrote:
> We don't have to duplicate the definition any more now that we may refer
> to a type instead.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi-schema.json | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)

Since this patch is not in yet, but DriveBackup IS in via commit
3037f364, should you expand the scope of this patch to cover that
command at the same time?

-- 
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: 621 bytes --]

Re: [Qemu-devel] [PATCH v3 0/3] qapi: Top-level type reference for command definitions

[Luiz Capitulino]

On Mon,  1 Jul 2013 16:31:49 +0200
Kevin Wolf <kwolf@redhat.com> 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?

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

Re: [Qemu-devel] [PATCH v3 0/3] qapi: Top-level type reference for command definitions

[Kevin Wolf]

Am 08.07.2013 um 19:37 hat Luiz Capitulino geschrieben:
> On Mon,  1 Jul 2013 16:31:49 +0200
> Kevin Wolf <kwolf@redhat.com> 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?

Thanks, Luiz. I've sent the follow-up patch now.

Kevin