Re: [Qemu-devel] [PATCH v2 20/29] qapi/types qapi/visit: Generate built-in stuff into separate files

[Markus Armbruster]

Eric Blake  writes:

> On 02/11/2018 03:35 AM, Markus Armbruster wrote:
>> Linking code from multiple separate QAPI schemata into the same
>> program is possible, but involves some weirdness around built-in
>> types:
>>
>> * We generate code for built-in types into .c only with option
>>--builtins.  The user is responsible for generating code for exactly
>>one QAPI schema per program with --builtins.
>>
>> * We generate code for built-in types into .h regardless of
>>--builtins, but guarded by #ifndef QAPI_VISIT_BUILTIN.  Because all
>>copies of this code are exactly the same, including any combination
>>of these headers works.
>>
>> Replace this contraption by something more conventional: generate code
>> for built-in types into their very own files: qapi-builtin-types.c,
>> qapi-builtin-visit.c, qapi-builtin-types.h, qapi-builtin-visit.h, but
>> only with --builtins.  Obey --output-dir, but ignore --prefix for
>> them.
>>
>> Make qapi-types.h include qapi-builtin-types.h.  With multiple
>> schemata you now have multiple qapi-types.[ch], but only one
>> qapi-builtin-types.[ch].  Same for qapi-visit.[ch] and
>> qapi-builtin-visit.[ch].
>>
>> Bonus: if all you need is built-in stuff, you can include a much
>> smaller header.  To be exploited shortly.
>>
>> Signed-off-by: Markus Armbruster 
>> ---
>> @@ -2046,6 +2046,7 @@ class QAPIGenH(QAPIGenC):
>>   class QAPIGenDoc(QAPIGen):
>> +
>>   def _top(self, fname):
>>   return (QAPIGen._top(self, fname)
>>   + '@c AUTOMATICALLY GENERATED, DO NOT MODIFY\n\n')
>
> Does this hunk belong in an earlier patch?

Yes: PATCH 05.

> Otherwise,
> Reviewed-by: Eric Blake 

Thanks!

Re: [Qemu-devel] [PATCH v2 20/29] qapi/types qapi/visit: Generate built-in stuff into separate files

[Michael Roth]

Quoting Markus Armbruster (2018-02-11 03:35:58)
> Linking code from multiple separate QAPI schemata into the same
> program is possible, but involves some weirdness around built-in
> types:
> 
> * We generate code for built-in types into .c only with option
>   --builtins.  The user is responsible for generating code for exactly
>   one QAPI schema per program with --builtins.
> 
> * We generate code for built-in types into .h regardless of
>   --builtins, but guarded by #ifndef QAPI_VISIT_BUILTIN.  Because all
>   copies of this code are exactly the same, including any combination
>   of these headers works.
> 
> Replace this contraption by something more conventional: generate code
> for built-in types into their very own files: qapi-builtin-types.c,
> qapi-builtin-visit.c, qapi-builtin-types.h, qapi-builtin-visit.h, but
> only with --builtins.  Obey --output-dir, but ignore --prefix for
> them.
> 
> Make qapi-types.h include qapi-builtin-types.h.  With multiple
> schemata you now have multiple qapi-types.[ch], but only one
> qapi-builtin-types.[ch].  Same for qapi-visit.[ch] and
> qapi-builtin-visit.[ch].
> 
> Bonus: if all you need is built-in stuff, you can include a much
> smaller header.  To be exploited shortly.
> 
> Signed-off-by: Markus Armbruster 

Reviewed-by: Michael Roth 

> ---
>  .gitignore |  2 ++
>  Makefile   | 13 ++
>  Makefile.objs  |  2 ++
>  scripts/qapi/common.py | 61 ---
>  scripts/qapi/types.py  | 61 +++
>  scripts/qapi/visit.py  | 64 
> +-
>  6 files changed, 116 insertions(+), 87 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 7d783e6e66..9477a08b6b 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -29,6 +29,8 @@
>  /qga/qapi-generated
>  /qapi-generated
>  /qapi-gen-timestamp
> +/qapi-builtin-types.[ch]
> +/qapi-builtin-visit.[ch]
>  /qapi-types.[ch]
>  /qapi-visit.[ch]
>  /qapi-event.[ch]
> diff --git a/Makefile b/Makefile
> index 164a38578e..60ddc9c945 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -90,10 +90,13 @@ endif
>  include $(SRC_PATH)/rules.mak
> 
>  GENERATED_FILES = qemu-version.h config-host.h qemu-options.def
> -GENERATED_FILES += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
> -GENERATED_FILES += qmp-commands.c qapi-types.c qapi-visit.c qapi-event.c
> -GENERATED_FILES += qmp-introspect.h
> -GENERATED_FILES += qmp-introspect.c
> +GENERATED_FILES += qapi-builtin-types.h qapi-builtin-types.c
> +GENERATED_FILES += qapi-types.h qapi-types.c
> +GENERATED_FILES += qapi-builtin-visit.h qapi-builtin-visit.c
> +GENERATED_FILES += qapi-visit.h qapi-visit.c
> +GENERATED_FILES += qmp-commands.h qmp-commands.c
> +GENERATED_FILES += qapi-event.h qapi-event.c
> +GENERATED_FILES += qmp-introspect.c qmp-introspect.h
>  GENERATED_FILES += qapi-doc.texi
> 
>  GENERATED_FILES += trace/generated-tcg-tracers.h
> @@ -520,7 +523,9 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json 
> $(SRC_PATH)/qapi/common.json \
> $(SRC_PATH)/qapi/transaction.json \
> $(SRC_PATH)/qapi/ui.json
> 
> +qapi-builtin-types.c qapi-builtin-types.h \
>  qapi-types.c qapi-types.h \
> +qapi-builtin-visit.c qapi-builtin-visit.h \
>  qapi-visit.c qapi-visit.h \
>  qmp-commands.h qmp-commands.c \
>  qapi-event.c qapi-event.h \
> diff --git a/Makefile.objs b/Makefile.objs
> index d255aaf194..2813e984fd 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -2,6 +2,8 @@
>  # Common libraries for tools and emulators
>  stub-obj-y = stubs/ crypto/
>  util-obj-y = util/ qobject/ qapi/
> +util-obj-y += qapi-builtin-types.o
> +util-obj-y += qapi-builtin-visit.o
>  util-obj-y += qmp-introspect.o qapi-types.o qapi-visit.o qapi-event.o
> 
>  chardev-obj-y = chardev/
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 31d2f73e7e..de12f8469a 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -1531,11 +1531,10 @@ class QAPISchema(object):
> 
>  def _def_builtin_type(self, name, json_type, c_type):
>  self._def_entity(QAPISchemaBuiltinType(name, json_type, c_type))
> -# TODO As long as we have QAPI_TYPES_BUILTIN to share multiple
> -# qapi-types.h from a single .c, all arrays of builtins must be
> -# declared in the first file whether or not they are used.  Nicer
> -# would be to use lazy instantiation, while figuring out how to
> -# avoid compilation issues with multiple qapi-types.h.
> +# Instantiating only the arrays that are actually used would
> +# be nice, but we can't as long as their generated code
> +# (qapi-builtin-types.[ch]) may be shared by some other
> +# schema.
>  self._make_array_type(name, None)
> 
>  def _def_predefineds(self):
> @@ -1992,14 +1991,15 @@ class QAPIGen(object):
>  return ''
> 
>  

Re: [Qemu-devel] [PATCH v2 20/29] qapi/types qapi/visit: Generate built-in stuff into separate files

[Marc-Andre Lureau]

Hi

On Sun, Feb 11, 2018 at 10:35 AM, Markus Armbruster  wrote:
> Linking code from multiple separate QAPI schemata into the same
> program is possible, but involves some weirdness around built-in
> types:
>
> * We generate code for built-in types into .c only with option
>   --builtins.  The user is responsible for generating code for exactly
>   one QAPI schema per program with --builtins.
>
> * We generate code for built-in types into .h regardless of
>   --builtins, but guarded by #ifndef QAPI_VISIT_BUILTIN.  Because all
>   copies of this code are exactly the same, including any combination
>   of these headers works.
>
> Replace this contraption by something more conventional: generate code
> for built-in types into their very own files: qapi-builtin-types.c,
> qapi-builtin-visit.c, qapi-builtin-types.h, qapi-builtin-visit.h, but
> only with --builtins.  Obey --output-dir, but ignore --prefix for
> them.
>
> Make qapi-types.h include qapi-builtin-types.h.  With multiple
> schemata you now have multiple qapi-types.[ch], but only one
> qapi-builtin-types.[ch].  Same for qapi-visit.[ch] and
> qapi-builtin-visit.[ch].
>
> Bonus: if all you need is built-in stuff, you can include a much
> smaller header.  To be exploited shortly.
>
> Signed-off-by: Markus Armbruster 

Reviewed-by: Marc-André Lureau 

> ---
>  .gitignore |  2 ++
>  Makefile   | 13 ++
>  Makefile.objs  |  2 ++
>  scripts/qapi/common.py | 61 ---
>  scripts/qapi/types.py  | 61 +++
>  scripts/qapi/visit.py  | 64 
> +-
>  6 files changed, 116 insertions(+), 87 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 7d783e6e66..9477a08b6b 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -29,6 +29,8 @@
>  /qga/qapi-generated
>  /qapi-generated
>  /qapi-gen-timestamp
> +/qapi-builtin-types.[ch]
> +/qapi-builtin-visit.[ch]
>  /qapi-types.[ch]
>  /qapi-visit.[ch]
>  /qapi-event.[ch]
> diff --git a/Makefile b/Makefile
> index 164a38578e..60ddc9c945 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -90,10 +90,13 @@ endif
>  include $(SRC_PATH)/rules.mak
>
>  GENERATED_FILES = qemu-version.h config-host.h qemu-options.def
> -GENERATED_FILES += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
> -GENERATED_FILES += qmp-commands.c qapi-types.c qapi-visit.c qapi-event.c
> -GENERATED_FILES += qmp-introspect.h
> -GENERATED_FILES += qmp-introspect.c
> +GENERATED_FILES += qapi-builtin-types.h qapi-builtin-types.c
> +GENERATED_FILES += qapi-types.h qapi-types.c
> +GENERATED_FILES += qapi-builtin-visit.h qapi-builtin-visit.c
> +GENERATED_FILES += qapi-visit.h qapi-visit.c
> +GENERATED_FILES += qmp-commands.h qmp-commands.c
> +GENERATED_FILES += qapi-event.h qapi-event.c
> +GENERATED_FILES += qmp-introspect.c qmp-introspect.h
>  GENERATED_FILES += qapi-doc.texi
>
>  GENERATED_FILES += trace/generated-tcg-tracers.h
> @@ -520,7 +523,9 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json 
> $(SRC_PATH)/qapi/common.json \
> $(SRC_PATH)/qapi/transaction.json \
> $(SRC_PATH)/qapi/ui.json
>
> +qapi-builtin-types.c qapi-builtin-types.h \
>  qapi-types.c qapi-types.h \
> +qapi-builtin-visit.c qapi-builtin-visit.h \
>  qapi-visit.c qapi-visit.h \
>  qmp-commands.h qmp-commands.c \
>  qapi-event.c qapi-event.h \
> diff --git a/Makefile.objs b/Makefile.objs
> index d255aaf194..2813e984fd 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -2,6 +2,8 @@
>  # Common libraries for tools and emulators
>  stub-obj-y = stubs/ crypto/
>  util-obj-y = util/ qobject/ qapi/
> +util-obj-y += qapi-builtin-types.o
> +util-obj-y += qapi-builtin-visit.o
>  util-obj-y += qmp-introspect.o qapi-types.o qapi-visit.o qapi-event.o
>
>  chardev-obj-y = chardev/
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 31d2f73e7e..de12f8469a 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -1531,11 +1531,10 @@ class QAPISchema(object):
>
>  def _def_builtin_type(self, name, json_type, c_type):
>  self._def_entity(QAPISchemaBuiltinType(name, json_type, c_type))
> -# TODO As long as we have QAPI_TYPES_BUILTIN to share multiple
> -# qapi-types.h from a single .c, all arrays of builtins must be
> -# declared in the first file whether or not they are used.  Nicer
> -# would be to use lazy instantiation, while figuring out how to
> -# avoid compilation issues with multiple qapi-types.h.
> +# Instantiating only the arrays that are actually used would
> +# be nice, but we can't as long as their generated code
> +# (qapi-builtin-types.[ch]) may be shared by some other
> +# schema.
>  self._make_array_type(name, None)
>
>  def _def_predefineds(self):
> @@ -1992,14 +1991,15 @@ class QAPIGen(object):
> 

Re: [Qemu-devel] [PATCH v2 20/29] qapi/types qapi/visit: Generate built-in stuff into separate files

[Eric Blake]

On 02/11/2018 03:35 AM, Markus Armbruster wrote:

Linking code from multiple separate QAPI schemata into the same
program is possible, but involves some weirdness around built-in
types:

* We generate code for built-in types into .c only with option
   --builtins.  The user is responsible for generating code for exactly
   one QAPI schema per program with --builtins.

* We generate code for built-in types into .h regardless of
   --builtins, but guarded by #ifndef QAPI_VISIT_BUILTIN.  Because all
   copies of this code are exactly the same, including any combination
   of these headers works.

Replace this contraption by something more conventional: generate code
for built-in types into their very own files: qapi-builtin-types.c,
qapi-builtin-visit.c, qapi-builtin-types.h, qapi-builtin-visit.h, but
only with --builtins.  Obey --output-dir, but ignore --prefix for
them.

Make qapi-types.h include qapi-builtin-types.h.  With multiple
schemata you now have multiple qapi-types.[ch], but only one
qapi-builtin-types.[ch].  Same for qapi-visit.[ch] and
qapi-builtin-visit.[ch].

Bonus: if all you need is built-in stuff, you can include a much
smaller header.  To be exploited shortly.

Signed-off-by: Markus Armbruster 
---
@@ -2046,6 +2046,7 @@ class QAPIGenH(QAPIGenC):
  
  
  class QAPIGenDoc(QAPIGen):

+
  def _top(self, fname):
  return (QAPIGen._top(self, fname)
  + '@c AUTOMATICALLY GENERATED, DO NOT MODIFY\n\n')


Does this hunk belong in an earlier patch?

Otherwise,
Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH v2 20/29] qapi/types qapi/visit: Generate built-in stuff into separate files

[Markus Armbruster]

Linking code from multiple separate QAPI schemata into the same
program is possible, but involves some weirdness around built-in
types:

* We generate code for built-in types into .c only with option
  --builtins.  The user is responsible for generating code for exactly
  one QAPI schema per program with --builtins.

* We generate code for built-in types into .h regardless of
  --builtins, but guarded by #ifndef QAPI_VISIT_BUILTIN.  Because all
  copies of this code are exactly the same, including any combination
  of these headers works.

Replace this contraption by something more conventional: generate code
for built-in types into their very own files: qapi-builtin-types.c,
qapi-builtin-visit.c, qapi-builtin-types.h, qapi-builtin-visit.h, but
only with --builtins.  Obey --output-dir, but ignore --prefix for
them.

Make qapi-types.h include qapi-builtin-types.h.  With multiple
schemata you now have multiple qapi-types.[ch], but only one
qapi-builtin-types.[ch].  Same for qapi-visit.[ch] and
qapi-builtin-visit.[ch].

Bonus: if all you need is built-in stuff, you can include a much
smaller header.  To be exploited shortly.

Signed-off-by: Markus Armbruster 
---
 .gitignore |  2 ++
 Makefile   | 13 ++
 Makefile.objs  |  2 ++
 scripts/qapi/common.py | 61 ---
 scripts/qapi/types.py  | 61 +++
 scripts/qapi/visit.py  | 64 +-
 6 files changed, 116 insertions(+), 87 deletions(-)

diff --git a/.gitignore b/.gitignore
index 7d783e6e66..9477a08b6b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -29,6 +29,8 @@
 /qga/qapi-generated
 /qapi-generated
 /qapi-gen-timestamp
+/qapi-builtin-types.[ch]
+/qapi-builtin-visit.[ch]
 /qapi-types.[ch]
 /qapi-visit.[ch]
 /qapi-event.[ch]
diff --git a/Makefile b/Makefile
index 164a38578e..60ddc9c945 100644
--- a/Makefile
+++ b/Makefile
@@ -90,10 +90,13 @@ endif
 include $(SRC_PATH)/rules.mak
 
 GENERATED_FILES = qemu-version.h config-host.h qemu-options.def
-GENERATED_FILES += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
-GENERATED_FILES += qmp-commands.c qapi-types.c qapi-visit.c qapi-event.c
-GENERATED_FILES += qmp-introspect.h
-GENERATED_FILES += qmp-introspect.c
+GENERATED_FILES += qapi-builtin-types.h qapi-builtin-types.c
+GENERATED_FILES += qapi-types.h qapi-types.c
+GENERATED_FILES += qapi-builtin-visit.h qapi-builtin-visit.c
+GENERATED_FILES += qapi-visit.h qapi-visit.c
+GENERATED_FILES += qmp-commands.h qmp-commands.c
+GENERATED_FILES += qapi-event.h qapi-event.c
+GENERATED_FILES += qmp-introspect.c qmp-introspect.h
 GENERATED_FILES += qapi-doc.texi
 
 GENERATED_FILES += trace/generated-tcg-tracers.h
@@ -520,7 +523,9 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json 
$(SRC_PATH)/qapi/common.json \
$(SRC_PATH)/qapi/transaction.json \
$(SRC_PATH)/qapi/ui.json
 
+qapi-builtin-types.c qapi-builtin-types.h \
 qapi-types.c qapi-types.h \
+qapi-builtin-visit.c qapi-builtin-visit.h \
 qapi-visit.c qapi-visit.h \
 qmp-commands.h qmp-commands.c \
 qapi-event.c qapi-event.h \
diff --git a/Makefile.objs b/Makefile.objs
index d255aaf194..2813e984fd 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -2,6 +2,8 @@
 # Common libraries for tools and emulators
 stub-obj-y = stubs/ crypto/
 util-obj-y = util/ qobject/ qapi/
+util-obj-y += qapi-builtin-types.o
+util-obj-y += qapi-builtin-visit.o
 util-obj-y += qmp-introspect.o qapi-types.o qapi-visit.o qapi-event.o
 
 chardev-obj-y = chardev/
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 31d2f73e7e..de12f8469a 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1531,11 +1531,10 @@ class QAPISchema(object):
 
 def _def_builtin_type(self, name, json_type, c_type):
 self._def_entity(QAPISchemaBuiltinType(name, json_type, c_type))
-# TODO As long as we have QAPI_TYPES_BUILTIN to share multiple
-# qapi-types.h from a single .c, all arrays of builtins must be
-# declared in the first file whether or not they are used.  Nicer
-# would be to use lazy instantiation, while figuring out how to
-# avoid compilation issues with multiple qapi-types.h.
+# Instantiating only the arrays that are actually used would
+# be nice, but we can't as long as their generated code
+# (qapi-builtin-types.[ch]) may be shared by some other
+# schema.
 self._make_array_type(name, None)
 
 def _def_predefineds(self):
@@ -1992,14 +1991,15 @@ class QAPIGen(object):
 return ''
 
 def write(self, output_dir, fname):
-if output_dir:
+pathname = os.path.join(output_dir, fname)
+dir = os.path.dirname(pathname)
+if dir:
 try:
-os.makedirs(output_dir)
+os.makedirs(dir)
 except os.error as e:
 if e.errno !=