qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/4] docs/interop/firmware.json: Use full include paths


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH 3/4] docs/interop/firmware.json: Use full include paths
Date: Fri, 8 Mar 2024 17:28:28 +0100
User-agent: Mozilla Thunderbird

On 8/3/24 17:09, Daniel P. Berrangé wrote:
On Fri, Mar 08, 2024 at 04:19:42PM +0100, Markus Armbruster wrote:
The coupling with the main QAPI schema is unfortunate.

The purpose of docs/interop/firmware.json is to serve as schema for
firmware descriptions: a firmware description is a JSON object that
conforms to this schema's struct Firmware.

Such a description should be installed along with each firmware binary.

QAPI tooling can be used to check conformance: parse the firmware
description JSON object, feed it to the generated visit_type_Firmware().
Success implies conformance.

If you find more uses for the C struct Firmware created by
visit_type_Firmware(), more power to you.

firmware.json needs machine.json for SysEmuTarget, and block-core.json
for BlockdevDriver.  The largest and the third-largest QAPI module just
for two enums.  Almost a quarter Mebibyte of code.

firmware.json can use BlockdevDriver, but we could question
whether it /should/ use BlockdevDriver. Is there really a
compelling reason to support every possible block driver for
readonly firmware and tiny nvram file ? I thin kit would be
totally reasonable to define a "FirmwareFormat" enum which
only permitted 'raw' and 'qcow2'.   If someone wants to
justify why we need another format, I'm all ears...

For SysEmuTarget its a little more useful, as in theory the
firmware could be extended to any QEMU target. In practice
thus far we've only used it todescribe EFI based firmware,
which is relevant for a subset of targets. It doesn't seem
to be a huge downside to define a FirmwareTarget enum with
only the arches we've actually got a use for so far. When
someone comes along with a need for non-EFI we can extend
it, and we'll need to extend libvirt at the same time anyway

qapi-gen.py generates more than 12kSLOC.  Without the include (and with
the enums dumbed down to 'str' to enable that), it generates less than
800.

We could use Sphinx to generate a manual from firmware.json's document.
Except that manual would be useless, because of its 11,000 lines of
HTML, less than 800 are for firmware.json.

=)


Options:

* Live with the mess.

* Refactor QAPI modules so firmware.json can include just the enums.

   Drawback: we spread the mess into qapi/.  Ugh.

* Copy the enums to firmware.json.

   Drawback: we risk them going stale.

IMHO copy the enum. While the risk exists, I don't think it is a
risk worth worrying about in reality. If someone points out a gap
that's important is a matter of minutes to patch it.

Since I concur with Daniel PoV, I'm queuing patches 1 (with
Markus reword) and 2 so far, so less to carry for v3.

Regards,

Phil.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]