Laszlo Ersek <ler...@redhat.com> writes: > On 04/26/18 17:51, Markus Armbruster wrote: >> Eric Blake <ebl...@redhat.com> writes: >> >>> On 04/26/2018 09:34 AM, Markus Armbruster wrote: >>>>> >>>>> Acerbic discovery of the day: @CpuInfoArch has "x86", while configure >>>>> produces: >>>>> >>>>> TARGET_NAME TARGET_BASE_ARCH >>>>> i386 i386 >>>>> x86_64 i386 >>>>> >>>>> Note how "i386" does not match "x86". >>>> >>>> Review fail. >>>> >>>> Just three weeks ago, we could still have fixed query-cpus-fast... >>> >>> Actually, I think we still can. We already documented in the 2.12 >>> release notes that the "arch" field of query-cpus-fast is known to be >>> broken for all but "s390x" (which is really the only arch field that >>> MUST be correct, as that is the only time we send additional >>> information). And introspection can easily see both the enum contents >>> (if we add something) as well as any other additions to the >>> query-cpus-fast output union (although that is less likely), to use >>> those as a witness for whether qemu is new enough to have fixed the >>> bogus "arch" values. I'd argue that if we change things right now, with >>> intent to include the change in 2.12.1, before people start relying on >>> the bogus "arch" of 2.12, then we should feel free to make >>> query-cpus-fast output whatever we want for all architectures other than >>> "s390x", even if it changes the current output of "x86". >> >> In other words, we managed to screw up query-cpus-fast sufficiently to >> let us fix it even now. Cool, let's do it! >> > > Let me clarify a little. > > The @CpuInfoArch enum has the following constants now: > > ['x86', 'sparc', 'ppc', 'mips', 'tricore', 's390', 'riscv', 'other' ] > > This enum was originally introduced in 2.6 (according to the > documentation), and only the @s390 and @riscv constants were added in 2.12. > > The enum constants show up in the following two places, *on the wire*: > > - in @CpuInfo.@arch, only produced by @query-cpus, > - in @CpuInfoFast.@arch, only produced by @query-cpus-fast. > > The plan would be to rewrite *all* those enum constants of @CpuInfoArch > to which the respective TARGET_BASE_ARCH values (from "configure") do > not map *identically*. Here's the mapping: > > TARGET_BASE_ARCH CpuInfoArch CpuInfoArch needs change > ---------------- ----------- ------------------------ > i386 x86 YES > sparc sparc no > ppc ppc no > mips mips no > tricore tricore no > s390x s390 YES > riscv riscv no > > In other words, @CpuInfoArch would have to be changed to the following: > > ['i386', 'sparc', 'ppc', 'mips', 'tricore', 's390x', 'riscv', 'other' ] > ^^^^^^ ^^^^^^^ > > This means that the @arch field, returned by @query-cpus and > @query-cpus-fast, would change incompatibly for those QAPI clients that > look specifically for "x86" or "s390". > > Is this a safe change? > > I would say, because of the 's390' -> 's390x' change, that it isn't. > > (Also, to confirm, the wiki section at > <https://wiki.qemu.org/Planning/2.12#Issues_that_will_not_be_fixed> states, > > * the query-cpus-fast QMP command reports bogus arch data for all > architectures except x86 and s390; applications should be careful to > not rely on the bogus information > > It (correctly) refers to "s390". That value would change.)
You're right, that's a compatibility break. We could perhaps still declare *all* @arch values useless in v2.12.0, then fix them in v2.12.1. Or we deprecate @arch right when we introduce @target, and drop it later in accordance with our deprecation policy (qemu-doc.texi @appendix Deprecated features). That way, the rather ridiculous code to compute it will be temporary. I think that's cleaner. @arch in query-cpus query-cpus-fast before 2.6 nonexistent 2.6 - 2.11 CpuInfoArch 2.12 cmd deprecated CpuInfoArch 2.13 cmd deprecated memb deprecated 2.14 cmd gone memb deprecated 2.15 cmd gone memb gone Opinions?
