qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: peter.maydell@linaro.org, marcel.a@redhat.com,
	"Andreas Färber" <afaerber@suse.de>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order
Date: Mon, 22 Sep 2014 15:36:41 +0300	[thread overview]
Message-ID: <20140922123641.GB7992@redhat.com> (raw)
In-Reply-To: <54201614.6040903@redhat.com>

On Mon, Sep 22, 2014 at 02:29:08PM +0200, Laszlo Ersek wrote:
> On 09/22/14 14:04, Andreas Färber wrote:
> > Am 22.09.2014 um 13:26 schrieb Laszlo Ersek:
> >> Based on the registration order captured in the previous patch, we
> >> sort the ad-hoc list printed for
> >>
> >>   qemu-system-XXXX -M \?
> >
> > Agree that the order is worth sanitizing. I would however argue that
> > registration order is not entirely stable either if you think of
> > non-PC cases where there's dozens of source files registering one
> > machine each. I would therefore propose alphabetical order as we do
> > for QOM'ified CPUs.
> 
> I did mention alphabetical order in the message of patch #1 -- in fact
> that was what I implemented first:
> 
> > The first idea to restore ordering is to sort the ad-hoc list in
> > machine_parse() by "MachineClass.name". Such a name-based ordering
> > would have to be reverse, so that more recent versioned machine types
> > appear higher on the list than older versioned machine types (eg. with
> > qemu-system-x86_64). However, such a reverse sort wreaks havoc between
> > non-versioned machine types (such as those of qemu-system-aarch64).
> 
> Simply put, it looks very ugly. Namely, if you sort it in alphabetically
> *ascending* order, then for the x86_64 target, you get:
> 
> > isapc                ISA-only PC
> > none                 empty machine
> > pc                   Standard PC (i440FX + PIIX, 1996) (alias of pc-i440fx-2.2)
> > pc-0.10              Standard PC (i440FX + PIIX, 1996)
> > pc-0.11              Standard PC (i440FX + PIIX, 1996)
> > pc-0.12              Standard PC (i440FX + PIIX, 1996)
> > pc-0.13              Standard PC (i440FX + PIIX, 1996)
> > pc-0.14              Standard PC (i440FX + PIIX, 1996)
> > pc-0.15              Standard PC (i440FX + PIIX, 1996)
> > pc-1.0               Standard PC (i440FX + PIIX, 1996)
> > pc-1.1               Standard PC (i440FX + PIIX, 1996)
> > pc-1.2               Standard PC (i440FX + PIIX, 1996)
> > pc-1.3               Standard PC (i440FX + PIIX, 1996)
> > pc-i440fx-1.4        Standard PC (i440FX + PIIX, 1996)
> > pc-i440fx-1.5        Standard PC (i440FX + PIIX, 1996)
> > pc-i440fx-1.6        Standard PC (i440FX + PIIX, 1996)
> > pc-i440fx-1.7        Standard PC (i440FX + PIIX, 1996)
> > pc-i440fx-2.0        Standard PC (i440FX + PIIX, 1996)
> > pc-i440fx-2.1        Standard PC (i440FX + PIIX, 1996)
> > pc-i440fx-2.2        Standard PC (i440FX + PIIX, 1996) (default)
> > pc-q35-1.4           Standard PC (Q35 + ICH9, 2009)
> > pc-q35-1.5           Standard PC (Q35 + ICH9, 2009)
> > pc-q35-1.6           Standard PC (Q35 + ICH9, 2009)
> > pc-q35-1.7           Standard PC (Q35 + ICH9, 2009)
> > pc-q35-2.0           Standard PC (Q35 + ICH9, 2009)
> > pc-q35-2.1           Standard PC (Q35 + ICH9, 2009)
> > pc-q35-2.2           Standard PC (Q35 + ICH9, 2009)
> > q35                  Standard PC (Q35 + ICH9, 2009) (alias of pc-q35-2.2)
> 
> which is very bad / unusual. Okay, so let's sort it in alphabetically
> descending order:
> 
> > q35                  Standard PC (Q35 + ICH9, 2009) (alias of pc-q35-2.2)
> > pc-q35-2.2           Standard PC (Q35 + ICH9, 2009)
> > pc-q35-2.1           Standard PC (Q35 + ICH9, 2009)
> > pc-q35-2.0           Standard PC (Q35 + ICH9, 2009)
> > pc-q35-1.7           Standard PC (Q35 + ICH9, 2009)
> > pc-q35-1.6           Standard PC (Q35 + ICH9, 2009)
> > pc-q35-1.5           Standard PC (Q35 + ICH9, 2009)
> > pc-q35-1.4           Standard PC (Q35 + ICH9, 2009)
> > pc-i440fx-2.2        Standard PC (i440FX + PIIX, 1996) (default)
> > pc-i440fx-2.1        Standard PC (i440FX + PIIX, 1996)
> > pc-i440fx-2.0        Standard PC (i440FX + PIIX, 1996)
> > pc-i440fx-1.7        Standard PC (i440FX + PIIX, 1996)
> > pc-i440fx-1.6        Standard PC (i440FX + PIIX, 1996)
> > pc-i440fx-1.5        Standard PC (i440FX + PIIX, 1996)
> > pc-i440fx-1.4        Standard PC (i440FX + PIIX, 1996)
> > pc-1.3               Standard PC (i440FX + PIIX, 1996)
> > pc-1.2               Standard PC (i440FX + PIIX, 1996)
> > pc-1.1               Standard PC (i440FX + PIIX, 1996)
> > pc-1.0               Standard PC (i440FX + PIIX, 1996)
> > pc-0.15              Standard PC (i440FX + PIIX, 1996)
> > pc-0.14              Standard PC (i440FX + PIIX, 1996)
> > pc-0.13              Standard PC (i440FX + PIIX, 1996)
> > pc-0.12              Standard PC (i440FX + PIIX, 1996)
> > pc-0.11              Standard PC (i440FX + PIIX, 1996)
> > pc-0.10              Standard PC (i440FX + PIIX, 1996)
> > pc                   Standard PC (i440FX + PIIX, 1996) (alias of pc-i440fx-2.2)
> > none                 empty machine
> > isapc                ISA-only PC
> 
> Okay, this is certainly bearable. However, let's see what the reverse
> alpha sort does to aarch64:
> 
> > z2                   Zipit Z2 (PXA27x)
> > xilinx-zynq-a9       Xilinx Zynq Platform Baseboard for Cortex-A9
> > virt                 ARM Virtual Machine
> > vexpress-a9          ARM Versatile Express for Cortex-A9
> > vexpress-a15         ARM Versatile Express for Cortex-A15
> > versatilepb          ARM Versatile/PB (ARM926EJ-S)
> > versatileab          ARM Versatile/AB (ARM926EJ-S)
> > verdex               Gumstix Verdex (PXA270)
> > tosa                 Tosa PDA (PXA255)
> > terrier              Terrier PDA (PXA270)
> > sx1-v1               Siemens SX1 (OMAP310) V1
> > sx1                  Siemens SX1 (OMAP310) V2
> > spitz                Spitz PDA (PXA270)
> > smdkc210             Samsung SMDKC210 board (Exynos4210)
> > realview-pbx-a9      ARM RealView Platform Baseboard Explore for Cortex-A9
> > realview-pb-a8       ARM RealView Platform Baseboard for Cortex-A8
> > realview-eb-mpcore   ARM RealView Emulation Baseboard (ARM11MPCore)
> > realview-eb          ARM RealView Emulation Baseboard (ARM926EJ-S)
> > nuri                 Samsung NURI board (Exynos4210)
> > none                 empty machine
> > n810                 Nokia N810 tablet aka. RX-44 (OMAP2420)
> > n800                 Nokia N800 tablet aka. RX-34 (OMAP2420)
> > musicpal             Marvell 88w8618 / MusicPal (ARM926EJ-S)
> > midway               Calxeda Midway (ECX-2000)
> > mainstone            Mainstone II (PXA27x)
> > lm3s811evb           Stellaris LM3S811EVB
> > lm3s6965evb          Stellaris LM3S6965EVB
> > kzm                  ARM KZM Emulation Baseboard (ARM1136)
> > integratorcp         ARM Integrator/CP (ARM926EJ-S)
> > highbank             Calxeda Highbank (ECX-1000)
> > cubieboard           cubietech cubieboard
> > connex               Gumstix Connex (PXA255)
> > collie               Collie PDA (SA-1110)
> > cheetah              Palm Tungsten|E aka. Cheetah PDA (OMAP310)
> > canon-a1100          Canon PowerShot A1100 IS
> > borzoi               Borzoi PDA (PXA270)
> > akita                Akita PDA (PXA270)
> 
> Ugh? The reverse sort is painfully obvious, and the user wonders why it
> is necessary. Why is z2 at the top, and akita at the bottom? Why is n810
> above n800 and not below it? And so on.
>
> Instead, the patchset restores the behavior that had been visible before
> commit 261747f1: programmer-controlled logical order *within* clusters,
> and unspecified order *between* clusters.

We could add a cluster name in the API.
Use that for sort between clusters only.
Hmm?

> IOW, I tested your idea (because it was my first idea as well), and I
> thought (and still think) that alpha sort is inferior to the
> pre-261747f1 order.
> 
> Thanks,
> Laszlo

  reply	other threads:[~2014-09-22 12:33 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-22 11:26 [Qemu-devel] [PATCH 0/2] list supported machine types in their registration order Laszlo Ersek
2014-09-22 11:26 ` [Qemu-devel] [PATCH 1/2] save registration order of machine types Laszlo Ersek
2014-09-22 11:26 ` [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order Laszlo Ersek
2014-09-22 12:04   ` Andreas Färber
2014-09-22 12:26     ` Michael S. Tsirkin
2014-09-22 12:29     ` Laszlo Ersek
2014-09-22 12:36       ` Michael S. Tsirkin [this message]
2014-09-22 12:50         ` Marcel Apfelbaum
2014-09-22 13:43           ` Laszlo Ersek
2014-09-22 15:07             ` Marcel Apfelbaum
2014-09-22 13:15         ` Laszlo Ersek
2014-09-22 15:57           ` Paolo Bonzini
2014-09-22 16:06             ` Andreas Färber
2014-09-22 16:17               ` Paolo Bonzini
2014-09-22 16:13             ` Laszlo Ersek

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=20140922123641.GB7992@redhat.com \
    --to=mst@redhat.com \
    --cc=afaerber@suse.de \
    --cc=lersek@redhat.com \
    --cc=marcel.a@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).