[PATCH v6 0/4] docs/interop/firmware.json: scripts/qapi-gen.py compatibility

[PATCH v6 0/4] docs/interop/firmware.json: scripts/qapi-gen.py compatibility

[Thomas Weißschuh]

docs/interop/firmware.json is currently not usable with qapi-gen.py due
to various non-functional issues.
Fix those issue to provide compatibility.

Kashyap, I stuck with the original wording for image format docs,
as I personally prefer them a bit.

Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
Changes in v6:
- Style cleanups
- Improve the comment for qcow2
- Document the new enums as Since: 3.0 (7.1 for the loongarch64 arch)
- Link to v5: https://lore.kernel.org/r/20240718-qapi-firmware-json-v5-0-0dba12d7aaf5@linutronix.de

Changes in v5:
- Add Reviewed-by from Daniel
- Add patch to drop Example section
- Link to v4: https://lore.kernel.org/r/20240718-qapi-firmware-json-v4-0-449ce672db5c@linutronix.de

Changes in v4:
- Update "since" to 9.1
- Add comment to 'member-name-exceptions'
- Document enum members and drop 'documentation-exceptions'
- Link to v3: https://lore.kernel.org/r/20240311-qapi-firmware-json-v3-0-ceea6e35eb4a@linutronix.de

Changes in v3:
- Drop already picked up patches
- Drop include of pragma.json
- Introduce new enums FirmwareFormat and FirmwareArchitecture
- Link to v2: https://lore.kernel.org/r/20240307-qapi-firmware-json-v2-0-3b29eabb9b9a@linutronix.de

Changes in v2:
- Add review tag from Philippe
- Add Fixes tag (Philippe)
- Add testcase (Philippe)
- Link to v1: https://lore.kernel.org/r/20240306-qapi-firmware-json-v1-0-619f7122a249@linutronix.de

---
Thomas Weißschuh (4):
      docs/interop/firmware.json: add new enum FirmwareFormat
      docs/interop/firmware.json: add new enum FirmwareArchitecture
      docs/interop/firmware.json: convert "Example" section
      docs: add test for firmware.json QAPI

 docs/interop/firmware.json | 47 +++++++++++++++++++++++++++++++++++++++++-----
 docs/meson.build           |  5 +++++
 2 files changed, 47 insertions(+), 5 deletions(-)
---
base-commit: 23fa74974d8c96bc95cbecc0d4e2d90f984939f6
change-id: 20240306-qapi-firmware-json-6fb1213936dd

Best regards,
-- 
Thomas Weißschuh <thomas.weissschuh@linutronix.de>

[PATCH v6 1/4] docs/interop/firmware.json: add new enum FirmwareFormat

[Thomas Weißschuh]

Only a small subset of all blockdev drivers make sense for firmware
images. Introduce and use a new enum to represent this.

This also reduces the dependency on firmware.json from the global qapi
definitions.

Claim "Since: 3.0" for the new enum, because that's correct for its
members, and the members are what matters in the interface.

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 docs/interop/firmware.json | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
index 54a1fc6c1041..d5d4c17f230b 100644
--- a/docs/interop/firmware.json
+++ b/docs/interop/firmware.json
@@ -15,7 +15,6 @@
 ##
 
 { 'include' : 'machine.json' }
-{ 'include' : 'block-core.json' }
 
 ##
 # @FirmwareOSInterface:
@@ -200,6 +199,20 @@
              'enrolled-keys', 'requires-smm', 'secure-boot',
              'verbose-dynamic', 'verbose-static' ] }
 
+##
+# @FirmwareFormat:
+#
+# Formats that are supported for firmware images.
+#
+# @raw: Raw disk image format.
+#
+# @qcow2: The QCOW2 image format.
+#
+# Since: 3.0
+##
+{ 'enum': 'FirmwareFormat',
+  'data': [ 'raw', 'qcow2' ] }
+
 ##
 # @FirmwareFlashFile:
 #
@@ -219,7 +232,7 @@
 ##
 { 'struct' : 'FirmwareFlashFile',
   'data'   : { 'filename' : 'str',
-               'format'   : 'BlockdevDriver' } }
+               'format'   : 'FirmwareFormat' } }
 
 
 ##

-- 
2.45.2

[PATCH v6 2/4] docs/interop/firmware.json: add new enum FirmwareArchitecture

[Thomas Weißschuh]

Only a small subset of all architectures supported by qemu make use of
firmware files. Introduce and use a new enum to represent this.

This also removes the dependency to machine.json from the global qapi
definitions.

Claim "Since: 3.0" for the new enum, because that's correct for most of
its members, and the members are what matters in the interface.

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 docs/interop/firmware.json | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
index d5d4c17f230b..ae4179994479 100644
--- a/docs/interop/firmware.json
+++ b/docs/interop/firmware.json
@@ -14,7 +14,10 @@
 # = Firmware
 ##
 
-{ 'include' : 'machine.json' }
+{ 'pragma': {
+    'member-name-exceptions': [
+        'FirmwareArchitecture' # x86_64
+    ] } }
 
 ##
 # @FirmwareOSInterface:
@@ -59,6 +62,27 @@
 { 'enum' : 'FirmwareDevice',
   'data' : [ 'flash', 'kernel', 'memory' ] }
 
+##
+# @FirmwareArchitecture:
+#
+# Enumeration of architectures for which Qemu uses additional
+# firmware files.
+#
+# @aarch64: 64-bit Arm.
+#
+# @arm: 32-bit Arm.
+#
+# @i386: 32-bit x86.
+#
+# @loongarch64: 64-bit LoongArch. (since: 7.1)
+#
+# @x86_64: 64-bit x86.
+#
+# Since: 3.0
+##
+{ 'enum' : 'FirmwareArchitecture',
+  'data' : [ 'aarch64', 'arm', 'i386', 'loongarch64', 'x86_64' ] }
+
 ##
 # @FirmwareTarget:
 #
@@ -80,7 +104,7 @@
 # Since: 3.0
 ##
 { 'struct' : 'FirmwareTarget',
-  'data'   : { 'architecture' : 'SysEmuTarget',
+  'data'   : { 'architecture' : 'FirmwareArchitecture',
                'machines'     : [ 'str' ] } }
 
 ##

-- 
2.45.2

[PATCH v6 3/4] docs/interop/firmware.json: convert "Example" section

[Thomas Weißschuh]

Since commit 3c5f6114d9ff ("qapi: remove "Example" doc section")
the "Example" section is not valid anymore.
It has been replaced by the "qmp-example" directive.

This was not detected earlier as firmware.json was not validated.
As this validation is about to be added, adapt firmware.json.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 docs/interop/firmware.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
index ae4179994479..57f55f6c5455 100644
--- a/docs/interop/firmware.json
+++ b/docs/interop/firmware.json
@@ -470,7 +470,7 @@
 #
 # Since: 3.0
 #
-# Examples:
+# .. qmp-example::
 #
 #     {
 #         "description": "SeaBIOS",

-- 
2.45.2

[PATCH v6 4/4] docs: add test for firmware.json QAPI

[Thomas Weißschuh]

To make sure that the QAPI description stays valid, add a testcase.

Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Link: https://lore.kernel.org/qemu-devel/d9ce0234-4beb-4b90-b14c-76810d3b81d7@linaro.org/
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 docs/meson.build | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/docs/meson.build b/docs/meson.build
index 9040f860ae1a..bcca45a342a3 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -99,3 +99,8 @@ if build_docs
   alias_target('html', sphinxdocs)
   alias_target('man', sphinxmans)
 endif
+
+test('QAPI firmware.json regression tests', python,
+     args: [qapi_gen.full_path(), '-o', meson.current_build_dir() / 'qapi',
+            meson.current_source_dir() / 'interop/firmware.json'],
+     env: test_env, suite: ['qapi-schema', 'qapi-interop'])

-- 
2.45.2

Re: [PATCH v6 2/4] docs/interop/firmware.json: add new enum FirmwareArchitecture

[Markus Armbruster]

Thomas Weißschuh <thomas.weissschuh@linutronix.de> writes:

> Only a small subset of all architectures supported by qemu make use of
> firmware files. Introduce and use a new enum to represent this.
>
> This also removes the dependency to machine.json from the global qapi
> definitions.
>
> Claim "Since: 3.0" for the new enum, because that's correct for most of
> its members, and the members are what matters in the interface.
>
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>  docs/interop/firmware.json | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
> index d5d4c17f230b..ae4179994479 100644
> --- a/docs/interop/firmware.json
> +++ b/docs/interop/firmware.json
> @@ -14,7 +14,10 @@
>  # = Firmware
>  ##
>  
> -{ 'include' : 'machine.json' }
> +{ 'pragma': {
> +    'member-name-exceptions': [
> +        'FirmwareArchitecture' # x86_64
> +    ] } }
>  
>  ##
>  # @FirmwareOSInterface:
> @@ -59,6 +62,27 @@
>  { 'enum' : 'FirmwareDevice',
>    'data' : [ 'flash', 'kernel', 'memory' ] }
>  
> +##
> +# @FirmwareArchitecture:
> +#
> +# Enumeration of architectures for which Qemu uses additional

QEMU

> +# firmware files.
> +#
> +# @aarch64: 64-bit Arm.
> +#
> +# @arm: 32-bit Arm.
> +#
> +# @i386: 32-bit x86.
> +#
> +# @loongarch64: 64-bit LoongArch. (since: 7.1)
> +#
> +# @x86_64: 64-bit x86.
> +#
> +# Since: 3.0
> +##
> +{ 'enum' : 'FirmwareArchitecture',
> +  'data' : [ 'aarch64', 'arm', 'i386', 'loongarch64', 'x86_64' ] }
> +
>  ##
>  # @FirmwareTarget:
>  #
> @@ -80,7 +104,7 @@
>  # Since: 3.0
>  ##
>  { 'struct' : 'FirmwareTarget',
> -  'data'   : { 'architecture' : 'SysEmuTarget',
> +  'data'   : { 'architecture' : 'FirmwareArchitecture',
>                 'machines'     : [ 'str' ] } }
>  
>  ##

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH v6 1/4] docs/interop/firmware.json: add new enum FirmwareFormat

[Markus Armbruster]

Thomas Weißschuh <thomas.weissschuh@linutronix.de> writes:

> Only a small subset of all blockdev drivers make sense for firmware
> images. Introduce and use a new enum to represent this.
>
> This also reduces the dependency on firmware.json from the global qapi
> definitions.
>
> Claim "Since: 3.0" for the new enum, because that's correct for its
> members, and the members are what matters in the interface.
>
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH v6 4/4] docs: add test for firmware.json QAPI

[Markus Armbruster]

Thomas Weißschuh <thomas.weissschuh@linutronix.de> writes:

> To make sure that the QAPI description stays valid, add a testcase.
>
> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Link: https://lore.kernel.org/qemu-devel/d9ce0234-4beb-4b90-b14c-76810d3b81d7@linaro.org/
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>  docs/meson.build | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/docs/meson.build b/docs/meson.build
> index 9040f860ae1a..bcca45a342a3 100644
> --- a/docs/meson.build
> +++ b/docs/meson.build
> @@ -99,3 +99,8 @@ if build_docs
>    alias_target('html', sphinxdocs)
>    alias_target('man', sphinxmans)
>  endif
> +
> +test('QAPI firmware.json regression tests', python,
> +     args: [qapi_gen.full_path(), '-o', meson.current_build_dir() / 'qapi',
> +            meson.current_source_dir() / 'interop/firmware.json'],
> +     env: test_env, suite: ['qapi-schema', 'qapi-interop'])

Acked-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH v6 0/4] docs/interop/firmware.json: scripts/qapi-gen.py compatibility

[Philippe Mathieu-Daudé]

On 19/7/24 09:37, Thomas Weißschuh wrote:

> Thomas Weißschuh (4):
>        docs/interop/firmware.json: add new enum FirmwareFormat
>        docs/interop/firmware.json: add new enum FirmwareArchitecture
>        docs/interop/firmware.json: convert "Example" section
>        docs: add test for firmware.json QAPI

Thanks, series queued!

Re: [PATCH v6 4/4] docs: add test for firmware.json QAPI

[Philippe Mathieu-Daudé]

Hi Thomas,

On 19/7/24 09:37, Thomas Weißschuh wrote:
> To make sure that the QAPI description stays valid, add a testcase.
> 
> Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Link: https://lore.kernel.org/qemu-devel/d9ce0234-4beb-4b90-b14c-76810d3b81d7@linaro.org/
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>   docs/meson.build | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/docs/meson.build b/docs/meson.build
> index 9040f860ae1a..bcca45a342a3 100644
> --- a/docs/meson.build
> +++ b/docs/meson.build
> @@ -99,3 +99,8 @@ if build_docs
>     alias_target('html', sphinxdocs)
>     alias_target('man', sphinxmans)
>   endif
> +
> +test('QAPI firmware.json regression tests', python,
> +     args: [qapi_gen.full_path(), '-o', meson.current_build_dir() / 'qapi',
> +            meson.current_source_dir() / 'interop/firmware.json'],
> +     env: test_env, suite: ['qapi-schema', 'qapi-interop'])

Did you test this on GitLab CI? Many jobs are failing as:

Program /builds/qemu/build/pyvenv/bin/sphinx-build skipped: feature docs 
disabled
../docs/meson.build:106:10: ERROR: Unknown variable "test_env".

See https://gitlab.com/philmd/qemu/-/pipelines/1383618475/failures

Re: [PATCH v6 4/4] docs: add test for firmware.json QAPI

[Thomas Weißschuh]

Hi Philippe,

On Tue, Jul 23, 2024 at 12:15:24AM GMT, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 19/7/24 09:37, Thomas Weißschuh wrote:
> > To make sure that the QAPI description stays valid, add a testcase.
> > 
> > Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Link: https://lore.kernel.org/qemu-devel/d9ce0234-4beb-4b90-b14c-76810d3b81d7@linaro.org/
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> > ---
> >   docs/meson.build | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/docs/meson.build b/docs/meson.build
> > index 9040f860ae1a..bcca45a342a3 100644
> > --- a/docs/meson.build
> > +++ b/docs/meson.build
> > @@ -99,3 +99,8 @@ if build_docs
> >     alias_target('html', sphinxdocs)
> >     alias_target('man', sphinxmans)
> >   endif
> > +
> > +test('QAPI firmware.json regression tests', python,
> > +     args: [qapi_gen.full_path(), '-o', meson.current_build_dir() / 'qapi',
> > +            meson.current_source_dir() / 'interop/firmware.json'],
> > +     env: test_env, suite: ['qapi-schema', 'qapi-interop'])
> 
> Did you test this on GitLab CI? Many jobs are failing as:
> 
> Program /builds/qemu/build/pyvenv/bin/sphinx-build skipped: feature docs
> disabled
> ../docs/meson.build:106:10: ERROR: Unknown variable "test_env".
> 
> See https://gitlab.com/philmd/qemu/-/pipelines/1383618475/failures

No, I only tested it locally. It works there.
I'll try to setup a personal GitLab CI.

To fix the issue, I think "test_env" can be removed.
The other calls to "qapi_gen", which 

As a fix for this issue, I think "test_env" can be removed,
and apparently "qapi_gen" can also be executed without the explicit
python reference:


diff --git a/docs/meson.build b/docs/meson.build
index bcca45a342a3..322452c87787 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -100,7 +100,7 @@ if build_docs
   alias_target('man', sphinxmans)
 endif

-test('QAPI firmware.json regression tests', python,
-     args: [qapi_gen.full_path(), '-o', meson.current_build_dir() / 'qapi',
+test('QAPI firmware.json regression tests', qapi_gen,
+     args: ['-o', meson.current_build_dir() / 'qapi',
             meson.current_source_dir() / 'interop/firmware.json'],
-     env: test_env, suite: ['qapi-schema', 'qapi-interop'])
+     suite: ['qapi-schema', 'qapi-interop'])