[libvirt] [Qemu-devel] [qemu RFC v2] qapi: add "firmware.json"

[Daniel P. Berrangé]

On Thu, Apr 19, 2018 at 09:48:36AM +0200, Markus Armbruster wrote:
> Laszlo Ersek <lersek at redhat.com> writes:
> 
> > On 04/18/18 10:47, Markus Armbruster wrote:
> >> Laszlo Ersek <lersek at 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.
> 
> configure gets its list like this:
> 
>     default_target_list=""
> 
>     mak_wilds=""
> 
>     if [ "$softmmu" = "yes" ]; then
>         mak_wilds="${mak_wilds} $source_path/default-configs/*-softmmu.mak"
>     fi
>     if [ "$linux_user" = "yes" ]; then
>         mak_wilds="${mak_wilds} $source_path/default-configs/*-linux-user.mak"
>     fi
>     if [ "$bsd_user" = "yes" ]; then
>         mak_wilds="${mak_wilds} $source_path/default-configs/*-bsd-user.mak"
>     fi
> 
>     for config in $mak_wilds; do
>         default_target_list="${default_target_list} $(basename "$config" .mak)"
>     done
> 
> Since we use QMP only in system emulation, a QAPI enum limited to the
> system emulation targets makes sense.
> 
> Replacing CpuInfoArch by such an enum will change the discriminator
> value from "other" to the real architecture, with the obvious
> compatibility concerns.  But we've accepted similar changes twice
> already: commit 9d0306dfdfb and commit 25fa194b7b1, both v2.12.0-rc0.
> 
> "other" was a bad idea.  Hindsight 20/20.
> 
> Getting rid of it in one go rather than piecemeal seems like the least
> bad way out.  Too late for 2.12, though.  Eric, what do you think?

Given the context in which this "other" value is used, I think it is
reasonable to kill it and put a full arch list in there.

No app is likely to be accessing the struct under "other" because it
is just an empty placeholder.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|