All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: qemu-devel@nongnu.org,
	Peter Crosthwaite <crosthwaite.peter@gmail.com>,
	qemu-stable@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
	Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH 1/6] qapi: fill in CpuInfoFast.arch in query-cpus-fast
Date: Wed, 25 Apr 2018 08:39:38 +0200	[thread overview]
Message-ID: <87r2n3aixh.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <20180424214550.32549-2-lersek@redhat.com> (Laszlo Ersek's message of "Tue, 24 Apr 2018 23:45:45 +0200")

Laszlo Ersek <lersek@redhat.com> writes:

> Commit ca230ff33f89 added added the @arch field to @CpuInfoFast, but it
> failed to set the new field in qmp_query_cpus_fast(), when TARGET_S390X
> was not defined. The updated @query-cpus-fast example in
> "qapi-schema.json" showed "arch":"x86" only because qmp_query_cpus_fast()
> calls g_malloc0() to allocate CpuInfoFast, and the CPU_INFO_ARCH_X86 enum
> constant is generated with value 0.
>
> All @arch values other than @s390 implied the @CpuInfoOther sub-struct for
> @CpuInfoFast -- at the time of writing the patch --, thus no fields other
> than @arch needed to be set when TARGET_S390X was not defined. Set @arch
> now, by copying the corresponding assignments from qmp_query_cpus().

Now I'm confused.

In the schema, @arch "riscv" implies CpuInfoRISCV:

    { 'union': 'CpuInfoFast',
      'base': {'cpu-index': 'int', 'qom-path': 'str',
               'thread-id': 'int', '*props': 'CpuInstanceProperties',
               'arch': 'CpuInfoArch' },
      'discriminator': 'arch',
      'data': { 'x86': 'CpuInfoOther',
                'sparc': 'CpuInfoOther',
                'ppc': 'CpuInfoOther',
                'mips': 'CpuInfoOther',
                'tricore': 'CpuInfoOther',
                's390': 'CpuInfoS390',
                'riscv': 'CpuInfoRISCV',
                'other': 'CpuInfoOther' } }

In qmp_query_cpus_fast(), it can't imply anything, because can't occur.
That's a bug, and this patch fixes it.  Except it sets @arch to "other"
instead of "riscv" when defined(TARGET_RISCV).  Why?  Oh, I see, that
gets fixed in the next patch.  Please explain that in your commit
message, or squash the two patches.  The latter feels simpler, so that's
what I'd do.

>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: qemu-stable@nongnu.org
> Fixes: ca230ff33f89bf7102cbfbc2328716da6750aaed
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

  parent reply	other threads:[~2018-04-25  6:39 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-24 21:45 [Qemu-devel] [PATCH 0/6] qapi: introduce the SysEmuTarget enumeration Laszlo Ersek
2018-04-24 21:45 ` [Qemu-devel] [PATCH 1/6] qapi: fill in CpuInfoFast.arch in query-cpus-fast Laszlo Ersek
2018-04-24 22:30   ` Eric Blake
2018-04-25 12:30     ` Laszlo Ersek
2018-04-25  6:39   ` Markus Armbruster [this message]
2018-04-25 12:30     ` Laszlo Ersek
2018-04-25  7:28   ` Cornelia Huck
2018-04-24 21:45 ` [Qemu-devel] [PATCH 2/6] qapi: handle the riscv CpuInfoArch " Laszlo Ersek
2018-04-24 22:32   ` Eric Blake
2018-04-25 12:32     ` Laszlo Ersek
2018-04-25  6:44   ` Markus Armbruster
2018-04-25  7:48     ` Cornelia Huck
2018-04-25 12:38       ` Viktor VM Mihajlovski
2018-04-25 12:43       ` Laszlo Ersek
2018-04-24 21:45 ` [Qemu-devel] [PATCH 3/6] qapi: add SysEmuTarget to "common.json" Laszlo Ersek
2018-04-24 23:11   ` Eric Blake
2018-04-25 12:54     ` Daniel P. Berrangé
2018-04-25 19:05       ` Laszlo Ersek
2018-04-25 19:08         ` Eric Blake
2018-04-25 22:57           ` Laszlo Ersek
2018-04-24 21:45 ` [Qemu-devel] [PATCH 4/6] qapi: change the type of TargetInfo.arch from string to enum SysEmuTarget Laszlo Ersek
2018-04-25  6:48   ` Markus Armbruster
2018-04-25 12:58     ` Laszlo Ersek
2018-04-24 21:45 ` [Qemu-devel] [PATCH 5/6] qapi: extract CpuInfoCommon to mitigate schema duplication Laszlo Ersek
2018-04-25  7:06   ` Markus Armbruster
2018-04-25 13:20     ` Laszlo Ersek
2018-04-25 17:12       ` Markus Armbruster
2018-04-25 19:12       ` Eric Blake
2018-04-25 22:56         ` Laszlo Ersek
2018-04-26  6:19           ` Markus Armbruster
2018-04-24 21:45 ` [Qemu-devel] [PATCH 6/6] qapi: discriminate CpuInfo[Fast] on SysEmuTarget, not CpuInfoArch Laszlo Ersek
2018-04-25  7:33   ` Markus Armbruster
2018-04-25 13:47     ` Laszlo Ersek
2018-04-26  6:26       ` Markus Armbruster
2018-04-26  9:18         ` Laszlo Ersek
2018-04-26 11:57           ` Markus Armbruster
2018-04-26 13:33           ` Laszlo Ersek
2018-04-26 14:34             ` Markus Armbruster
2018-04-26 14:48               ` Eric Blake
2018-04-26 15:51                 ` Markus Armbruster
2018-04-26 16:30                   ` Laszlo Ersek
2018-04-27  6:53                     ` Markus Armbruster
2018-04-27 13:46                       ` Eric Blake
2018-04-24 22:03 ` [Qemu-devel] [PATCH 0/6] qapi: introduce the SysEmuTarget enumeration no-reply
2018-04-25 12:26   ` Laszlo Ersek
2018-04-25 14:37     ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87r2n3aixh.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=crosthwaite.peter@gmail.com \
    --cc=lersek@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.