diff mbox

[2/2] machine_parse(): list supported machine types in their registration order

Message ID 1411385195-13495-3-git-send-email-lersek@redhat.com
State New
Headers show

Commit Message

Laszlo Ersek Sept. 22, 2014, 11:26 a.m. UTC 
Based on the registration order captured in the previous patch, we sort
the ad-hoc list printed for

  qemu-system-XXXX -M \?

The GLib documentation is not overly clear if g_slist_sort() allocates a
new list or reorders the input list in place, but:
- it slightly suggests the latter,
- and there is prior use in qemu that is consistent with the latter:
  g_slist_sort() in qdev_print_devinfos() [qdev-monitor.c] sorts the
  return value of object_class_get_list() the same as we do here in
  machine_parse().

The patch has the following effect on the x86_64 output:

Before:

> Supported machines are:
> pc-0.13              Standard PC (i440FX + PIIX, 1996)
> pc-i440fx-2.0        Standard PC (i440FX + PIIX, 1996)
> pc-1.0               Standard PC (i440FX + PIIX, 1996)
> pc-i440fx-2.1        Standard PC (i440FX + PIIX, 1996)
> pc-q35-1.7           Standard PC (Q35 + ICH9, 2009)
> pc-1.1               Standard PC (i440FX + PIIX, 1996)
> pc-0.14              Standard PC (i440FX + PIIX, 1996)
> pc-q35-2.0           Standard PC (Q35 + ICH9, 2009)
> pc-i440fx-1.4        Standard PC (i440FX + PIIX, 1996)
> pc-i440fx-1.5        Standard PC (i440FX + PIIX, 1996)
> pc-0.15              Standard PC (i440FX + PIIX, 1996)
> pc-q35-1.4           Standard PC (Q35 + ICH9, 2009)
> isapc                ISA-only PC
> pc                   Standard PC (i440FX + PIIX, 1996) (alias of pc-i440fx-2.2)
> pc-i440fx-2.2        Standard PC (i440FX + PIIX, 1996) (default)
> pc-1.2               Standard PC (i440FX + PIIX, 1996)
> pc-0.10              Standard PC (i440FX + PIIX, 1996)
> pc-0.11              Standard PC (i440FX + PIIX, 1996)
> pc-q35-2.1           Standard PC (Q35 + ICH9, 2009)
> q35                  Standard PC (Q35 + ICH9, 2009) (alias of pc-q35-2.2)
> pc-q35-2.2           Standard PC (Q35 + ICH9, 2009)
> pc-i440fx-1.6        Standard PC (i440FX + PIIX, 1996)
> pc-i440fx-1.7        Standard PC (i440FX + PIIX, 1996)
> none                 empty machine
> pc-q35-1.5           Standard PC (Q35 + ICH9, 2009)
> pc-q35-1.6           Standard PC (Q35 + ICH9, 2009)
> pc-0.12              Standard PC (i440FX + PIIX, 1996)
> pc-1.3               Standard PC (i440FX + PIIX, 1996)

After:

> Supported machines are:
> pc                   Standard PC (i440FX + PIIX, 1996) (alias of pc-i440fx-2.2)
> 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)
> isapc                ISA-only PC
> 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)
> none                 empty machine

On the aarch64 output:

Before:

> Supported machines are:
> lm3s811evb           Stellaris LM3S811EVB
> canon-a1100          Canon PowerShot A1100 IS
> vexpress-a15         ARM Versatile Express for Cortex-A15
> vexpress-a9          ARM Versatile Express for Cortex-A9
> xilinx-zynq-a9       Xilinx Zynq Platform Baseboard for Cortex-A9
> connex               Gumstix Connex (PXA255)
> n800                 Nokia N800 tablet aka. RX-34 (OMAP2420)
> lm3s6965evb          Stellaris LM3S6965EVB
> versatileab          ARM Versatile/AB (ARM926EJ-S)
> borzoi               Borzoi PDA (PXA270)
> tosa                 Tosa PDA (PXA255)
> cheetah              Palm Tungsten|E aka. Cheetah PDA (OMAP310)
> midway               Calxeda Midway (ECX-2000)
> mainstone            Mainstone II (PXA27x)
> n810                 Nokia N810 tablet aka. RX-44 (OMAP2420)
> terrier              Terrier PDA (PXA270)
> highbank             Calxeda Highbank (ECX-1000)
> cubieboard           cubietech cubieboard
> sx1-v1               Siemens SX1 (OMAP310) V1
> sx1                  Siemens SX1 (OMAP310) V2
> realview-eb-mpcore   ARM RealView Emulation Baseboard (ARM11MPCore)
> kzm                  ARM KZM Emulation Baseboard (ARM1136)
> akita                Akita PDA (PXA270)
> z2                   Zipit Z2 (PXA27x)
> musicpal             Marvell 88w8618 / MusicPal (ARM926EJ-S)
> realview-pb-a8       ARM RealView Platform Baseboard for Cortex-A8
> versatilepb          ARM Versatile/PB (ARM926EJ-S)
> realview-eb          ARM RealView Emulation Baseboard (ARM926EJ-S)
> realview-pbx-a9      ARM RealView Platform Baseboard Explore for Cortex-A9
> spitz                Spitz PDA (PXA270)
> none                 empty machine
> virt                 ARM Virtual Machine
> collie               Collie PDA (SA-1110)
> smdkc210             Samsung SMDKC210 board (Exynos4210)
> verdex               Gumstix Verdex (PXA270)
> nuri                 Samsung NURI board (Exynos4210)
> integratorcp         ARM Integrator/CP (ARM926EJ-S)

After:

> Supported machines are:
> collie               Collie PDA (SA-1110)
> nuri                 Samsung NURI board (Exynos4210)
> smdkc210             Samsung SMDKC210 board (Exynos4210)
> connex               Gumstix Connex (PXA255)
> verdex               Gumstix Verdex (PXA270)
> highbank             Calxeda Highbank (ECX-1000)
> midway               Calxeda Midway (ECX-2000)
> canon-a1100          Canon PowerShot A1100 IS
> integratorcp         ARM Integrator/CP (ARM926EJ-S)
> kzm                  ARM KZM Emulation Baseboard (ARM1136)
> mainstone            Mainstone II (PXA27x)
> musicpal             Marvell 88w8618 / MusicPal (ARM926EJ-S)
> n800                 Nokia N800 tablet aka. RX-34 (OMAP2420)
> n810                 Nokia N810 tablet aka. RX-44 (OMAP2420)
> sx1                  Siemens SX1 (OMAP310) V2
> sx1-v1               Siemens SX1 (OMAP310) V1
> cheetah              Palm Tungsten|E aka. Cheetah PDA (OMAP310)
> realview-eb          ARM RealView Emulation Baseboard (ARM926EJ-S)
> realview-eb-mpcore   ARM RealView Emulation Baseboard (ARM11MPCore)
> realview-pb-a8       ARM RealView Platform Baseboard for Cortex-A8
> realview-pbx-a9      ARM RealView Platform Baseboard Explore for Cortex-A9
> akita                Akita PDA (PXA270)
> spitz                Spitz PDA (PXA270)
> borzoi               Borzoi PDA (PXA270)
> terrier              Terrier PDA (PXA270)
> lm3s811evb           Stellaris LM3S811EVB
> lm3s6965evb          Stellaris LM3S6965EVB
> tosa                 Tosa PDA (PXA255)
> versatilepb          ARM Versatile/PB (ARM926EJ-S)
> versatileab          ARM Versatile/AB (ARM926EJ-S)
> vexpress-a9          ARM Versatile Express for Cortex-A9
> vexpress-a15         ARM Versatile Express for Cortex-A15
> virt                 ARM Virtual Machine
> xilinx-zynq-a9       Xilinx Zynq Platform Baseboard for Cortex-A9
> z2                   Zipit Z2 (PXA27x)
> cubieboard           cubietech cubieboard
> none                 empty machine

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1145042
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 vl.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Andreas Färber Sept. 22, 2014, 12:04 p.m. UTC | #1 
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.

> The GLib documentation is not overly clear if g_slist_sort() allocates a
> new list or reorders the input list in place, but:
> - it slightly suggests the latter,
> - and there is prior use in qemu that is consistent with the latter:
>   g_slist_sort() in qdev_print_devinfos() [qdev-monitor.c] sorts the
>   return value of object_class_get_list() the same as we do here in
>   machine_parse().

I believe I also assumed that for -cpu ? and/or QMP.

Regards,
Andreas
Michael S. Tsirkin Sept. 22, 2014, 12:26 p.m. UTC | #2 
On Mon, Sep 22, 2014 at 02:04:44PM +0200, 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.

Did you try?

Indeed for x86 sort -r on the list looks kind of ok:
q35                  Standard PC (Q35 + ICH9, 2009) (alias of pc-q35-2.1)
pc                   Standard PC (i440FX + PIIX, 1996) (alias of pc-i440fx-2.1)
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.1        Standard PC (i440FX + PIIX, 1996) (default)
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)
none                 empty machine
isapc                ISA-only PC

But this is just luck.
Not so on other targets. Laszlo mentions aarch:

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)


I guess it's almost reasonable if we special-case the empty
machine somehow, but we also need to sort numbers properly
a15 should be before a9?
Laszlo Ersek Sept. 22, 2014, 12:29 p.m. UTC | #3 
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.

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
Michael S. Tsirkin Sept. 22, 2014, 12:36 p.m. UTC | #4 
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
Marcel Apfelbaum Sept. 22, 2014, 12:50 p.m. UTC | #5 
On Mon, 2014-09-22 at 15:36 +0300, Michael S. Tsirkin wrote:
> 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?
Since the machines have now QOM types we can leverage the types
as "clusters", and each machine hierarchy can have its own
sorting interface to arrange their descendants however is desired.

It will look like:
1. Get all machine types that implement "sort" interface.
2. Define a sort order between them (a interface property ?)
3. Ask for each of them to sort its descendants.

In this way we have maximum control on the output and
give each cluster a proper way to decide the ordering
based on the semantics and not alphabetical/registering order. 

Thanks,
Marcel 

> 
> > 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
Laszlo Ersek Sept. 22, 2014, 1:15 p.m. UTC | #6 
On 09/22/14 14:36, Michael S. Tsirkin wrote:
> 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?

That's doable if you're okay with __FILE__ as the "cluster key". Because
that way the current registration calls can go through a macro that
substitutes the "cluster key" automatically.

Otherwise, all call sites would have to be updated manually, which is
not an inviting prospect:

$ git grep -E '\<qemu_register(_pc)?_machine\>' | wc -l
115

Even if we just count the clusters, they're way too many:

$ git grep -E '\<machine_init\>' | wc -l
66

Thanks
Laszlo
Laszlo Ersek Sept. 22, 2014, 1:43 p.m. UTC | #7 
On 09/22/14 14:50, Marcel Apfelbaum wrote:
> On Mon, 2014-09-22 at 15:36 +0300, Michael S. Tsirkin wrote:
>> 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?
> Since the machines have now QOM types we can leverage the types
> as "clusters", and each machine hierarchy can have its own
> sorting interface to arrange their descendants however is desired.
> 
> It will look like:
> 1. Get all machine types that implement "sort" interface.
> 2. Define a sort order between them (a interface property ?)
> 3. Ask for each of them to sort its descendants.
> 
> In this way we have maximum control on the output and
> give each cluster a proper way to decide the ordering
> based on the semantics and not alphabetical/registering order. 

Guys -- no offense meant, but this architecture astronautics &
bikeshedding is exactly why I, in my foresight, haven't assigned the
RHBZ in question to myself, and why I had unsubscribed from qemu-devel
months ago (or at least stopped mail delivery, can't recall exactly).

Here's the situation. Commit 261747f1 has broken some behavior of qemu
that is technically trivial / irrelevant, but a nuisance for users.
Noone to my knowledge had ever complained about the machtype listing
previously to commit 261747f1. Therefore the pre-261747f1 behavior is
reasonable and sound as a user-visible target to restore. The patchset I
posted is a technically simple solution that behaves identically to qemu
prior to 261747f1. The diffstat of the patchset is "20 insertions".

In the span of three followups in the thread, we've arrived at an
abstract sort interface, just so the user can see a more or less
traditionally ordered machtype list with -M '?'.

I think this is completely wrong; both specifically for the issue at
hand -- creeping object orientation -- and in general as a development /
community process  -- qemu is now basically untouchable for people who
want (or must) spend their time primarily on different things.

I agree that projects shouldn't take drive-by patches from irresponsible
contributors. At the same time I don't think I would qualify as one. My
patchset is not one that I throw over the wall. This is the solution
that I consider technically good, and what I have time for.

I guess it would be easier for me to understand this level of
nit-picking / over-engineering if my patchset ran a high risk of
regressions in some area. Whereas I'm only trying to undo an already
in-tree regression, as non-intrusively as possible

I only wanted to help out Marcel with RHBZ 1145042, because the breakage
in the visible machtype ordering is a consequence of his earlier patch,
and it seemed easy enough to address. If my patchset is not acceptable
for technical reasons, I can live with that (albeit in disagreement);
Marcel or someone else will hopefully fix the issue then.

Cheers
Laszlo
Marcel Apfelbaum Sept. 22, 2014, 3:07 p.m. UTC | #8 
On Mon, 2014-09-22 at 15:43 +0200, Laszlo Ersek wrote:
> On 09/22/14 14:50, Marcel Apfelbaum wrote:
> > On Mon, 2014-09-22 at 15:36 +0300, Michael S. Tsirkin wrote:
> >> 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?
> > Since the machines have now QOM types we can leverage the types
> > as "clusters", and each machine hierarchy can have its own
> > sorting interface to arrange their descendants however is desired.
> > 
> > It will look like:
> > 1. Get all machine types that implement "sort" interface.
> > 2. Define a sort order between them (a interface property ?)
> > 3. Ask for each of them to sort its descendants.
> > 
> > In this way we have maximum control on the output and
> > give each cluster a proper way to decide the ordering
> > based on the semantics and not alphabetical/registering order. 
> 
> Guys -- no offense meant, but this architecture astronautics &
> bikeshedding is exactly why I, in my foresight, haven't assigned the
> RHBZ in question to myself, and why I had unsubscribed from qemu-devel
> months ago (or at least stopped mail delivery, can't recall exactly).
> 
> Here's the situation. Commit 261747f1 has broken some behavior of qemu
> that is technically trivial / irrelevant, but a nuisance for users.
> Noone to my knowledge had ever complained about the machtype listing
> previously to commit 261747f1. Therefore the pre-261747f1 behavior is
> reasonable and sound as a user-visible target to restore. The patchset I
> posted is a technically simple solution that behaves identically to qemu
> prior to 261747f1. The diffstat of the patchset is "20 insertions".
> 
> In the span of three followups in the thread, we've arrived at an
> abstract sort interface, just so the user can see a more or less
> traditionally ordered machtype list with -M '?'.
> 
> I think this is completely wrong; both specifically for the issue at
> hand -- creeping object orientation -- and in general as a development /
> community process  -- qemu is now basically untouchable for people who
> want (or must) spend their time primarily on different things.
> 
> I agree that projects shouldn't take drive-by patches from irresponsible
> contributors. At the same time I don't think I would qualify as one. My
> patchset is not one that I throw over the wall. This is the solution
> that I consider technically good, and what I have time for.
> 
> I guess it would be easier for me to understand this level of
> nit-picking / over-engineering if my patchset ran a high risk of
> regressions in some area. Whereas I'm only trying to undo an already
> in-tree regression, as non-intrusively as possible
> 
> I only wanted to help out Marcel with RHBZ 1145042, because the breakage
> in the visible machtype ordering is a consequence of his earlier patch,
> and it seemed easy enough to address. If my patchset is not acceptable
> for technical reasons, I can live with that (albeit in disagreement);
> Marcel or someone else will hopefully fix the issue then.
Hi Laszlo,
This was only a suggestion, trying to discuss a possible "cluster" solution,
and nothing more.
It is not related to the patch submission which I find it reasonable
and I completely agree that it solves a regression introduced by my patches.

I'll be more careful in the feature to emphasize clearly when I am
discussing the patches and when I am only throwing ideas for others to comment.


Thanks,
Marcel

> 
> Cheers
> Laszlo
>
Paolo Bonzini Sept. 22, 2014, 3:57 p.m. UTC | #9 
Il 22/09/2014 15:15, Laszlo Ersek ha scritto:
> $ git grep -E '\<qemu_register(_pc)?_machine\>' | wc -l
> 115
> 
> Even if we just count the clusters, they're way too many:
> 
> $ git grep -E '\<machine_init\>' | wc -l
> 66

There are just a couple of multi-machine clusters (well, three: pseries,
pc, q35).  So the default clusters can just default to the machine type.

Paolo
Andreas Färber Sept. 22, 2014, 4:06 p.m. UTC | #10 
Am 22.09.2014 um 17:57 schrieb Paolo Bonzini:
> Il 22/09/2014 15:15, Laszlo Ersek ha scritto:
>> $ git grep -E '\<qemu_register(_pc)?_machine\>' | wc -l
>> 115
>>
>> Even if we just count the clusters, they're way too many:
>>
>> $ git grep -E '\<machine_init\>' | wc -l
>> 66
> 
> There are just a couple of multi-machine clusters (well, three: pseries,
> pc, q35).  So the default clusters can just default to the machine type.

I would've gone even simpler and special-cased pc/q35 and "none" (like
we did for -cpu "host") in the comparison function, sparing us any big
interface work.

Therefore from my point of review, this was merely about how we
implement the comparison function. No reason to get a heart attack IMO.

Cheers,
Andreas
Laszlo Ersek Sept. 22, 2014, 4:13 p.m. UTC | #11 
On 09/22/14 17:57, Paolo Bonzini wrote:
> Il 22/09/2014 15:15, Laszlo Ersek ha scritto:
>> $ git grep -E '\<qemu_register(_pc)?_machine\>' | wc -l
>> 115
>>
>> Even if we just count the clusters, they're way too many:
>>
>> $ git grep -E '\<machine_init\>' | wc -l
>> 66
> 
> There are just a couple of multi-machine clusters (well, three: pseries,
> pc, q35).  So the default clusters can just default to the machine type.

Thanks for the idea, I'll try to code this up.

Laszlo
Paolo Bonzini Sept. 22, 2014, 4:17 p.m. UTC | #12 
Il 22/09/2014 18:06, Andreas Färber ha scritto:
> Am 22.09.2014 um 17:57 schrieb Paolo Bonzini:
>> Il 22/09/2014 15:15, Laszlo Ersek ha scritto:
>>> $ git grep -E '\<qemu_register(_pc)?_machine\>' | wc -l
>>> 115
>>>
>>> Even if we just count the clusters, they're way too many:
>>>
>>> $ git grep -E '\<machine_init\>' | wc -l
>>> 66
>>
>> There are just a couple of multi-machine clusters (well, three: pseries,
>> pc, q35).  So the default clusters can just default to the machine type.
> 
> I would've gone even simpler and special-cased pc/q35 and "none" (like
> we did for -cpu "host") in the comparison function, sparing us any big
> interface work.
> 
> Therefore from my point of review, this was merely about how we
> implement the comparison function. No reason to get a heart attack IMO.

Yeah, what I'm proposing is something like

	MachineClass *a = pa, *b = pb;

	cl1 = a->cluster ? a->cluster : object_class_get_name(a);
	cl2 = b->cluster ? b->cluster : object_class_get_name(b);
	res = strcmp(cl1, cl2);
	if (res)
		return res;
	return strcmp(object_class_get_name(a),
		      object_class_get_name(b));

Plus a

	mc->cluster = qm->cluster;

in vl.c's machine_class_init.

Paolo
diff mbox

Patch

diff --git a/vl.c b/vl.c
index b62d9b2..00c7dab 100644
--- a/vl.c
+++ b/vl.c
@@ -2652,6 +2652,15 @@  static int debugcon_parse(const char *devname)
     return 0;
 }
 
+static gint machine_class_cmp(gconstpointer a, gconstpointer b)
+{
+    const MachineClass *mc1 = a, *mc2 = b;
+
+    return mc1->registration_order < mc2->registration_order ? -1 :
+           mc1->registration_order > mc2->registration_order ?  1 :
+           0;
+}
+
 static MachineClass *machine_parse(const char *name)
 {
     MachineClass *mc = NULL;
@@ -2668,6 +2677,7 @@  static MachineClass *machine_parse(const char *name)
         error_printf("Use -machine help to list supported machines!\n");
     } else {
         printf("Supported machines are:\n");
+        machines = g_slist_sort(machines, machine_class_cmp);
         for (el = machines; el; el = el->next) {
             MachineClass *mc = el->data;
             if (mc->alias) {