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

On 04/19/18 11:12, Daniel P. Berrangé wrote: > On Thu, Apr 19, 2018 at 10:39:32AM +0200, Laszlo Ersek wrote: >> On 04/19/18 09:56, Daniel P. Berrangé wrote: >>> On Thu, Apr 19, 2018 at 09:48:36AM +0200, Markus Armbruster wrote: >>>> Laszlo Ersek <lersek(a)redhat.com&gt; writes: >>>> >>>>> On 04/18/18 10:47, Markus Armbruster wrote: >>>>>> Laszlo Ersek <lersek(a)redhat.com&gt; writes: >>>> 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. >> >> Commit 9d0306dfdfb added "s390" and "CpuInfoS390", which I guess had the >> potential to confuse QMP clients that didn't expect "s390", but >> otherwise it didn't mess with preexistent enum values / structures. > > NB, qemu-system-s390x would previously have returned "other" in > this field, and now it returns "s390". So while it didn't > remove "other" from the list of things that could potentially > exist, it did change what the s390x binary will actually report. > >> The same applies to commit 25fa194b7b1, just with "riscv" / >> "CpuInfoRISCV" substituted. >> >> Removing "other" might confuse QMP clients that expect it, except >> (according to Daniel) no such client exists, probably. > > When I say removing "other", I imply that we add an explicit arch > for all those which we currently are missing. IOW, all qemu-system-XXX > binaries which currently report "other" would change to report their > respective "XXX" values. > > So in this way, it is exactly the same as what we did when we > introduced "s390" as an option. > > The only difference is that once we have every binary reporting the > correct arch, we can now also remove "other" from the schema itself > as it will then be unused. Can we please translate this into more actionable items for me, because I'm getting confused :) First, if I add "i386" and "x86_64" to the enum list, we'll have all three of "i386", "x86_64" and "x86". Is that useful? How will that work?

Second, assuming I add constants for the ~10 (?) softmmu arches, can I still use @CpuInfoOther as the type for the corresponding new members in @CpuInfo? What C code changes will be necessary?