[Qemu-devel] [PATCH v3 37/50] qapi: add conditions to SPICE type/commands/events on the schema

Marc-André Lureau posted 50 patches 8 years, 5 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v3 37/50] qapi: add conditions to SPICE type/commands/events on the schema
Posted by Marc-André Lureau 8 years, 5 months ago
Add #if defined(CONFIG_SPICE) in generated code, and adjust the
qmp/hmp code accordingly.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qapi/char.json | 10 ++++++----
 qapi/ui.json   | 30 ++++++++++++++++++++----------
 monitor.c      |  3 ---
 qmp.c          | 16 ----------------
 4 files changed, 26 insertions(+), 33 deletions(-)

diff --git a/qapi/char.json b/qapi/char.json
index ae19dcd1ed..7fa1762ae5 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -318,7 +318,8 @@
 # Since: 1.5
 ##
 { 'struct': 'ChardevSpiceChannel', 'data': { 'type'  : 'str' },
-  'base': 'ChardevCommon' }
+  'base': 'ChardevCommon',
+  'if': 'defined(CONFIG_SPICE)' }
 
 ##
 # @ChardevSpicePort:
@@ -330,7 +331,8 @@
 # Since: 1.5
 ##
 { 'struct': 'ChardevSpicePort', 'data': { 'fqdn'  : 'str' },
-  'base': 'ChardevCommon' }
+  'base': 'ChardevCommon',
+  'if': 'defined(CONFIG_SPICE)' }
 
 ##
 # @ChardevVC:
@@ -384,8 +386,8 @@
                                        'testdev': 'ChardevCommon',
                                        'stdio'  : 'ChardevStdio',
                                        'console': 'ChardevCommon',
-                                       'spicevmc' : 'ChardevSpiceChannel',
-                                       'spiceport' : 'ChardevSpicePort',
+                                       'spicevmc' : { 'type': 'ChardevSpiceChannel', 'if': 'defined(CONFIG_SPICE)' },
+                                       'spiceport' : { 'type': 'ChardevSpicePort', 'if': 'defined(CONFIG_SPICE)' },
                                        'vc'     : 'ChardevVC',
                                        'ringbuf': 'ChardevRingbuf',
                                        # next one is just for compatibility
diff --git a/qapi/ui.json b/qapi/ui.json
index 4b573d214b..daa4168c14 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -110,7 +110,8 @@
 { 'struct': 'SpiceBasicInfo',
   'data': { 'host': 'str',
             'port': 'str',
-            'family': 'NetworkAddressFamily' } }
+            'family': 'NetworkAddressFamily' },
+  'if': 'defined(CONFIG_SPICE)' }
 
 ##
 # @SpiceServerInfo:
@@ -123,7 +124,8 @@
 ##
 { 'struct': 'SpiceServerInfo',
   'base': 'SpiceBasicInfo',
-  'data': { '*auth': 'str' } }
+  'data': { '*auth': 'str' },
+  'if': 'defined(CONFIG_SPICE)' }
 
 ##
 # @SpiceChannel:
@@ -148,7 +150,8 @@
 { 'struct': 'SpiceChannel',
   'base': 'SpiceBasicInfo',
   'data': {'connection-id': 'int', 'channel-type': 'int', 'channel-id': 'int',
-           'tls': 'bool'} }
+           'tls': 'bool'},
+  'if': 'defined(CONFIG_SPICE)' }
 
 ##
 # @SpiceQueryMouseMode:
@@ -167,7 +170,8 @@
 # Since: 1.1
 ##
 { 'enum': 'SpiceQueryMouseMode',
-  'data': [ 'client', 'server', 'unknown' ] }
+  'data': [ 'client', 'server', 'unknown' ],
+  'if': 'defined(CONFIG_SPICE)' }
 
 ##
 # @SpiceInfo:
@@ -204,7 +208,8 @@
 { 'struct': 'SpiceInfo',
   'data': {'enabled': 'bool', 'migrated': 'bool', '*host': 'str', '*port': 'int',
            '*tls-port': 'int', '*auth': 'str', '*compiled-version': 'str',
-           'mouse-mode': 'SpiceQueryMouseMode', '*channels': ['SpiceChannel']} }
+           'mouse-mode': 'SpiceQueryMouseMode', '*channels': ['SpiceChannel']},
+  'if': 'defined(CONFIG_SPICE)' }
 
 ##
 # @query-spice:
@@ -249,7 +254,8 @@
 #    }
 #
 ##
-{ 'command': 'query-spice', 'returns': 'SpiceInfo' }
+{ 'command': 'query-spice', 'returns': 'SpiceInfo',
+  'if': 'defined(CONFIG_SPICE)' }
 
 ##
 # @SPICE_CONNECTED:
@@ -274,7 +280,8 @@
 ##
 { 'event': 'SPICE_CONNECTED',
   'data': { 'server': 'SpiceBasicInfo',
-            'client': 'SpiceBasicInfo' } }
+            'client': 'SpiceBasicInfo' },
+  'if': 'defined(CONFIG_SPICE)' }
 
 ##
 # @SPICE_INITIALIZED:
@@ -302,7 +309,8 @@
 ##
 { 'event': 'SPICE_INITIALIZED',
   'data': { 'server': 'SpiceServerInfo',
-            'client': 'SpiceChannel' } }
+            'client': 'SpiceChannel' },
+  'if': 'defined(CONFIG_SPICE)' }
 
 ##
 # @SPICE_DISCONNECTED:
@@ -327,7 +335,8 @@
 ##
 { 'event': 'SPICE_DISCONNECTED',
   'data': { 'server': 'SpiceBasicInfo',
-            'client': 'SpiceBasicInfo' } }
+            'client': 'SpiceBasicInfo' },
+  'if': 'defined(CONFIG_SPICE)' }
 
 ##
 # @SPICE_MIGRATE_COMPLETED:
@@ -342,7 +351,8 @@
 #      "event": "SPICE_MIGRATE_COMPLETED" }
 #
 ##
-{ 'event': 'SPICE_MIGRATE_COMPLETED' }
+{ 'event': 'SPICE_MIGRATE_COMPLETED',
+  'if': 'defined(CONFIG_SPICE)' }
 
 ##
 # == VNC
diff --git a/monitor.c b/monitor.c
index 5685697f59..135a1e0821 100644
--- a/monitor.c
+++ b/monitor.c
@@ -970,9 +970,6 @@ static void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
  */
 static void qmp_unregister_commands_hack(void)
 {
-#ifndef CONFIG_SPICE
-    qmp_unregister_command(&qmp_commands, "query-spice");
-#endif
 #ifndef CONFIG_REPLICATION
     qmp_unregister_command(&qmp_commands, "xen-set-replication");
     qmp_unregister_command(&qmp_commands, "query-xen-replication-status");
diff --git a/qmp.c b/qmp.c
index 2c90dacb56..90816ba283 100644
--- a/qmp.c
+++ b/qmp.c
@@ -130,22 +130,6 @@ void qmp_cpu_add(int64_t id, Error **errp)
     }
 }
 
-#ifndef CONFIG_SPICE
-/*
- * qmp-commands.hx ensures that QMP command query-spice exists only
- * #ifdef CONFIG_SPICE.  Necessary for an accurate query-commands
- * result.  However, the QAPI schema is blissfully unaware of that,
- * and the QAPI code generator happily generates a dead
- * qmp_marshal_query_spice() that calls qmp_query_spice().  Provide it
- * one, or else linking fails.  FIXME Educate the QAPI schema on
- * CONFIG_SPICE.
- */
-SpiceInfo *qmp_query_spice(Error **errp)
-{
-    abort();
-};
-#endif
-
 void qmp_cont(Error **errp)
 {
     BlockBackend *blk;
-- 
2.14.1.146.gd35faa819


Re: [Qemu-devel] [PATCH v3 37/50] qapi: add conditions to SPICE type/commands/events on the schema
Posted by Markus Armbruster 8 years, 1 month ago
Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Add #if defined(CONFIG_SPICE) in generated code, and adjust the
> qmp/hmp code accordingly.

I'd like to see a description of how this affects QMP and HMP, like the
one in the previous patch.

Are there any occurrences of SPICE in the schema that aren't made
conditional by this patch?

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  qapi/char.json | 10 ++++++----
>  qapi/ui.json   | 30 ++++++++++++++++++++----------
>  monitor.c      |  3 ---
>  qmp.c          | 16 ----------------
>  4 files changed, 26 insertions(+), 33 deletions(-)
>
> diff --git a/qapi/char.json b/qapi/char.json
> index ae19dcd1ed..7fa1762ae5 100644
> --- a/qapi/char.json
> +++ b/qapi/char.json
> @@ -318,7 +318,8 @@
>  # Since: 1.5
>  ##
>  { 'struct': 'ChardevSpiceChannel', 'data': { 'type'  : 'str' },
> -  'base': 'ChardevCommon' }
> +  'base': 'ChardevCommon',
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @ChardevSpicePort:
> @@ -330,7 +331,8 @@
>  # Since: 1.5
>  ##
>  { 'struct': 'ChardevSpicePort', 'data': { 'fqdn'  : 'str' },
> -  'base': 'ChardevCommon' }
> +  'base': 'ChardevCommon',
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @ChardevVC:
> @@ -384,8 +386,8 @@
>                                         'testdev': 'ChardevCommon',
>                                         'stdio'  : 'ChardevStdio',
>                                         'console': 'ChardevCommon',
> -                                       'spicevmc' : 'ChardevSpiceChannel',
> -                                       'spiceport' : 'ChardevSpicePort',
> +                                       'spicevmc' : { 'type': 'ChardevSpiceChannel', 'if': 'defined(CONFIG_SPICE)' },
> +                                       'spiceport' : { 'type': 'ChardevSpicePort', 'if': 'defined(CONFIG_SPICE)' },
>                                         'vc'     : 'ChardevVC',
>                                         'ringbuf': 'ChardevRingbuf',
>                                         # next one is just for compatibility
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 4b573d214b..daa4168c14 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -110,7 +110,8 @@
>  { 'struct': 'SpiceBasicInfo',
>    'data': { 'host': 'str',
>              'port': 'str',
> -            'family': 'NetworkAddressFamily' } }
> +            'family': 'NetworkAddressFamily' },
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @SpiceServerInfo:
> @@ -123,7 +124,8 @@
>  ##
>  { 'struct': 'SpiceServerInfo',
>    'base': 'SpiceBasicInfo',
> -  'data': { '*auth': 'str' } }
> +  'data': { '*auth': 'str' },
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @SpiceChannel:
> @@ -148,7 +150,8 @@
>  { 'struct': 'SpiceChannel',
>    'base': 'SpiceBasicInfo',
>    'data': {'connection-id': 'int', 'channel-type': 'int', 'channel-id': 'int',
> -           'tls': 'bool'} }
> +           'tls': 'bool'},
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @SpiceQueryMouseMode:
> @@ -167,7 +170,8 @@
>  # Since: 1.1
>  ##
>  { 'enum': 'SpiceQueryMouseMode',
> -  'data': [ 'client', 'server', 'unknown' ] }
> +  'data': [ 'client', 'server', 'unknown' ],
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @SpiceInfo:
> @@ -204,7 +208,8 @@
>  { 'struct': 'SpiceInfo',
>    'data': {'enabled': 'bool', 'migrated': 'bool', '*host': 'str', '*port': 'int',
>             '*tls-port': 'int', '*auth': 'str', '*compiled-version': 'str',
> -           'mouse-mode': 'SpiceQueryMouseMode', '*channels': ['SpiceChannel']} }
> +           'mouse-mode': 'SpiceQueryMouseMode', '*channels': ['SpiceChannel']},
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @query-spice:
> @@ -249,7 +254,8 @@
>  #    }
>  #
>  ##
> -{ 'command': 'query-spice', 'returns': 'SpiceInfo' }
> +{ 'command': 'query-spice', 'returns': 'SpiceInfo',
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @SPICE_CONNECTED:
> @@ -274,7 +280,8 @@
>  ##
>  { 'event': 'SPICE_CONNECTED',
>    'data': { 'server': 'SpiceBasicInfo',
> -            'client': 'SpiceBasicInfo' } }
> +            'client': 'SpiceBasicInfo' },
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @SPICE_INITIALIZED:
> @@ -302,7 +309,8 @@
>  ##
>  { 'event': 'SPICE_INITIALIZED',
>    'data': { 'server': 'SpiceServerInfo',
> -            'client': 'SpiceChannel' } }
> +            'client': 'SpiceChannel' },
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @SPICE_DISCONNECTED:
> @@ -327,7 +335,8 @@
>  ##
>  { 'event': 'SPICE_DISCONNECTED',
>    'data': { 'server': 'SpiceBasicInfo',
> -            'client': 'SpiceBasicInfo' } }
> +            'client': 'SpiceBasicInfo' },
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @SPICE_MIGRATE_COMPLETED:
> @@ -342,7 +351,8 @@
>  #      "event": "SPICE_MIGRATE_COMPLETED" }
>  #
>  ##
> -{ 'event': 'SPICE_MIGRATE_COMPLETED' }
> +{ 'event': 'SPICE_MIGRATE_COMPLETED',
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # == VNC
> diff --git a/monitor.c b/monitor.c
> index 5685697f59..135a1e0821 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -970,9 +970,6 @@ static void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
>   */
>  static void qmp_unregister_commands_hack(void)
>  {
> -#ifndef CONFIG_SPICE
> -    qmp_unregister_command(&qmp_commands, "query-spice");
> -#endif
>  #ifndef CONFIG_REPLICATION
>      qmp_unregister_command(&qmp_commands, "xen-set-replication");
>      qmp_unregister_command(&qmp_commands, "query-xen-replication-status");
> diff --git a/qmp.c b/qmp.c
> index 2c90dacb56..90816ba283 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -130,22 +130,6 @@ void qmp_cpu_add(int64_t id, Error **errp)
>      }
>  }
>  
> -#ifndef CONFIG_SPICE
> -/*
> - * qmp-commands.hx ensures that QMP command query-spice exists only
> - * #ifdef CONFIG_SPICE.  Necessary for an accurate query-commands
> - * result.  However, the QAPI schema is blissfully unaware of that,
> - * and the QAPI code generator happily generates a dead
> - * qmp_marshal_query_spice() that calls qmp_query_spice().  Provide it
> - * one, or else linking fails.  FIXME Educate the QAPI schema on
> - * CONFIG_SPICE.
> - */
> -SpiceInfo *qmp_query_spice(Error **errp)
> -{
> -    abort();
> -};
> -#endif
> -
>  void qmp_cont(Error **errp)
>  {
>      BlockBackend *blk;

Hmm, shouldn't you compile out "info spice", like you did for "info vnc"
in the previous patch?