modules: add metadata database

[PATCH v2 02/18] qapi: add ModuleInfo schema

Posted by Gerd Hoffmann 4 years ago

Add QAPI schema for the module info database.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qapi/meson.build      |  1 +
 qapi/modules.json     | 36 ++++++++++++++++++++++++++++++++++++
 qapi/qapi-schema.json |  1 +
 3 files changed, 38 insertions(+)
 create mode 100644 qapi/modules.json

diff --git a/qapi/meson.build b/qapi/meson.build
index 376f4ceafe74..596aa5d71168 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -36,6 +36,7 @@ qapi_all_modules = [
   'migration',
   'misc',
   'misc-target',
+  'modules',
   'net',
   'pragma',
   'qom',
diff --git a/qapi/modules.json b/qapi/modules.json
new file mode 100644
index 000000000000..5420977d8765
--- /dev/null
+++ b/qapi/modules.json
@@ -0,0 +1,36 @@
+# -*- Mode: Python -*-
+# vim: filetype=python
+
+##
+# @ModuleInfo:
+#
+# qemu module metadata
+#
+# @name: module name
+#
+# @objs: list of qom objects implemented by the module.
+#
+# @deps: list of other modules this module depends on.
+#
+# @arch: module architecture.
+#
+# @opts: qemu opts implemented by module.
+#
+# Since: 6.1
+##
+{ 'struct': 'ModuleInfo',
+  'data': { 'name'  : 'str',
+            '*objs' : ['str'],
+            '*deps' : ['str'],
+            '*arch' : 'str',
+            '*opts' : 'str'}}
+
+##
+# @Modules:
+#
+# qemu module list
+#
+# Since: 6.1
+##
+{ 'struct': 'Modules',
+  'data': { 'list' : ['ModuleInfo']}}
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 4912b9744e69..5baa511c2ff5 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -93,3 +93,4 @@
 { 'include': 'audio.json' }
 { 'include': 'acpi.json' }
 { 'include': 'pci.json' }
+{ 'include': 'modules.json' }
-- 
2.31.1

Re: [PATCH v2 02/18] qapi: add ModuleInfo schema

Posted by Markus Armbruster 4 years ago

Gerd Hoffmann <kraxel@redhat.com> writes:

> Add QAPI schema for the module info database.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qapi/meson.build      |  1 +
>  qapi/modules.json     | 36 ++++++++++++++++++++++++++++++++++++
>  qapi/qapi-schema.json |  1 +
>  3 files changed, 38 insertions(+)
>  create mode 100644 qapi/modules.json
>
> diff --git a/qapi/meson.build b/qapi/meson.build
> index 376f4ceafe74..596aa5d71168 100644
> --- a/qapi/meson.build
> +++ b/qapi/meson.build
> @@ -36,6 +36,7 @@ qapi_all_modules = [
>    'migration',
>    'misc',
>    'misc-target',
> +  'modules',
>    'net',
>    'pragma',
>    'qom',
> diff --git a/qapi/modules.json b/qapi/modules.json
> new file mode 100644
> index 000000000000..5420977d8765
> --- /dev/null
> +++ b/qapi/modules.json
> @@ -0,0 +1,36 @@
> +# -*- Mode: Python -*-
> +# vim: filetype=python
> +

Missing a section heading like

   ##
   # = Loadable modules
   ##

Without it, the contents gets appended to whatever section precedes it
in qapi-schema.json, which is almost certainly not what you want.

> +##
> +# @ModuleInfo:
> +#
> +# qemu module metadata

It's spelled QEMU :)

Suggest "Loadable module meta-data".

> +#
> +# @name: module name
> +#
> +# @objs: list of qom objects implemented by the module.

s/qom objects/QOM types/

> +#
> +# @deps: list of other modules this module depends on.

Suggest to spell out that these are @name of other loadable modules.

> +#
> +# @arch: module architecture.

Semantics?

Should this be enum SysEmuTarget?

> +#
> +# @opts: qemu opts implemented by module.

Is this the name of a QemuOptsList?

Since this isn't a list, a module can only ever provide one
QemuOptsList.  Sure that's okay?

> +#
> +# Since: 6.1
> +##
> +{ 'struct': 'ModuleInfo',
> +  'data': { 'name'  : 'str',
> +            '*objs' : ['str'],
> +            '*deps' : ['str'],
> +            '*arch' : 'str',
> +            '*opts' : 'str'}}
> +
> +##
> +# @Modules:
> +#
> +# qemu module list
> +#
> +# Since: 6.1
> +##
> +{ 'struct': 'Modules',
> +  'data': { 'list' : ['ModuleInfo']}}

This defines only types, no QMP commands or events.  Why do you need the
types to be QAPI types?

> diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
> index 4912b9744e69..5baa511c2ff5 100644
> --- a/qapi/qapi-schema.json
> +++ b/qapi/qapi-schema.json
> @@ -93,3 +93,4 @@
>  { 'include': 'audio.json' }
>  { 'include': 'acpi.json' }
>  { 'include': 'pci.json' }
> +{ 'include': 'modules.json' }

Is this the place you want the section to be?  Remember, generated
documentation follows source order.

Re: [PATCH v2 02/18] qapi: add ModuleInfo schema

Posted by Gerd Hoffmann 4 years ago

  Hi,

> > +# @arch: module architecture.
> 
> Semantics?
> 
> Should this be enum SysEmuTarget?

Probably, will check ...

> > +# @opts: qemu opts implemented by module.
> 
> Is this the name of a QemuOptsList?
> 
> Since this isn't a list, a module can only ever provide one
> QemuOptsList.  Sure that's okay?

For the current two in-tree cases yes, and I don't expect this to change
in the future.  We could turn this into a list though to make it
future-proof.

> > +{ 'struct': 'Modules',
> > +  'data': { 'list' : ['ModuleInfo']}}
> 
> This defines only types, no QMP commands or events.  Why do you need the
> types to be QAPI types?

Want re-use the code to serialize/parse json from/to structs.
(see patches #3 + #13).

> > --- a/qapi/qapi-schema.json
> > +++ b/qapi/qapi-schema.json
> > @@ -93,3 +93,4 @@
> >  { 'include': 'audio.json' }
> >  { 'include': 'acpi.json' }
> >  { 'include': 'pci.json' }
> > +{ 'include': 'modules.json' }
> 
> Is this the place you want the section to be?  Remember, generated
> documentation follows source order.

Ah, *this* the ordering is important for.  I'll check, was just
appending to the end of the list ...

thanks,
  Gerd

Re: [PATCH v2 02/18] qapi: add ModuleInfo schema

Posted by Markus Armbruster 4 years ago

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
>> > +# @arch: module architecture.
>> 
>> Semantics?
>> 
>> Should this be enum SysEmuTarget?
>
> Probably, will check ...
>
>> > +# @opts: qemu opts implemented by module.
>> 
>> Is this the name of a QemuOptsList?
>> 
>> Since this isn't a list, a module can only ever provide one
>> QemuOptsList.  Sure that's okay?
>
> For the current two in-tree cases yes, and I don't expect this to change
> in the future.  We could turn this into a list though to make it
> future-proof.

If it's not much of a bother, then why not?

>> > +{ 'struct': 'Modules',
>> > +  'data': { 'list' : ['ModuleInfo']}}
>> 
>> This defines only types, no QMP commands or events.  Why do you need the
>> types to be QAPI types?
>
> Want re-use the code to serialize/parse json from/to structs.
> (see patches #3 + #13).

Okay, that's fair.

[...]