* [Qemu-devel] [PATCH 0/2] list supported machine types in their registration order
@ 2014-09-22 11:26 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
0 siblings, 2 replies; 15+ messages in thread
From: Laszlo Ersek @ 2014-09-22 11:26 UTC (permalink / raw)
To: qemu-devel, marcel.a, afaerber, mst, peter.maydell, lersek
The patch messages say it all.
Thanks,
Laszlo
Laszlo Ersek (2):
save registration order of machine types
machine_parse(): list supported machine types in their registration
order
include/hw/boards.h | 2 ++
include/sysemu/sysemu.h | 2 ++
hw/i386/pc.c | 2 ++
vl.c | 14 ++++++++++++++
4 files changed, 20 insertions(+)
--
1.8.3.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 1/2] save registration order of machine types 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 ` Laszlo Ersek 2014-09-22 11:26 ` [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order Laszlo Ersek 1 sibling, 0 replies; 15+ messages in thread From: Laszlo Ersek @ 2014-09-22 11:26 UTC (permalink / raw) To: qemu-devel, marcel.a, afaerber, mst, peter.maydell, lersek Commit 261747f1 ("vl: Use MachineClass instead of global QEMUMachine list") broke the ordering of the machine types in the user-visible output of qemu-system-XXXX -M \? This occurred because registration was rebased from a manually maintained linked list to GLib hash tables: qemu_register_machine() type_register() type_register_internal() type_table_add() g_hash_table_insert() and because the listing was rebased accordingly, from the traversal of the list to the traversal of the hash table (rendered as an ad-hoc list): machine_parse() object_class_get_list(TYPE_MACHINE) object_class_foreach() g_hash_table_foreach() The current order is a "random" one, for practical purposes, which is annoying for users. 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). The behavior prior to commit 261747f1 was that: (1) when a given function called qemu_register_machine() several times in sequence, such as in: machine_init(some_machine_init) some_machine_init() qemu_register_machine(...) qemu_register_machine(...) qemu_register_machine(...) then those registration calls influenced the relative order of those machine types directly, and (2) the ordering between functions passed to machine_init() was unspecified. We can restore this behavior by capturing the serial number of the qemu_register_machine() invocation in the MachineClass that it registers. RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1145042 Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- include/hw/boards.h | 2 ++ include/sysemu/sysemu.h | 2 ++ hw/i386/pc.c | 2 ++ vl.c | 4 ++++ 4 files changed, 10 insertions(+) diff --git a/include/hw/boards.h b/include/hw/boards.h index dfb6718..0665ca7 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -41,6 +41,7 @@ struct QEMUMachine { const char *default_boot_order; GlobalProperty *compat_props; const char *hw_version; + unsigned registration_order; }; void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, @@ -102,6 +103,7 @@ struct MachineClass { HotplugHandler *(*get_hotplug_handler)(MachineState *machine, DeviceState *dev); + unsigned registration_order; }; /** diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index d8539fd..5958b6b 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -24,6 +24,8 @@ int qemu_uuid_parse(const char *str, uint8_t *uuid); #define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx" #define UUID_NONE "00000000-0000-0000-0000-000000000000" +extern unsigned machtype_registration_order; + bool runstate_check(RunState state); void runstate_set(RunState new_state); int runstate_is_running(void); diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 2c2e9dc..95ccce3 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1537,6 +1537,7 @@ static void pc_generic_machine_class_init(ObjectClass *oc, void *data) mc->default_boot_order = qm->default_boot_order; mc->compat_props = qm->compat_props; mc->hw_version = qm->hw_version; + mc->registration_order = qm->registration_order; } void qemu_register_pc_machine(QEMUMachine *m) @@ -1549,6 +1550,7 @@ void qemu_register_pc_machine(QEMUMachine *m) .class_data = (void *)m, }; + m->registration_order = machtype_registration_order++; type_register(&ti); g_free(name); } diff --git a/vl.c b/vl.c index dc792fe..b62d9b2 100644 --- a/vl.c +++ b/vl.c @@ -1596,8 +1596,11 @@ static void machine_class_init(ObjectClass *oc, void *data) mc->default_boot_order = qm->default_boot_order; mc->compat_props = qm->compat_props; mc->hw_version = qm->hw_version; + mc->registration_order = qm->registration_order; } +unsigned machtype_registration_order; + int qemu_register_machine(QEMUMachine *m) { char *name = g_strconcat(m->name, TYPE_MACHINE_SUFFIX, NULL); @@ -1608,6 +1611,7 @@ int qemu_register_machine(QEMUMachine *m) .class_data = (void *)m, }; + m->registration_order = machtype_registration_order++; type_register(&ti); g_free(name); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order 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 ` Laszlo Ersek 2014-09-22 12:04 ` Andreas Färber 1 sibling, 1 reply; 15+ messages in thread From: Laszlo Ersek @ 2014-09-22 11:26 UTC (permalink / raw) To: qemu-devel, marcel.a, afaerber, mst, peter.maydell, lersek 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(+) 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) { -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order 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 0 siblings, 2 replies; 15+ messages in thread From: Andreas Färber @ 2014-09-22 12:04 UTC (permalink / raw) To: Laszlo Ersek, qemu-devel, marcel.a, mst, peter.maydell 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 -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order 2014-09-22 12:04 ` Andreas Färber @ 2014-09-22 12:26 ` Michael S. Tsirkin 2014-09-22 12:29 ` Laszlo Ersek 1 sibling, 0 replies; 15+ messages in thread From: Michael S. Tsirkin @ 2014-09-22 12:26 UTC (permalink / raw) To: Andreas Färber; +Cc: peter.maydell, Laszlo Ersek, qemu-devel, marcel.a 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? -- MST ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order 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 1 sibling, 1 reply; 15+ messages in thread From: Laszlo Ersek @ 2014-09-22 12:29 UTC (permalink / raw) To: Andreas Färber, qemu-devel, marcel.a, mst, peter.maydell 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order 2014-09-22 12:29 ` Laszlo Ersek @ 2014-09-22 12:36 ` Michael S. Tsirkin 2014-09-22 12:50 ` Marcel Apfelbaum 2014-09-22 13:15 ` Laszlo Ersek 0 siblings, 2 replies; 15+ messages in thread From: Michael S. Tsirkin @ 2014-09-22 12:36 UTC (permalink / raw) To: Laszlo Ersek; +Cc: peter.maydell, marcel.a, Andreas Färber, qemu-devel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order 2014-09-22 12:36 ` Michael S. Tsirkin @ 2014-09-22 12:50 ` Marcel Apfelbaum 2014-09-22 13:43 ` Laszlo Ersek 2014-09-22 13:15 ` Laszlo Ersek 1 sibling, 1 reply; 15+ messages in thread From: Marcel Apfelbaum @ 2014-09-22 12:50 UTC (permalink / raw) To: Michael S. Tsirkin Cc: peter.maydell, Laszlo Ersek, Andreas Färber, qemu-devel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order 2014-09-22 12:50 ` Marcel Apfelbaum @ 2014-09-22 13:43 ` Laszlo Ersek 2014-09-22 15:07 ` Marcel Apfelbaum 0 siblings, 1 reply; 15+ messages in thread From: Laszlo Ersek @ 2014-09-22 13:43 UTC (permalink / raw) To: Marcel Apfelbaum, Michael S. Tsirkin Cc: peter.maydell, Andreas Färber, qemu-devel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order 2014-09-22 13:43 ` Laszlo Ersek @ 2014-09-22 15:07 ` Marcel Apfelbaum 0 siblings, 0 replies; 15+ messages in thread From: Marcel Apfelbaum @ 2014-09-22 15:07 UTC (permalink / raw) To: Laszlo Ersek Cc: qemu-devel, peter.maydell, Andreas Färber, Michael S. Tsirkin 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 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order 2014-09-22 12:36 ` Michael S. Tsirkin 2014-09-22 12:50 ` Marcel Apfelbaum @ 2014-09-22 13:15 ` Laszlo Ersek 2014-09-22 15:57 ` Paolo Bonzini 1 sibling, 1 reply; 15+ messages in thread From: Laszlo Ersek @ 2014-09-22 13:15 UTC (permalink / raw) To: Michael S. Tsirkin Cc: peter.maydell, marcel.a, Andreas Färber, qemu-devel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order 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:13 ` Laszlo Ersek 0 siblings, 2 replies; 15+ messages in thread From: Paolo Bonzini @ 2014-09-22 15:57 UTC (permalink / raw) To: Laszlo Ersek, Michael S. Tsirkin Cc: qemu-devel, peter.maydell, Andreas Färber, marcel.a 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order 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 1 sibling, 1 reply; 15+ messages in thread From: Andreas Färber @ 2014-09-22 16:06 UTC (permalink / raw) To: Paolo Bonzini, Laszlo Ersek, Michael S. Tsirkin Cc: peter.maydell, qemu-devel, marcel.a 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 -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order 2014-09-22 16:06 ` Andreas Färber @ 2014-09-22 16:17 ` Paolo Bonzini 0 siblings, 0 replies; 15+ messages in thread From: Paolo Bonzini @ 2014-09-22 16:17 UTC (permalink / raw) To: Andreas Färber, Laszlo Ersek, Michael S. Tsirkin Cc: peter.maydell, qemu-devel, marcel.a 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] machine_parse(): list supported machine types in their registration order 2014-09-22 15:57 ` Paolo Bonzini 2014-09-22 16:06 ` Andreas Färber @ 2014-09-22 16:13 ` Laszlo Ersek 1 sibling, 0 replies; 15+ messages in thread From: Laszlo Ersek @ 2014-09-22 16:13 UTC (permalink / raw) To: Paolo Bonzini, Michael S. Tsirkin Cc: qemu-devel, peter.maydell, Andreas Färber, marcel.a 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2014-09-22 16:17 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 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
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).