diff mbox series

[3/6] qapi: Generate command registration stuff into separate files

Message ID 20191120182551.23795-4-armbru@redhat.com (mailing list archive)
State New, archived
Headers show
Series qapi: Module fixes and cleanups | expand

Commit Message

Markus Armbruster Nov. 20, 2019, 6:25 p.m. UTC 
Having to include qapi-commands.h just for qmp_init_marshal() is
suboptimal.  Generate it into separate files.  This lets
monitor/misc.c, qga/main.c, and the generated qapi-commands-FOO.h
include less.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/devel/qapi-code-gen.txt | 19 ++++++++++++++++---
 Makefile                     |  4 +++-
 monitor/misc.c               |  7 ++++++-
 qga/main.c                   |  2 +-
 tests/test-qmp-cmds.c        |  1 +
 .gitignore                   |  1 +
 qapi/Makefile.objs           |  1 +
 qga/Makefile.objs            |  1 +
 scripts/qapi/commands.py     | 15 +++++++++++----
 tests/.gitignore             |  1 +
 tests/Makefile.include       |  5 ++++-
 11 files changed, 46 insertions(+), 11 deletions(-)

Comments

Eric Blake Nov. 20, 2019, 7:26 p.m. UTC | #1 
On 11/20/19 12:25 PM, Markus Armbruster wrote:
> Having to include qapi-commands.h just for qmp_init_marshal() is
> suboptimal.  Generate it into separate files.  This lets
> monitor/misc.c, qga/main.c, and the generated qapi-commands-FOO.h
> include less.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

> +++ b/docs/devel/qapi-code-gen.txt
> @@ -1493,6 +1493,10 @@ $(prefix)qapi-commands.c: Command marshal/dispatch functions for each
>   $(prefix)qapi-commands.h: Function prototypes for the QMP commands
>                             specified in the schema
>   
> +$(prefix)qapi-init-commands.h - Command initialization prototype
> +
> +$(prefix)qapi-init-commands.h - Command initialization code

Looks like you meant s/h/c/


> +    #endif /* EXAMPLE_QAPI_INIT_COMMANDS_H */
> +    $ cat qapi-generated/example-qapi-init-commands.
> +[Uninteresting stuff omitted...]

missing a 'c'

> +++ b/Makefile

>   
> -QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qapi-commands.h)
> +QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qapi-commands.h qga-qapi-init-commands.h)

Worth using \ for line-wrapping?

With those addressed,
Reviewed-by: Eric Blake <eblake@redhat.com>
Markus Armbruster Nov. 27, 2019, 4:12 p.m. UTC | #2 
Eric Blake <eblake@redhat.com> writes:

> On 11/20/19 12:25 PM, Markus Armbruster wrote:
>> Having to include qapi-commands.h just for qmp_init_marshal() is
>> suboptimal.  Generate it into separate files.  This lets
>> monitor/misc.c, qga/main.c, and the generated qapi-commands-FOO.h
>> include less.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>
>> +++ b/docs/devel/qapi-code-gen.txt
>> @@ -1493,6 +1493,10 @@ $(prefix)qapi-commands.c: Command marshal/dispatch functions for each
>>   $(prefix)qapi-commands.h: Function prototypes for the QMP commands
>>                             specified in the schema
>>   +$(prefix)qapi-init-commands.h - Command initialization prototype
>> +
>> +$(prefix)qapi-init-commands.h - Command initialization code
>
> Looks like you meant s/h/c/

I did.  Thanks!

>> +    #endif /* EXAMPLE_QAPI_INIT_COMMANDS_H */
>> +    $ cat qapi-generated/example-qapi-init-commands.
>> +[Uninteresting stuff omitted...]
>
> missing a 'c'

Yes.

>> +++ b/Makefile
>
>>   -QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h
>> qga-qapi-visit.h qga-qapi-commands.h)
>> +QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qapi-commands.h qga-qapi-init-commands.h)
>
> Worth using \ for line-wrapping?

Hmm, it's used only in the following line, could just as well inline
there and line-wrap that:

    $(qga-obj-y): $(QGALIB_GEN)

Hmm^2, why do we even have that line?  The compiler-generated .d should
take care of dependencies.  Dig, dig, ...

    commit 016c77ad62a8ad607dd4349d8cb8ad1365bab831
    Author: Michael Roth <mdroth@linux.vnet.ibm.com>
    Date:   Tue Jul 26 11:39:24 2011 -0500

        Makefile: add missing deps on $(GENERATED_HEADERS)

        This fixes a build issue with make -j6+ due to qapi-generated files
        being built before $(GENERATED_HEADERS) have been created.

        Tested-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
        Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
        Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Yes, but the same issue exists for all the other generated headers, and
we solve it differently: put them into $(generated-files-y), have
Makefile depend on them.

I'll clean this up on top.  No need to beautify lines now that will go
away then.

> With those addressed,
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!
diff mbox series

Patch

diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 45c93a43cc..3f37339d16 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -1493,6 +1493,10 @@  $(prefix)qapi-commands.c: Command marshal/dispatch functions for each
 $(prefix)qapi-commands.h: Function prototypes for the QMP commands
                           specified in the schema
 
+$(prefix)qapi-init-commands.h - Command initialization prototype
+
+$(prefix)qapi-init-commands.h - Command initialization code
+
 Example:
 
     $ cat qapi-generated/example-qapi-commands.h
@@ -1502,11 +1506,9 @@  Example:
     #define EXAMPLE_QAPI_COMMANDS_H
 
     #include "example-qapi-types.h"
-    #include "qapi/qmp/dispatch.h"
 
     UserDefOne *qmp_my_command(UserDefOneList *arg1, Error **errp);
     void qmp_marshal_my_command(QDict *args, QObject **ret, Error **errp);
-    void example_qmp_init_marshal(QmpCommandList *cmds);
 
     #endif /* EXAMPLE_QAPI_COMMANDS_H */
     $ cat qapi-generated/example-qapi-commands.c
@@ -1566,7 +1568,19 @@  Example:
         visit_end_struct(v, NULL);
         visit_free(v);
     }
+[Uninteresting stuff omitted...]
+    $ cat qapi-generated/example-qapi-init-commands.h
+[Uninteresting stuff omitted...]
+    #ifndef EXAMPLE_QAPI_INIT_COMMANDS_H
+    #define EXAMPLE_QAPI_INIT_COMMANDS_H
 
+    #include "qapi/qmp/dispatch.h"
+
+    void example_qmp_init_marshal(QmpCommandList *cmds);
+
+    #endif /* EXAMPLE_QAPI_INIT_COMMANDS_H */
+    $ cat qapi-generated/example-qapi-init-commands.
+[Uninteresting stuff omitted...]
     void example_qmp_init_marshal(QmpCommandList *cmds)
     {
         QTAILQ_INIT(cmds);
@@ -1574,7 +1588,6 @@  Example:
         qmp_register_command(cmds, "my-command",
                              qmp_marshal_my_command, QCO_NO_OPTIONS);
     }
-
 [Uninteresting stuff omitted...]
 
 For a modular QAPI schema (see section Include directives), code for
diff --git a/Makefile b/Makefile
index b437a346d7..8dad949483 100644
--- a/Makefile
+++ b/Makefile
@@ -117,6 +117,7 @@  GENERATED_QAPI_FILES += qapi/qapi-builtin-visit.h qapi/qapi-builtin-visit.c
 GENERATED_QAPI_FILES += qapi/qapi-visit.h qapi/qapi-visit.c
 GENERATED_QAPI_FILES += $(QAPI_MODULES:%=qapi/qapi-visit-%.h)
 GENERATED_QAPI_FILES += $(QAPI_MODULES:%=qapi/qapi-visit-%.c)
+GENERATED_QAPI_FILES += qapi/qapi-init-commands.h qapi/qapi-init-commands.c
 GENERATED_QAPI_FILES += qapi/qapi-commands.h qapi/qapi-commands.c
 GENERATED_QAPI_FILES += $(QAPI_MODULES:%=qapi/qapi-commands-%.h)
 GENERATED_QAPI_FILES += $(QAPI_MODULES:%=qapi/qapi-commands-%.c)
@@ -610,6 +611,7 @@  $(SRC_PATH)/scripts/qapi-gen.py
 qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h \
 qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h \
 qga/qapi-generated/qga-qapi-commands.h qga/qapi-generated/qga-qapi-commands.c \
+qga/qapi-generated/qga-qapi-init-commands.h qga/qapi-generated/qga-qapi-init-commands.c \
 qga/qapi-generated/qga-qapi-doc.texi: \
 qga/qapi-generated/qapi-gen-timestamp ;
 qga/qapi-generated/qapi-gen-timestamp: $(SRC_PATH)/qga/qapi-schema.json $(qapi-py)
@@ -628,7 +630,7 @@  qapi-gen-timestamp: $(qapi-modules) $(qapi-py)
 		"GEN","$(@:%-timestamp=%)")
 	@>$@
 
-QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qapi-commands.h)
+QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qapi-commands.h qga-qapi-init-commands.h)
 $(qga-obj-y): $(QGALIB_GEN)
 
 qemu-ga$(EXESUF): $(qga-obj-y) $(COMMON_LDADDS)
diff --git a/monitor/misc.c b/monitor/misc.c
index 3baa15f3bf..7c4b599342 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -66,8 +66,13 @@ 
 #include "qemu/option.h"
 #include "qemu/thread.h"
 #include "block/qapi.h"
-#include "qapi/qapi-commands.h"
+#include "qapi/qapi-commands-char.h"
+#include "qapi/qapi-commands-migration.h"
+#include "qapi/qapi-commands-misc.h"
+#include "qapi/qapi-commands-qom.h"
+#include "qapi/qapi-commands-trace.h"
 #include "qapi/qapi-emit-events.h"
+#include "qapi/qapi-init-commands.h"
 #include "qapi/error.h"
 #include "qapi/qmp-event.h"
 #include "qapi/qapi-introspect.h"
diff --git a/qga/main.c b/qga/main.c
index c35c2a2120..e5c39c189a 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -24,7 +24,7 @@ 
 #include "qapi/qmp/qjson.h"
 #include "qapi/qmp/qstring.h"
 #include "guest-agent-core.h"
-#include "qga-qapi-commands.h"
+#include "qga-qapi-init-commands.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/error.h"
 #include "channel.h"
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index 27b0afe55a..79507d9e54 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -7,6 +7,7 @@ 
 #include "tests/test-qapi-types.h"
 #include "tests/test-qapi-visit.h"
 #include "test-qapi-commands.h"
+#include "test-qapi-init-commands.h"
 
 static QmpCommandList qmp_commands;
 
diff --git a/.gitignore b/.gitignore
index 7de868d1ea..efad605e1a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -37,6 +37,7 @@ 
 /qapi/qapi-emit-events.[ch]
 /qapi/qapi-events-*.[ch]
 /qapi/qapi-events.[ch]
+/qapi/qapi-init-commands.[ch]
 /qapi/qapi-introspect.[ch]
 /qapi/qapi-types-*.[ch]
 /qapi/qapi-types.[ch]
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index dd3f5e6f94..a8f1f4c35e 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -30,3 +30,4 @@  obj-y += $(QAPI_TARGET_MODULES:%=qapi-events-%.o)
 obj-y += qapi-events.o
 obj-y += $(QAPI_TARGET_MODULES:%=qapi-commands-%.o)
 obj-y += qapi-commands.o
+obj-y += qapi-init-commands.o
diff --git a/qga/Makefile.objs b/qga/Makefile.objs
index 80e6bb3c2e..9c558ae51c 100644
--- a/qga/Makefile.objs
+++ b/qga/Makefile.objs
@@ -5,5 +5,6 @@  qga-obj-$(CONFIG_WIN32) += commands-win32.o channel-win32.o service-win32.o
 qga-obj-$(CONFIG_WIN32) += vss-win32.o
 qga-obj-y += qapi-generated/qga-qapi-types.o qapi-generated/qga-qapi-visit.o
 qga-obj-y += qapi-generated/qga-qapi-commands.o
+qga-obj-y += qapi-generated/qga-qapi-init-commands.o
 
 qga-vss-dll-obj-$(CONFIG_QGA_VSS) += vss-win32/
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index ab98e504f3..47f4a18cfe 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -263,18 +263,25 @@  class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
                              commands=commands, visit=visit))
         self._genh.add(mcgen('''
 #include "%(types)s.h"
-#include "qapi/qmp/dispatch.h"
 
 ''',
                              types=types))
 
     def visit_end(self):
-        (genc, genh) = self._module[self._main_module]
-        genh.add(mcgen('''
+        self._add_system_module('init', ' * QAPI Commands initialization')
+        self._genh.add(mcgen('''
+#include "qapi/qmp/dispatch.h"
+
 void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
 ''',
                        c_prefix=c_name(self._prefix, protect=False)))
-        genc.add(gen_registry(self._regy.get_content(), self._prefix))
+        self._genc.preamble_add(mcgen('''
+#include "qemu/osdep.h"
+#include "%(prefix)sqapi-commands.h"
+#include "%(prefix)sqapi-init-commands.h"
+''',
+                                      prefix=self._prefix))
+        self._genc.add(gen_registry(self._regy.get_content(), self._prefix))
 
     def visit_command(self, name, info, ifcond, arg_type, ret_type, gen,
                       success_response, boxed, allow_oob, allow_preconfig,
diff --git a/tests/.gitignore b/tests/.gitignore
index f9c0170881..7306866f21 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -12,6 +12,7 @@  test-*
 !test-*.c
 !docker/test-*
 test-qapi-commands.[ch]
+test-qapi-init-commands.[ch]
 include/test-qapi-commands-sub-module.[ch]
 test-qapi-commands-sub-sub-module.[ch]
 test-qapi-emit-events.[ch]
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 75b377d1a9..ce854ee556 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -501,6 +501,7 @@  generated-files-y += tests/test-qapi-visit.h
 generated-files-y += tests/include/test-qapi-visit-sub-module.h
 generated-files-y += tests/test-qapi-visit-sub-sub-module.h
 generated-files-y += tests/test-qapi-commands.h
+generated-files-y += tests/test-qapi-init-commands.h
 generated-files-y += tests/include/test-qapi-commands-sub-module.h
 generated-files-y += tests/test-qapi-commands-sub-sub-module.h
 generated-files-y += tests/test-qapi-emit-events.h
@@ -613,6 +614,8 @@  tests/test-qapi-commands-sub-sub-module.h \
 tests/test-qapi-commands-sub-sub-module.c \
 tests/test-qapi-emit-events.c tests/test-qapi-emit-events.h \
 tests/test-qapi-events.c tests/test-qapi-events.h \
+tests/test-qapi-init-commands.c \
+tests/test-qapi-init-commands.h \
 tests/include/test-qapi-events-sub-module.c \
 tests/include/test-qapi-events-sub-module.h \
 tests/test-qapi-events-sub-sub-module.c \
@@ -643,7 +646,7 @@  tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y) tests/t
 tests/test-qobject-output-visitor$(EXESUF): tests/test-qobject-output-visitor.o $(test-qapi-obj-y)
 tests/test-clone-visitor$(EXESUF): tests/test-clone-visitor.o $(test-qapi-obj-y)
 tests/test-qobject-input-visitor$(EXESUF): tests/test-qobject-input-visitor.o $(test-qapi-obj-y)
-tests/test-qmp-cmds$(EXESUF): tests/test-qmp-cmds.o tests/test-qapi-commands.o $(test-qapi-obj-y)
+tests/test-qmp-cmds$(EXESUF): tests/test-qmp-cmds.o tests/test-qapi-commands.o tests/test-qapi-init-commands.o $(test-qapi-obj-y)
 tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y)
 tests/test-opts-visitor$(EXESUF): tests/test-opts-visitor.o $(test-qapi-obj-y)