qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuT


From: Laszlo Ersek
Subject: Re: [Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuTarget, not CpuInfoArch
Date: Thu, 26 Apr 2018 18:30:19 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0

On 04/26/18 17:51, Markus Armbruster wrote:
> Eric Blake <address@hidden> 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 @address@hidden, only produced by @query-cpus,
- in @address@hidden, 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.)

Thanks
Laszlo



reply via email to

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