6:35 a.m.
On 04/18/18 10:47, Markus Armbruster wrote:
Laszlo Ersek <lersek(a)redhat.com> writes:
> +##
> +# @FirmwareArchitecture:
> +#
> +# Defines the target architectures (emulator binaries) that firmware may
> +# execute on.
> +#
> +# @aarch64: The firmware can be executed by the qemu-system-aarch64 emulator.
> +#
> +# @arm: The firmware can be executed by the qemu-system-arm emulator.
> +#
> +# @i386: The firmware can be executed by the qemu-system-i386 emulator.
> +#
> +# @x86_64: The firmware can be executed by the qemu-system-x86_64 emulator.
> +#
> +# Since: 2.13
> +##
> +{ 'enum' : 'FirmwareArchitecture',
> + 'data' : [ 'aarch64', 'arm', 'i386',
'x86_64' ] }
Partial dupe of CpuInfoArch from misc.json. Neither covers all our
target architectures. Should we have one that does in common.json
instead?
If we had one there, I'd use it here :)
For collecting the arch identifiers, is it OK to run "./configure
--help", and look for the "*-softmmu" options under
"--target-list=LIST"? Because that's what I need here; the qemu-system-*
emulators.
> +##
> +# @FirmwareFeature:
> +#
> +# Defines the features that firmware may support, and the platform requirements
> +# that firmware may present.
> +#
> +# @acpi-s3: The firmware supports S3 sleep (suspend to RAM), as defined in the
> +# ACPI specification. On the "pc" machine type of the @i386 and
> +# @x86_64 emulation targets, S3 can be enabled with "-global
> +# PIIX4_PM.disable_s3=0" and disabled with "-global
> +# PIIX4_PM.disable_s3=1". On the "q35" machine type of the
@i386 and
> +# @x86_64 emulation targets, S3 can be enabled with "-global
> +# ICH9-LPC.disable_s3=0" and disabled with "-global
> +# ICH9-LPC.disable_s3=1".
> +#
> +# @acpi-s4: The firmware supports S4 hibernation (suspend to disk), as defined
> +# in the ACPI specification. On the "pc" machine type of the
@i386
> +# and @x86_64 emulation targets, S4 can be enabled with "-global
> +# PIIX4_PM.disable_s4=0" and disabled with "-global
> +# PIIX4_PM.disable_s4=1". On the "q35" machine type of the
@i386 and
> +# @x86_64 emulation targets, S4 can be enabled with "-global
> +# ICH9-LPC.disable_s4=0" and disabled with "-global
> +# ICH9-LPC.disable_s4=1".
Would you mind breaking documentation lines a bit ealier?
Not at all; I aimed at 79 characters (like I do for the QEMU C source
code as well), but perhaps that's too wide for schema docs. What width
do you prefer?
While at it: is it possible to break the documentation for a single
@entry to multiple paragraphs? I tried it (simply by inserting an empty
line, without starting another @entry), and the generator didn't like it.
> +##
> +# @FirmwareFlashFile:
> +#
> +# Defines common properties that are necessary for loading a firmware file into
> +# a pflash chip. The corresponding QEMU command line option is "-drive
> +# file=@pathname,format=@format". Note however that the option-argument shown
> +# here is incomplete; it is completed under @FirmwareMappingFlash.
> +#
> +# @pathname: Specifies the pathname on the host filesystem where the firmware
> +# file can be found.
We use @filename elsewhere in the QAPI schema. Let's stick to that.
More of the same below.
Will do.
> +#
> +# @format: Specifies the block format of the file pointed-to by @pathname, such
> +# as @raw or @qcow2.
> +#
> +# Since: 2.13
> +##
> +{ 'struct' : 'FirmwareFlashFile',
> + 'data' : { 'pathname' : 'str',
> + 'format' : 'BlockdevDriver' } }
> +
> +##
> +# @FirmwareMappingFlash:
> +#
> +# Describes loading and mapping properties for the firmware executable and its
> +# accompanying NVRAM file, when @FirmwareDevice is @flash.
> +#
> +# @executable: Identifies the firmware executable. The firmware executable may
> +# be shared by multiple virtual machine definitions. The
> +# corresponding QEMU command line option is "-drive
> +#
if=pflash,unit=0,readonly=on,file=@executable.@pathname,format=@executable.(a)format".
I guess @executable.@pathname means member @pathname of @executable. I
read it as two distinct parameters first, then wondered where parameter
@pathname is. Perhaps @executable.pathname would be clearer.
I can try that, yes -- in the HTML documentation, will the monospace
style apply to the full field specification?
> +#
> +# @nvram_template: Identifies the NVRAM template compatible with @executable.
> +# Management software instantiates an individual copy -- a
> +# specific NVRAM file -- from @nvram_template.@pathname for
> +# each new virtual machine definition created.
> +# @nvram_template.@pathname itself is never mapped into
> +# virtual machines, only individual copies of it are. An NVRAM
> +# file is typically used for persistently storing the
> +# non-volatile UEFI variables of a virtual machine definition.
> +# The corresponding QEMU command line option is "-drive
> +#
if=pflash,unit=1,readonly=off,file=PATHNAME_OF_PRIVATE_NVRAM_FILE,format=@nvram_template.(a)format".
> +#
> +# Since: 2.13
> +##
> +{ 'struct' : 'FirmwareMappingFlash',
> + 'data' : { 'executable' : 'FirmwareFlashFile',
> + 'nvram_template' : 'FirmwareFlashFile' } }
Here's one thing I'll comment on myself: "nvram_template" should have
been spelled "nvram-template" :)
> +
> +##
> +# @FirmwareMappingKernel:
> +#
> +# Describes loading and mapping properties for the firmware executable, when
> +# @FirmwareDevice is @kernel.
> +#
> +# @pathname: Identifies the firmware executable. The firmware executable may be
> +# shared by multiple virtual machine definitions. The corresponding
> +# QEMU command line option is "-kernel @pathname".
> +#
> +# Since: 2.13
> +##
> +{ 'struct' : 'FirmwareMappingKernel',
> + 'data' : { 'pathname' : 'str' } }
> +
> +##
> +# @FirmwareMappingMemory:
> +#
> +# Describes loading and mapping properties for the firmware executable, when
> +# @FirmwareDevice is @memory.
> +#
> +# @pathname: Identifies the firmware executable. The firmware executable may be
> +# shared by multiple virtual machine definitions. The corresponding
> +# QEMU command line option is "-bios @pathname".
> +#
> +# Since: 2.13
> +##
> +{ 'struct' : 'FirmwareMappingMemory',
> + 'data' : { 'pathname' : 'str' } }
> +
> +##
> +# @FirmwareMapping:
> +#
> +# Provides a discriminated structure for firmware to describe its loading /
> +# mapping properties.
> +#
> +# @device: Selects the device type that the firmware must be mapped into.
> +#
> +# Since: 2.13
> +##
> +{ 'union' : 'FirmwareMapping',
> + 'base' : { 'device' : 'FirmwareDevice' },
> + 'discriminator' : 'device',
> + 'data' : { 'flash' : 'FirmwareMappingFlash',
> + 'kernel' : 'FirmwareMappingKernel',
> + 'memory' : 'FirmwareMappingMemory' } }
The FirmwareMapping* all have a member @pathname. It could be moved to
the base. Makes sense if we think future FirmwareMappingFOOs will also
have it. Your choice.
I could do that, but there would be consequences:
- FirmwareMappingKernel and FirmwareMappingMemory would become empty
structures,
- in the documentation of @FirmwareMapping, I couldn't document @flash /
@kernel / @memory individually (I tried -- it doesn't work; the
generator wants to generate the documentation automatically).
The issue is that I would like to keep the QEMU cmdline options
"-kernel" and "-bios" *close* to @FirmwareMappingKernel and
@FirmwareMappingMemory. Now, if I make those structs empty, but keep the
-kernel / -bios cmdline options right under them, then I cannot refer to
@pathname (well, @filename) in those options -- because @filename will
no longer be defined under @FirmwareMappingKernel / @FirmwareMappingMemory.
And if I move the documentation of -kernel / -bios under
@FirmwareMapping, then it's awkward again, because then I have to dump
both -kernel and -bios into the documentation of @filename, and explain
which belongs to which union member / discriminator value.
So, if it's not a problem, I'd like to stick with this variant, for the
sake of documenting -kernel and -bios more clearly.
> +##
> +# @Firmware:
> +#
> +# Describes a firmware (or a firmware use case) to management software.
> +#
> +# @description: Provides a human-readable description of the firmware.
> +# Management software may or may not display @description.
> +#
> +# @type: Exposes the type of the firmware. @type is generally the primary key
> +# for which management software looks when picking a firmware for a new
> +# virtual machine definition.
> +#
> +# @mapping: Describes the loading / mapping properties of the firmware.
> +#
> +# @targets: Collects the target architectures (QEMU system emulators) and their
> +# machine types that may execute the firmware.
> +#
> +# @features: Lists the features that the firmware supports, and the platform
> +# requirements it presents.
> +#
> +# @tags: A list of auxiliary strings associated with the firmware for which
> +# @description is not approprite, due to the latter's possible exposure
s/approprite/appropriate/
Feed the new comments to a spell checker?
Right, I got "aspell" installed, and sometimes run it on my status
reports :) I was too tired to do it for the schema. Good catch, thanks.
Introspection has a similar need for validating data against the
schema,
but it solves it with a test case without adding a QMP command. Commit
39a18158165:
A new test-qmp-input-visitor test case feeds its result for both
tests/qapi-schema/qapi-schema-test.json and qapi-schema.json to a
QmpInputVisitor to verify it actually conforms to the schema.
The test case has since moved to tests/test-qobject-input-visitor.c,
function test_visitor_in_qmp_introspect(). Please check it out to see
whether you can do without a QMP command, too.
Paolo suggested earlier that no C code should be added ultimately; the
schema should be moved under "docs/interop/". I was OK with that, just
wanted to keep the examples verifiable against the schema until the
patch got committed:
http://mid.mail-archive.com/9d6b3e23-ed02-0079-50d0-15de8b11810f@redhat.com
So in the non-RFC version, the QMP command shouldn't exist at all.
Thanks,
Laszlo