[Qemu-devel] [PATCH v3 0/6] Clean up bogus default boot order

[Qemu-devel] [PATCH v3 0/6] Clean up bogus default boot order

[Markus Armbruster]

The first five patches are admittedly related to the stated purpose of
this series pretty much only by "I can't stand perpetuating this
stupid crap".  Max Filippov and Peter Maydell already cleaned up
Xtensa and ARM the same way.

v3:
* Rebase
* Don't screw up default CPU model for machine "isapc" in 1/6
v2:
* Straightforward rebase
* PATCH 6 improved commit message (Alex)

Markus Armbruster (6):
  pc: Don't prematurely explode QEMUMachineInitArgs
  pc: Don't explode QEMUMachineInitArgs into local variables needlessly
  sun4: Don't prematurely explode QEMUMachineInitArgs
  ppc: Don't explode QEMUMachineInitArgs into local variables
    needlessly
  ppc: Don't duplicate QEMUMachineInitArgs in PPCE500Params
  hw: Clean up bogus default boot order

 hw/alpha/dp264.c                         |   1 -
 hw/arm/collie.c                          |   1 -
 hw/arm/exynos4_boards.c                  |   2 -
 hw/arm/gumstix.c                         |   2 -
 hw/arm/highbank.c                        |   2 -
 hw/arm/integratorcp.c                    |   1 -
 hw/arm/kzm.c                             |   1 -
 hw/arm/mainstone.c                       |   1 -
 hw/arm/musicpal.c                        |   1 -
 hw/arm/nseries.c                         |   6 +-
 hw/arm/omap_sx1.c                        |   2 -
 hw/arm/palm.c                            |   1 -
 hw/arm/realview.c                        |   4 -
 hw/arm/spitz.c                           |   4 -
 hw/arm/stellaris.c                       |   2 -
 hw/arm/tosa.c                            |   1 -
 hw/arm/versatilepb.c                     |   2 -
 hw/arm/vexpress.c                        |   2 -
 hw/arm/xilinx_zynq.c                     |   1 -
 hw/arm/z2.c                              |   1 -
 hw/core/null-machine.c                   |   1 -
 hw/cris/axis_dev88.c                     |   1 -
 hw/i386/pc_piix.c                        | 102 +++++++++---------------
 hw/i386/pc_q35.c                         |  28 +++----
 hw/i386/xen_machine_pv.c                 |   1 -
 hw/lm32/lm32_boards.c                    |   2 -
 hw/lm32/milkymist.c                      |   1 -
 hw/m68k/an5206.c                         |   1 -
 hw/m68k/dummy_m68k.c                     |   1 -
 hw/m68k/mcf5208.c                        |   1 -
 hw/microblaze/petalogix_ml605_mmu.c      |   1 -
 hw/microblaze/petalogix_s3adsp1800_mmu.c |   1 -
 hw/mips/mips_fulong2e.c                  |   1 -
 hw/mips/mips_jazz.c                      |   2 -
 hw/mips/mips_malta.c                     |   1 -
 hw/mips/mips_mipssim.c                   |   1 -
 hw/mips/mips_r4k.c                       |   1 -
 hw/openrisc/openrisc_sim.c               |   1 -
 hw/ppc/e500.c                            |  35 +++++----
 hw/ppc/e500.h                            |  13 +--
 hw/ppc/e500plat.c                        |  15 +---
 hw/ppc/mac_newworld.c                    |   4 +-
 hw/ppc/mac_oldworld.c                    |   4 +-
 hw/ppc/mpc8544ds.c                       |  15 +---
 hw/ppc/ppc405_boards.c                   |   2 -
 hw/ppc/ppc440_bamboo.c                   |   1 -
 hw/ppc/prep.c                            |   4 +-
 hw/ppc/spapr.c                           |   4 +-
 hw/ppc/virtex_ml507.c                    |   1 -
 hw/s390x/s390-virtio-ccw.c               |   1 -
 hw/s390x/s390-virtio.c                   |   1 -
 hw/sh4/r2d.c                             |   1 -
 hw/sh4/shix.c                            |   1 -
 hw/sparc/leon3.c                         |   1 -
 hw/sparc/sun4m.c                         | 131 ++++++++-----------------------
 hw/sparc64/sun4u.c                       |  58 +++++---------
 hw/unicore32/puv3.c                      |   1 -
 hw/xtensa/xtensa_lx60.c                  |   2 -
 hw/xtensa/xtensa_sim.c                   |   1 -
 include/hw/boards.h                      |   7 +-
 vl.c                                     |   4 +-
 61 files changed, 137 insertions(+), 356 deletions(-)

-- 
1.7.11.7

[Qemu-devel] [PATCH v3 1/6] pc: Don't prematurely explode QEMUMachineInitArgs

[Markus Armbruster]

Don't explode QEMUMachineInitArgs before passing it to pc_init1().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/i386/pc_piix.c | 72 ++++++++++++++++---------------------------------------
 1 file changed, 20 insertions(+), 52 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index b58c255..9327ac1 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -60,14 +60,9 @@ static bool has_pvpanic = true;
 static bool has_pci_info = true;
 
 /* PC hardware initialisation */
-static void pc_init1(MemoryRegion *system_memory,
+static void pc_init1(QEMUMachineInitArgs *args,
+                     MemoryRegion *system_memory,
                      MemoryRegion *system_io,
-                     ram_addr_t ram_size,
-                     const char *boot_device,
-                     const char *kernel_filename,
-                     const char *kernel_cmdline,
-                     const char *initrd_filename,
-                     const char *cpu_model,
                      int pci_enabled,
                      int kvmclock_enabled)
 {
@@ -102,19 +97,19 @@ static void pc_init1(MemoryRegion *system_memory,
     object_property_add_child(qdev_get_machine(), "icc-bridge",
                               OBJECT(icc_bridge), NULL);
 
-    pc_cpus_init(cpu_model, icc_bridge);
+    pc_cpus_init(args->cpu_model, icc_bridge);
     pc_acpi_init("acpi-dsdt.aml");
 
     if (kvm_enabled() && kvmclock_enabled) {
         kvmclock_create();
     }
 
-    if (ram_size >= 0xe0000000 ) {
-        above_4g_mem_size = ram_size - 0xe0000000;
+    if (args->ram_size >= 0xe0000000) {
+        above_4g_mem_size = args->ram_size - 0xe0000000;
         below_4g_mem_size = 0xe0000000;
     } else {
         above_4g_mem_size = 0;
-        below_4g_mem_size = ram_size;
+        below_4g_mem_size = args->ram_size;
     }
 
     if (pci_enabled) {
@@ -131,17 +126,19 @@ static void pc_init1(MemoryRegion *system_memory,
 
     /* Set PCI window size the way seabios has always done it. */
     /* Power of 2 so bios can cover it with a single MTRR */
-    if (ram_size <= 0x80000000)
+    if (args->ram_size <= 0x80000000) {
         guest_info->pci_info.w32.begin = 0x80000000;
-    else if (ram_size <= 0xc0000000)
+    } else if (args->ram_size <= 0xc0000000) {
         guest_info->pci_info.w32.begin = 0xc0000000;
-    else
+    } else {
         guest_info->pci_info.w32.begin = 0xe0000000;
+    }
 
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
         fw_cfg = pc_memory_init(system_memory,
-                       kernel_filename, kernel_cmdline, initrd_filename,
+                       args->kernel_filename, args->kernel_cmdline,
+                       args->initrd_filename,
                        below_4g_mem_size, above_4g_mem_size,
                        rom_memory, &ram_memory, guest_info);
     }
@@ -157,7 +154,7 @@ static void pc_init1(MemoryRegion *system_memory,
 
     if (pci_enabled) {
         pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
-                              system_memory, system_io, ram_size,
+                              system_memory, system_io, args->ram_size,
                               below_4g_mem_size,
                               0x100000000ULL - below_4g_mem_size,
                               0x100000000ULL + above_4g_mem_size,
@@ -219,7 +216,7 @@ static void pc_init1(MemoryRegion *system_memory,
         }
     }
 
-    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device,
+    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
                  floppy, idebus[0], idebus[1], rtc_state);
 
     if (pci_enabled && usb_enabled(false)) {
@@ -248,17 +245,7 @@ static void pc_init1(MemoryRegion *system_memory,
 
 static void pc_init_pci(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
-    pc_init1(get_system_memory(),
-             get_system_io(),
-             ram_size, boot_device,
-             kernel_filename, kernel_cmdline,
-             initrd_filename, cpu_model, 1, 1);
+    pc_init1(args, get_system_memory(), get_system_io(), 1, 1);
 }
 
 static void pc_init_pci_1_5(QEMUMachineInitArgs *args)
@@ -296,42 +283,23 @@ static void pc_init_pci_1_0(QEMUMachineInitArgs *args)
 /* PC init function for pc-0.10 to pc-0.13, and reused by xenfv */
 static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
     has_pvpanic = false;
     has_pci_info = false;
     disable_kvm_pv_eoi();
     enable_compat_apic_id_mode();
-    pc_init1(get_system_memory(),
-             get_system_io(),
-             ram_size, boot_device,
-             kernel_filename, kernel_cmdline,
-             initrd_filename, cpu_model, 1, 0);
+    pc_init1(args, get_system_memory(), get_system_io(), 1, 0);
 }
 
 static void pc_init_isa(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
     has_pvpanic = false;
     has_pci_info = false;
-    if (cpu_model == NULL)
-        cpu_model = "486";
+    if (!args->cpu_model) {
+        args->cpu_model = "486";
+    }
     disable_kvm_pv_eoi();
     enable_compat_apic_id_mode();
-    pc_init1(get_system_memory(),
-             get_system_io(),
-             ram_size, boot_device,
-             kernel_filename, kernel_cmdline,
-             initrd_filename, cpu_model, 0, 1);
+    pc_init1(args, get_system_memory(), get_system_io(), 0, 1);
 }
 
 #ifdef CONFIG_XEN
-- 
1.7.11.7

[Qemu-devel] [PATCH v3 2/6] pc: Don't explode QEMUMachineInitArgs into local variables needlessly

[Markus Armbruster]

Don't explode when the variable is used just a few times, and never
changed.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/i386/pc_q35.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 6f10246..1657eb3 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -52,12 +52,6 @@ static bool has_pci_info = true;
 /* PC hardware initialisation */
 static void pc_q35_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
     ram_addr_t below_4g_mem_size, above_4g_mem_size;
     Q35PCIHost *q35_host;
     PCIBus *host_bus;
@@ -84,17 +78,17 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     object_property_add_child(qdev_get_machine(), "icc-bridge",
                               OBJECT(icc_bridge), NULL);
 
-    pc_cpus_init(cpu_model, icc_bridge);
+    pc_cpus_init(args->cpu_model, icc_bridge);
     pc_acpi_init("q35-acpi-dsdt.aml");
 
     kvmclock_create();
 
-    if (ram_size >= 0xb0000000) {
-        above_4g_mem_size = ram_size - 0xb0000000;
+    if (args->ram_size >= 0xb0000000) {
+        above_4g_mem_size = args->ram_size - 0xb0000000;
         below_4g_mem_size = 0xb0000000;
     } else {
         above_4g_mem_size = 0;
-        below_4g_mem_size = ram_size;
+        below_4g_mem_size = args->ram_size;
     }
 
     /* pci enabled */
@@ -112,8 +106,10 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
-        pc_memory_init(get_system_memory(), kernel_filename, kernel_cmdline,
-                       initrd_filename, below_4g_mem_size, above_4g_mem_size,
+        pc_memory_init(get_system_memory(),
+                       args->kernel_filename, args->kernel_cmdline,
+                       args->initrd_filename,
+                       below_4g_mem_size, above_4g_mem_size,
                        rom_memory, &ram_memory, guest_info);
     }
 
@@ -199,7 +195,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
                                     0xb100),
                       8, NULL, 0);
 
-    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device,
+    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
                  floppy, idebus[0], idebus[1], rtc_state);
 
     /* the rest devices to which pci devfn is automatically assigned */
-- 
1.7.11.7

[Qemu-devel] [PATCH v3 3/6] sun4: Don't prematurely explode QEMUMachineInitArgs

[Markus Armbruster]

Don't explode QEMUMachineInitArgs before passing it to
sun4m_hw_init(), sun4uv_init().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/sparc/sun4m.c   | 113 ++++++++++++-----------------------------------------
 hw/sparc64/sun4u.c |  52 +++++++-----------------
 2 files changed, 40 insertions(+), 125 deletions(-)

diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 7a0c1ab..97c3ee0 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -816,12 +816,10 @@ static void dummy_fdc_tc(void *opaque, int irq, int level)
 {
 }
 
-static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
-                          const char *boot_device,
-                          const char *kernel_filename,
-                          const char *kernel_cmdline,
-                          const char *initrd_filename, const char *cpu_model)
+static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
+                          QEMUMachineInitArgs *args)
 {
+    const char *cpu_model = args->cpu_model;
     unsigned int i;
     void *iommu, *espdma, *ledma, *nvram;
     qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS],
@@ -847,10 +845,10 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
 
 
     /* set up devices */
-    ram_init(0, RAM_size, hwdef->max_mem);
+    ram_init(0, args->ram_size, hwdef->max_mem);
     /* models without ECC don't trap when missing ram is accessed */
     if (!hwdef->ecc_base) {
-        empty_slot_init(RAM_size, hwdef->max_mem - RAM_size);
+        empty_slot_init(args->ram_size, hwdef->max_mem - args->ram_size);
     }
 
     prom_init(hwdef->slavio_base, bios_name);
@@ -973,11 +971,12 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
         empty_slot_init(hwdef->bpp_base, 0x20);
     }
 
-    kernel_size = sun4m_load_kernel(kernel_filename, initrd_filename,
-                                    RAM_size);
+    kernel_size = sun4m_load_kernel(args->kernel_filename,
+                                    args->initrd_filename,
+                                    args->ram_size);
 
-    nvram_init(nvram, (uint8_t *)&nd_table[0].macaddr, kernel_cmdline,
-               boot_device, RAM_size, kernel_size, graphic_width,
+    nvram_init(nvram, (uint8_t *)&nd_table[0].macaddr, args->kernel_cmdline,
+               args->boot_device, args->ram_size, kernel_size, graphic_width,
                graphic_height, graphic_depth, hwdef->nvram_machine_id,
                "Sun4m");
 
@@ -993,19 +992,20 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
     fw_cfg_add_i16(fw_cfg, FW_CFG_SUN4M_DEPTH, graphic_depth);
     fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, KERNEL_LOAD_ADDR);
     fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
-    if (kernel_cmdline) {
+    if (args->kernel_cmdline) {
         fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR);
-        pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE, kernel_cmdline);
-        fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline);
+        pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE,
+                         args->kernel_cmdline);
+        fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, args->kernel_cmdline);
         fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
-                       strlen(kernel_cmdline) + 1);
+                       strlen(args->kernel_cmdline) + 1);
     } else {
         fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
         fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, 0);
     }
     fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
     fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, 0); // not used
-    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, boot_device[0]);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_device[0]);
     qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
 }
 
@@ -1269,118 +1269,55 @@ static const struct sun4m_hwdef sun4m_hwdefs[] = {
 /* SPARCstation 5 hardware initialisation */
 static void ss5_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t RAM_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
-    sun4m_hw_init(&sun4m_hwdefs[0], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[0], args);
 }
 
 /* SPARCstation 10 hardware initialisation */
 static void ss10_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t RAM_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
-    sun4m_hw_init(&sun4m_hwdefs[1], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[1], args);
 }
 
 /* SPARCserver 600MP hardware initialisation */
 static void ss600mp_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t RAM_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
-    sun4m_hw_init(&sun4m_hwdefs[2], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[2], args);
 }
 
 /* SPARCstation 20 hardware initialisation */
 static void ss20_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t RAM_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
-    sun4m_hw_init(&sun4m_hwdefs[3], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[3], args);
 }
 
 /* SPARCstation Voyager hardware initialisation */
 static void vger_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t RAM_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
-    sun4m_hw_init(&sun4m_hwdefs[4], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[4], args);
 }
 
 /* SPARCstation LX hardware initialisation */
 static void ss_lx_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t RAM_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
-    sun4m_hw_init(&sun4m_hwdefs[5], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[5], args);
 }
 
 /* SPARCstation 4 hardware initialisation */
 static void ss4_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t RAM_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
-    sun4m_hw_init(&sun4m_hwdefs[6], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[6], args);
 }
 
 /* SPARCClassic hardware initialisation */
 static void scls_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t RAM_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
-    sun4m_hw_init(&sun4m_hwdefs[7], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[7], args);
 }
 
 /* SPARCbook hardware initialisation */
 static void sbook_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t RAM_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
-    sun4m_hw_init(&sun4m_hwdefs[8], RAM_size, boot_device, kernel_filename,
-                  kernel_cmdline, initrd_filename, cpu_model);
+    sun4m_hw_init(&sun4m_hwdefs[8], args);
 }
 
 static QEMUMachine ss5_machine = {
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 564c195..22e9818 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -802,10 +802,7 @@ static SPARCCPU *cpu_devinit(const char *cpu_model, const struct hwdef *hwdef)
 }
 
 static void sun4uv_init(MemoryRegion *address_space_mem,
-                        ram_addr_t RAM_size,
-                        const char *boot_devices,
-                        const char *kernel_filename, const char *kernel_cmdline,
-                        const char *initrd_filename, const char *cpu_model,
+                        QEMUMachineInitArgs *args,
                         const struct hwdef *hwdef)
 {
     SPARCCPU *cpu;
@@ -820,10 +817,10 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     FWCfgState *fw_cfg;
 
     /* init CPUs */
-    cpu = cpu_devinit(cpu_model, hwdef);
+    cpu = cpu_devinit(args->cpu_model, hwdef);
 
     /* set up devices */
-    ram_init(0, RAM_size);
+    ram_init(0, args->ram_size);
 
     prom_init(hwdef->prom_addr, bios_name);
 
@@ -869,13 +866,15 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
 
     initrd_size = 0;
     initrd_addr = 0;
-    kernel_size = sun4u_load_kernel(kernel_filename, initrd_filename,
+    kernel_size = sun4u_load_kernel(args->kernel_filename,
+                                    args->initrd_filename,
                                     ram_size, &initrd_size, &initrd_addr,
                                     &kernel_addr, &kernel_entry);
 
-    sun4u_NVRAM_set_params(nvram, NVRAM_SIZE, "Sun4u", RAM_size, boot_devices,
+    sun4u_NVRAM_set_params(nvram, NVRAM_SIZE, "Sun4u", args->ram_size,
+                           args->boot_device,
                            kernel_addr, kernel_size,
-                           kernel_cmdline,
+                           args->kernel_cmdline,
                            initrd_addr, initrd_size,
                            /* XXX: need an option to load a NVRAM image */
                            0,
@@ -889,16 +888,16 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
     fw_cfg_add_i64(fw_cfg, FW_CFG_KERNEL_ADDR, kernel_entry);
     fw_cfg_add_i64(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
-    if (kernel_cmdline) {
+    if (args->kernel_cmdline) {
         fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
-                       strlen(kernel_cmdline) + 1);
-        fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline);
+                       strlen(args->kernel_cmdline) + 1);
+        fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, args->kernel_cmdline);
     } else {
         fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, 0);
     }
     fw_cfg_add_i64(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
     fw_cfg_add_i64(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, boot_devices[0]);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_device[0]);
 
     fw_cfg_add_i16(fw_cfg, FW_CFG_SPARC64_WIDTH, graphic_width);
     fw_cfg_add_i16(fw_cfg, FW_CFG_SPARC64_HEIGHT, graphic_height);
@@ -940,40 +939,19 @@ static const struct hwdef hwdefs[] = {
 /* Sun4u hardware initialisation */
 static void sun4u_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t RAM_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_devices = args->boot_device;
-    sun4uv_init(get_system_memory(), RAM_size, boot_devices, kernel_filename,
-                kernel_cmdline, initrd_filename, cpu_model, &hwdefs[0]);
+    sun4uv_init(get_system_memory(), args, &hwdefs[0]);
 }
 
 /* Sun4v hardware initialisation */
 static void sun4v_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t RAM_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_devices = args->boot_device;
-    sun4uv_init(get_system_memory(), RAM_size, boot_devices, kernel_filename,
-                kernel_cmdline, initrd_filename, cpu_model, &hwdefs[1]);
+    sun4uv_init(get_system_memory(), args, &hwdefs[1]);
 }
 
 /* Niagara hardware initialisation */
 static void niagara_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t RAM_size = args->ram_size;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
-    const char *boot_devices = args->boot_device;
-    sun4uv_init(get_system_memory(), RAM_size, boot_devices, kernel_filename,
-                kernel_cmdline, initrd_filename, cpu_model, &hwdefs[2]);
+    sun4uv_init(get_system_memory(), args, &hwdefs[2]);
 }
 
 static QEMUMachine sun4u_machine = {
-- 
1.7.11.7

[Qemu-devel] [PATCH v3 4/6] ppc: Don't explode QEMUMachineInitArgs into local variables needlessly

[Markus Armbruster]

Don't explode when the variable is used just once, and never changed.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Alexander Graf <agraf@suse.de>
---
 hw/ppc/e500plat.c  | 18 ++++++------------
 hw/ppc/mpc8544ds.c | 18 ++++++------------
 2 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index c852995..a78de07 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -30,19 +30,13 @@ static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
 
 static void e500plat_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *boot_device = args->boot_device;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
     PPCE500Params params = {
-        .ram_size = ram_size,
-        .boot_device = boot_device,
-        .kernel_filename = kernel_filename,
-        .kernel_cmdline = kernel_cmdline,
-        .initrd_filename = initrd_filename,
-        .cpu_model = cpu_model,
+        .ram_size = args->ram_size,
+        .boot_device = args->boot_device,
+        .kernel_filename = args->kernel_filename,
+        .kernel_cmdline = args->kernel_cmdline,
+        .initrd_filename = args->initrd_filename,
+        .cpu_model = args->cpu_model,
         .pci_first_slot = 0x1,
         .pci_nr_slots = PCI_SLOT_MAX - 1,
         .fixup_devtree = e500plat_fixup_devtree,
diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
index 444da02..4e551af 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -28,19 +28,13 @@ static void mpc8544ds_fixup_devtree(PPCE500Params *params, void *fdt)
 
 static void mpc8544ds_init(QEMUMachineInitArgs *args)
 {
-    ram_addr_t ram_size = args->ram_size;
-    const char *boot_device = args->boot_device;
-    const char *cpu_model = args->cpu_model;
-    const char *kernel_filename = args->kernel_filename;
-    const char *kernel_cmdline = args->kernel_cmdline;
-    const char *initrd_filename = args->initrd_filename;
     PPCE500Params params = {
-        .ram_size = ram_size,
-        .boot_device = boot_device,
-        .kernel_filename = kernel_filename,
-        .kernel_cmdline = kernel_cmdline,
-        .initrd_filename = initrd_filename,
-        .cpu_model = cpu_model,
+        .ram_size = args->ram_size,
+        .boot_device = args->boot_device,
+        .kernel_filename = args->kernel_filename,
+        .kernel_cmdline = args->kernel_cmdline,
+        .initrd_filename = args->initrd_filename,
+        .cpu_model = args->cpu_model,
         .pci_first_slot = 0x11,
         .pci_nr_slots = 2,
         .fixup_devtree = mpc8544ds_fixup_devtree,
-- 
1.7.11.7

[Qemu-devel] [PATCH v3 5/6] ppc: Don't duplicate QEMUMachineInitArgs in PPCE500Params

[Markus Armbruster]

Pass on the generic arguments unadulterated, and the machine-specific
ones as separate argument.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Alexander Graf <agraf@suse.de>
---
 hw/ppc/e500.c      | 35 ++++++++++++++++++-----------------
 hw/ppc/e500.h      | 13 +++----------
 hw/ppc/e500plat.c  |  8 +-------
 hw/ppc/mpc8544ds.c |  8 +-------
 4 files changed, 23 insertions(+), 41 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index f00a62a..e79612b 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -124,13 +124,14 @@ static void dt_serial_create(void *fdt, unsigned long long offset,
 }
 
 static int ppce500_load_device_tree(CPUPPCState *env,
+                                    QEMUMachineInitArgs *args,
                                     PPCE500Params *params,
                                     hwaddr addr,
                                     hwaddr initrd_base,
                                     hwaddr initrd_size)
 {
     int ret = -1;
-    uint64_t mem_reg_property[] = { 0, cpu_to_be64(params->ram_size) };
+    uint64_t mem_reg_property[] = { 0, cpu_to_be64(args->ram_size) };
     int fdt_size;
     void *fdt;
     uint8_t hypercall[16];
@@ -205,7 +206,7 @@ static int ppce500_load_device_tree(CPUPPCState *env,
     }
 
     ret = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
-                                      params->kernel_cmdline);
+                                      args->kernel_cmdline);
     if (ret < 0)
         fprintf(stderr, "couldn't set /chosen/bootargs\n");
 
@@ -559,7 +560,7 @@ static qemu_irq *ppce500_init_mpic(PPCE500Params *params, MemoryRegion *ccsr,
     return mpic;
 }
 
-void ppce500_init(PPCE500Params *params)
+void ppce500_init(QEMUMachineInitArgs *args, PPCE500Params *params)
 {
     MemoryRegion *address_space_mem = get_system_memory();
     MemoryRegion *ram = g_new(MemoryRegion, 1);
@@ -584,8 +585,8 @@ void ppce500_init(PPCE500Params *params)
     PPCE500CCSRState *ccsr;
 
     /* Setup CPUs */
-    if (params->cpu_model == NULL) {
-        params->cpu_model = "e500v2_v30";
+    if (args->cpu_model == NULL) {
+        args->cpu_model = "e500v2_v30";
     }
 
     irqs = g_malloc0(smp_cpus * sizeof(qemu_irq *));
@@ -595,7 +596,7 @@ void ppce500_init(PPCE500Params *params)
         CPUState *cs;
         qemu_irq *input;
 
-        cpu = cpu_ppc_init(params->cpu_model);
+        cpu = cpu_ppc_init(args->cpu_model);
         if (cpu == NULL) {
             fprintf(stderr, "Unable to initialize CPU!\n");
             exit(1);
@@ -634,7 +635,7 @@ void ppce500_init(PPCE500Params *params)
 
     /* Fixup Memory size on a alignment boundary */
     ram_size &= ~(RAM_SIZES_ALIGN - 1);
-    params->ram_size = ram_size;
+    args->ram_size = ram_size;
 
     /* Register Memory */
     memory_region_init_ram(ram, NULL, "mpc8544ds.ram", ram_size);
@@ -701,11 +702,11 @@ void ppce500_init(PPCE500Params *params)
     sysbus_create_simple("e500-spin", MPC8544_SPIN_BASE, NULL);
 
     /* Load kernel. */
-    if (params->kernel_filename) {
-        kernel_size = load_uimage(params->kernel_filename, &entry,
+    if (args->kernel_filename) {
+        kernel_size = load_uimage(args->kernel_filename, &entry,
                                   &loadaddr, NULL);
         if (kernel_size < 0) {
-            kernel_size = load_elf(params->kernel_filename, NULL, NULL,
+            kernel_size = load_elf(args->kernel_filename, NULL, NULL,
                                    &elf_entry, &elf_lowaddr, NULL, 1,
                                    ELF_MACHINE, 0);
             entry = elf_entry;
@@ -714,7 +715,7 @@ void ppce500_init(PPCE500Params *params)
         /* XXX try again as binary */
         if (kernel_size < 0) {
             fprintf(stderr, "qemu: could not load kernel '%s'\n",
-                    params->kernel_filename);
+                    args->kernel_filename);
             exit(1);
         }
 
@@ -726,14 +727,14 @@ void ppce500_init(PPCE500Params *params)
     }
 
     /* Load initrd. */
-    if (params->initrd_filename) {
+    if (args->initrd_filename) {
         initrd_base = (cur_base + INITRD_LOAD_PAD) & ~INITRD_PAD_MASK;
-        initrd_size = load_image_targphys(params->initrd_filename, initrd_base,
+        initrd_size = load_image_targphys(args->initrd_filename, initrd_base,
                                           ram_size - initrd_base);
 
         if (initrd_size < 0) {
             fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
-                    params->initrd_filename);
+                    args->initrd_filename);
             exit(1);
         }
 
@@ -741,12 +742,12 @@ void ppce500_init(PPCE500Params *params)
     }
 
     /* If we're loading a kernel directly, we must load the device tree too. */
-    if (params->kernel_filename) {
+    if (args->kernel_filename) {
         struct boot_info *boot_info;
         int dt_size;
 
-        dt_size = ppce500_load_device_tree(env, params, dt_base, initrd_base,
-                                           initrd_size);
+        dt_size = ppce500_load_device_tree(env, args, params, dt_base,
+                                           initrd_base, initrd_size);
         if (dt_size < 0) {
             fprintf(stderr, "couldn't load device tree\n");
             exit(1);
diff --git a/hw/ppc/e500.h b/hw/ppc/e500.h
index 226c93d..52726a2 100644
--- a/hw/ppc/e500.h
+++ b/hw/ppc/e500.h
@@ -1,25 +1,18 @@
 #ifndef PPCE500_H
 #define PPCE500_H
 
+#include "hw/boards.h"
+
 typedef struct PPCE500Params {
-    /* Standard QEMU machine init params */
-    ram_addr_t ram_size;
-    const char *boot_device;
-    const char *kernel_filename;
-    const char *kernel_cmdline;
-    const char *initrd_filename;
-    const char *cpu_model;
     int pci_first_slot;
     int pci_nr_slots;
 
-    /* e500-specific params */
-
     /* required -- must at least add toplevel board compatible */
     void (*fixup_devtree)(struct PPCE500Params *params, void *fdt);
 
     int mpic_version;
 } PPCE500Params;
 
-void ppce500_init(PPCE500Params *params);
+void ppce500_init(QEMUMachineInitArgs *args, PPCE500Params *params);
 
 #endif
diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index a78de07..bf65b69 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -31,12 +31,6 @@ static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
 static void e500plat_init(QEMUMachineInitArgs *args)
 {
     PPCE500Params params = {
-        .ram_size = args->ram_size,
-        .boot_device = args->boot_device,
-        .kernel_filename = args->kernel_filename,
-        .kernel_cmdline = args->kernel_cmdline,
-        .initrd_filename = args->initrd_filename,
-        .cpu_model = args->cpu_model,
         .pci_first_slot = 0x1,
         .pci_nr_slots = PCI_SLOT_MAX - 1,
         .fixup_devtree = e500plat_fixup_devtree,
@@ -49,7 +43,7 @@ static void e500plat_init(QEMUMachineInitArgs *args)
         params.mpic_version = OPENPIC_MODEL_FSL_MPIC_20;
     }
 
-    ppce500_init(&params);
+    ppce500_init(args, &params);
 }
 
 static QEMUMachine e500plat_machine = {
diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
index 4e551af..1888e75 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -29,19 +29,13 @@ static void mpc8544ds_fixup_devtree(PPCE500Params *params, void *fdt)
 static void mpc8544ds_init(QEMUMachineInitArgs *args)
 {
     PPCE500Params params = {
-        .ram_size = args->ram_size,
-        .boot_device = args->boot_device,
-        .kernel_filename = args->kernel_filename,
-        .kernel_cmdline = args->kernel_cmdline,
-        .initrd_filename = args->initrd_filename,
-        .cpu_model = args->cpu_model,
         .pci_first_slot = 0x11,
         .pci_nr_slots = 2,
         .fixup_devtree = mpc8544ds_fixup_devtree,
         .mpic_version = OPENPIC_MODEL_FSL_MPIC_20,
     };
 
-    ppce500_init(&params);
+    ppce500_init(args, &params);
 }
 
 
-- 
1.7.11.7

[Qemu-devel] [PATCH v3 6/6] hw: Clean up bogus default boot order

[Markus Armbruster]

We set default boot order "cad" in every single machine definition
except "pseries" and "moxiesim", even though very few boards actually
care for boot order, and "cad" makes sense for even fewer.

Machines that care:

* pc and its variants

  Accept up to three letters 'a', 'b' (undocumented alias for 'a'),
  'c', 'd' and 'n'.  Reject all others (fatal with -boot).

* nseries (n800, n810)

  Check whether order starts with 'n'.  Silently ignored otherwise.

* prep, g3beige, mac99

  Extract the first character the machine understands (subset of
  'a'..'f').  Silently ignored otherwise.

* spapr

  Accept an arbitrary string (vl.c restricts it to contain only
  'a'..'p', no duplicates).

* sun4[mdc]

  Use the first character.  Silently ignored otherwise.

Strip characters these machines ignore from their default boot order.

For all other machines, remove the unused default boot order
alltogether.

Note that my rename of QEMUMachine member boot_order to
default_boot_order and QEMUMachineInitArgs member boot_device to
boot_order has a welcome side effect: it makes every use of boot
orders visible in this patch, for easy review.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/alpha/dp264.c                         |  1 -
 hw/arm/collie.c                          |  1 -
 hw/arm/exynos4_boards.c                  |  2 --
 hw/arm/gumstix.c                         |  2 --
 hw/arm/highbank.c                        |  2 --
 hw/arm/integratorcp.c                    |  1 -
 hw/arm/kzm.c                             |  1 -
 hw/arm/mainstone.c                       |  1 -
 hw/arm/musicpal.c                        |  1 -
 hw/arm/nseries.c                         |  6 +++---
 hw/arm/omap_sx1.c                        |  2 --
 hw/arm/palm.c                            |  1 -
 hw/arm/realview.c                        |  4 ----
 hw/arm/spitz.c                           |  4 ----
 hw/arm/stellaris.c                       |  2 --
 hw/arm/tosa.c                            |  1 -
 hw/arm/versatilepb.c                     |  2 --
 hw/arm/vexpress.c                        |  2 --
 hw/arm/xilinx_zynq.c                     |  1 -
 hw/arm/z2.c                              |  1 -
 hw/core/null-machine.c                   |  1 -
 hw/cris/axis_dev88.c                     |  1 -
 hw/i386/pc_piix.c                        | 32 ++++++++++++++++----------------
 hw/i386/pc_q35.c                         |  8 ++++----
 hw/i386/xen_machine_pv.c                 |  1 -
 hw/lm32/lm32_boards.c                    |  2 --
 hw/lm32/milkymist.c                      |  1 -
 hw/m68k/an5206.c                         |  1 -
 hw/m68k/dummy_m68k.c                     |  1 -
 hw/m68k/mcf5208.c                        |  1 -
 hw/microblaze/petalogix_ml605_mmu.c      |  1 -
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  1 -
 hw/mips/mips_fulong2e.c                  |  1 -
 hw/mips/mips_jazz.c                      |  2 --
 hw/mips/mips_malta.c                     |  1 -
 hw/mips/mips_mipssim.c                   |  1 -
 hw/mips/mips_r4k.c                       |  1 -
 hw/openrisc/openrisc_sim.c               |  1 -
 hw/ppc/e500plat.c                        |  1 -
 hw/ppc/mac_newworld.c                    |  4 ++--
 hw/ppc/mac_oldworld.c                    |  4 ++--
 hw/ppc/mpc8544ds.c                       |  1 -
 hw/ppc/ppc405_boards.c                   |  2 --
 hw/ppc/ppc440_bamboo.c                   |  1 -
 hw/ppc/prep.c                            |  4 ++--
 hw/ppc/spapr.c                           |  4 ++--
 hw/ppc/virtex_ml507.c                    |  1 -
 hw/s390x/s390-virtio-ccw.c               |  1 -
 hw/s390x/s390-virtio.c                   |  1 -
 hw/sh4/r2d.c                             |  1 -
 hw/sh4/shix.c                            |  1 -
 hw/sparc/leon3.c                         |  1 -
 hw/sparc/sun4m.c                         | 22 +++++++++++-----------
 hw/sparc64/sun4u.c                       | 10 +++++-----
 hw/unicore32/puv3.c                      |  1 -
 hw/xtensa/xtensa_lx60.c                  |  2 --
 hw/xtensa/xtensa_sim.c                   |  1 -
 include/hw/boards.h                      |  7 ++-----
 vl.c                                     |  4 ++--
 59 files changed, 51 insertions(+), 119 deletions(-)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 95fde61..20795ac 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -173,7 +173,6 @@ static QEMUMachine clipper_machine = {
     .init = clipper_init,
     .max_cpus = 4,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void clipper_machine_init(void)
diff --git a/hw/arm/collie.c b/hw/arm/collie.c
index a19857a..8878b0e 100644
--- a/hw/arm/collie.c
+++ b/hw/arm/collie.c
@@ -62,7 +62,6 @@ static QEMUMachine collie_machine = {
     .name = "collie",
     .desc = "Collie PDA (SA-1110)",
     .init = collie_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void collie_machine_init(void)
diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
index 7c90b2d..2929f9f 100644
--- a/hw/arm/exynos4_boards.c
+++ b/hw/arm/exynos4_boards.c
@@ -150,14 +150,12 @@ static QEMUMachine exynos4_machines[EXYNOS4_NUM_OF_BOARDS] = {
         .desc = "Samsung NURI board (Exynos4210)",
         .init = nuri_init,
         .max_cpus = EXYNOS4210_NCPUS,
-        DEFAULT_MACHINE_OPTIONS,
     },
     [EXYNOS4_BOARD_SMDKC210] = {
         .name = "smdkc210",
         .desc = "Samsung SMDKC210 board (Exynos4210)",
         .init = smdkc210_init,
         .max_cpus = EXYNOS4210_NCPUS,
-        DEFAULT_MACHINE_OPTIONS,
     },
 };
 
diff --git a/hw/arm/gumstix.c b/hw/arm/gumstix.c
index b8cab10..e97fbbd 100644
--- a/hw/arm/gumstix.c
+++ b/hw/arm/gumstix.c
@@ -122,14 +122,12 @@ static QEMUMachine connex_machine = {
     .name = "connex",
     .desc = "Gumstix Connex (PXA255)",
     .init = connex_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine verdex_machine = {
     .name = "verdex",
     .desc = "Gumstix Verdex (PXA270)",
     .init = verdex_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void gumstix_machine_init(void)
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index be264d3..4dfcfd0 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -359,7 +359,6 @@ static QEMUMachine highbank_machine = {
     .init = highbank_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine midway_machine = {
@@ -368,7 +367,6 @@ static QEMUMachine midway_machine = {
     .init = midway_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void calxeda_machines_init(void)
diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 249a430..5b20859 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -513,7 +513,6 @@ static QEMUMachine integratorcp_machine = {
     .desc = "ARM Integrator/CP (ARM926EJ-S)",
     .init = integratorcp_init,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void integratorcp_machine_init(void)
diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
index bd6c05c..0007923 100644
--- a/hw/arm/kzm.c
+++ b/hw/arm/kzm.c
@@ -146,7 +146,6 @@ static QEMUMachine kzm_machine = {
     .name = "kzm",
     .desc = "ARM KZM Emulation Baseboard (ARM1136)",
     .init = kzm_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void kzm_machine_init(void)
diff --git a/hw/arm/mainstone.c b/hw/arm/mainstone.c
index 8e5fc26..b244f7e 100644
--- a/hw/arm/mainstone.c
+++ b/hw/arm/mainstone.c
@@ -179,7 +179,6 @@ static QEMUMachine mainstone2_machine = {
     .name = "mainstone",
     .desc = "Mainstone II (PXA27x)",
     .init = mainstone_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void mainstone_machine_init(void)
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index b06d442..bed9e16 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1681,7 +1681,6 @@ static QEMUMachine musicpal_machine = {
     .name = "musicpal",
     .desc = "Marvell 88w8618 / MusicPal (ARM926EJ-S)",
     .init = musicpal_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void musicpal_machine_init(void)
diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index f6c9dc0..9ef31ca 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -1340,7 +1340,7 @@ static void n8x0_init(QEMUMachineInitArgs *args,
     }
 
     if (option_rom[0].name &&
-        (args->boot_device[0] == 'n' || !args->kernel_filename)) {
+        (args->boot_order[0] == 'n' || !args->kernel_filename)) {
         uint8_t nolo_tags[0x10000];
         /* No, wait, better start at the ROM.  */
         s->mpu->cpu->env.regs[15] = OMAP2_Q2_BASE + 0x400000;
@@ -1396,14 +1396,14 @@ static QEMUMachine n800_machine = {
     .name = "n800",
     .desc = "Nokia N800 tablet aka. RX-34 (OMAP2420)",
     .init = n800_init,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "",
 };
 
 static QEMUMachine n810_machine = {
     .name = "n810",
     .desc = "Nokia N810 tablet aka. RX-44 (OMAP2420)",
     .init = n810_init,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "",
 };
 
 static void nseries_machine_init(void)
diff --git a/hw/arm/omap_sx1.c b/hw/arm/omap_sx1.c
index 05b0353..b0f8664 100644
--- a/hw/arm/omap_sx1.c
+++ b/hw/arm/omap_sx1.c
@@ -219,14 +219,12 @@ static QEMUMachine sx1_machine_v2 = {
     .name = "sx1",
     .desc = "Siemens SX1 (OMAP310) V2",
     .init = sx1_init_v2,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine sx1_machine_v1 = {
     .name = "sx1-v1",
     .desc = "Siemens SX1 (OMAP310) V1",
     .init = sx1_init_v1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void sx1_machine_init(void)
diff --git a/hw/arm/palm.c b/hw/arm/palm.c
index cdc3c3a..3e39044 100644
--- a/hw/arm/palm.c
+++ b/hw/arm/palm.c
@@ -273,7 +273,6 @@ static QEMUMachine palmte_machine = {
     .name = "cheetah",
     .desc = "Palm Tungsten|E aka. Cheetah PDA (OMAP310)",
     .init = palmte_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void palmte_machine_init(void)
diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 3060f48..1f4e5b0 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -371,7 +371,6 @@ static QEMUMachine realview_eb_machine = {
     .desc = "ARM RealView Emulation Baseboard (ARM926EJ-S)",
     .init = realview_eb_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine realview_eb_mpcore_machine = {
@@ -380,14 +379,12 @@ static QEMUMachine realview_eb_mpcore_machine = {
     .init = realview_eb_mpcore_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine realview_pb_a8_machine = {
     .name = "realview-pb-a8",
     .desc = "ARM RealView Platform Baseboard for Cortex-A8",
     .init = realview_pb_a8_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine realview_pbx_a9_machine = {
@@ -396,7 +393,6 @@ static QEMUMachine realview_pbx_a9_machine = {
     .init = realview_pbx_a9_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void realview_machine_init(void)
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 593b75e..5062020 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -959,28 +959,24 @@ static QEMUMachine akitapda_machine = {
     .name = "akita",
     .desc = "Akita PDA (PXA270)",
     .init = akita_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine spitzpda_machine = {
     .name = "spitz",
     .desc = "Spitz PDA (PXA270)",
     .init = spitz_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine borzoipda_machine = {
     .name = "borzoi",
     .desc = "Borzoi PDA (PXA270)",
     .init = borzoi_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine terrierpda_machine = {
     .name = "terrier",
     .desc = "Terrier PDA (PXA270)",
     .init = terrier_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void spitz_machine_init(void)
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index a2b6b17..3b9bca6 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -1331,14 +1331,12 @@ static QEMUMachine lm3s811evb_machine = {
     .name = "lm3s811evb",
     .desc = "Stellaris LM3S811EVB",
     .init = lm3s811evb_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine lm3s6965evb_machine = {
     .name = "lm3s6965evb",
     .desc = "Stellaris LM3S6965EVB",
     .init = lm3s6965evb_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void stellaris_machine_init(void)
diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
index 47d1f4f..c00d8c2 100644
--- a/hw/arm/tosa.c
+++ b/hw/arm/tosa.c
@@ -251,7 +251,6 @@ static QEMUMachine tosapda_machine = {
     .name = "tosa",
     .desc = "Tosa PDA (PXA255)",
     .init = tosa_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void tosapda_machine_init(void)
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index 725f60f..326bf63 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -363,7 +363,6 @@ static QEMUMachine versatilepb_machine = {
     .desc = "ARM Versatile/PB (ARM926EJ-S)",
     .init = vpb_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine versatileab_machine = {
@@ -371,7 +370,6 @@ static QEMUMachine versatileab_machine = {
     .desc = "ARM Versatile/AB (ARM926EJ-S)",
     .init = vab_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void versatile_machine_init(void)
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 7d1b823..072e91c 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -553,7 +553,6 @@ static QEMUMachine vexpress_a9_machine = {
     .init = vexpress_a9_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine vexpress_a15_machine = {
@@ -562,7 +561,6 @@ static QEMUMachine vexpress_a15_machine = {
     .init = vexpress_a15_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void vexpress_machine_init(void)
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 3444823..608686a 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -236,7 +236,6 @@ static QEMUMachine zynq_machine = {
     .block_default_type = IF_SCSI,
     .max_cpus = 1,
     .no_sdcard = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void zynq_machine_init(void)
diff --git a/hw/arm/z2.c b/hw/arm/z2.c
index 07a127b..2e0d5d4 100644
--- a/hw/arm/z2.c
+++ b/hw/arm/z2.c
@@ -373,7 +373,6 @@ static QEMUMachine z2_machine = {
     .name = "z2",
     .desc = "Zipit Z2 (PXA27x)",
     .init = z2_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void z2_machine_init(void)
diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c
index bdf109f..d813c08 100644
--- a/hw/core/null-machine.c
+++ b/hw/core/null-machine.c
@@ -24,7 +24,6 @@ static QEMUMachine machine_none = {
     .desc = "empty machine",
     .init = machine_none_init,
     .max_cpus = 0,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void register_machines(void)
diff --git a/hw/cris/axis_dev88.c b/hw/cris/axis_dev88.c
index 9104d61..03058d3 100644
--- a/hw/cris/axis_dev88.c
+++ b/hw/cris/axis_dev88.c
@@ -355,7 +355,6 @@ static QEMUMachine axisdev88_machine = {
     .desc = "AXIS devboard 88",
     .init = axisdev88_init,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void axisdev88_machine_init(void)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9327ac1..3700bd5 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -216,7 +216,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
         }
     }
 
-    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
+    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
                  floppy, idebus[0], idebus[1], rtc_state);
 
     if (pci_enabled && usb_enabled(false)) {
@@ -324,7 +324,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
     .hot_add_cpu = pc_hot_add_cpu,
     .max_cpus = 255,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "cad",
 };
 
 static QEMUMachine pc_i440fx_machine_v1_5 = {
@@ -337,7 +337,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
         PC_COMPAT_1_5,
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "cad",
 };
 
 static QEMUMachine pc_i440fx_machine_v1_4 = {
@@ -345,11 +345,11 @@ static QEMUMachine pc_i440fx_machine_v1_4 = {
     .desc = "Standard PC (i440FX + PIIX, 1996)",
     .init = pc_init_pci_1_4,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_4,
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_1_3 \
@@ -377,11 +377,11 @@ static QEMUMachine pc_machine_v1_3 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_3,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_3,
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_1_2 \
@@ -417,11 +417,11 @@ static QEMUMachine pc_machine_v1_2 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_2,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_2,
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_1_1 \
@@ -461,11 +461,11 @@ static QEMUMachine pc_machine_v1_1 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_2,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_1,
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_1_0 \
@@ -497,12 +497,12 @@ static QEMUMachine pc_machine_v1_0 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_0,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_0,
         { /* end of list */ }
     },
     .hw_version = "1.0",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_0_15 \
@@ -513,12 +513,12 @@ static QEMUMachine pc_machine_v0_15 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_0,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_15,
         { /* end of list */ }
     },
     .hw_version = "0.15",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_0_14 \
@@ -546,6 +546,7 @@ static QEMUMachine pc_machine_v0_14 = {
     .desc = "Standard PC",
     .init = pc_init_pci_1_0,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_14, 
         {
@@ -560,7 +561,6 @@ static QEMUMachine pc_machine_v0_14 = {
         { /* end of list */ }
     },
     .hw_version = "0.14",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_0_13 \
@@ -580,6 +580,7 @@ static QEMUMachine pc_machine_v0_13 = {
     .desc = "Standard PC",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_13,
         {
@@ -598,7 +599,6 @@ static QEMUMachine pc_machine_v0_13 = {
         { /* end of list */ }
     },
     .hw_version = "0.13",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_0_12 \
@@ -630,6 +630,7 @@ static QEMUMachine pc_machine_v0_12 = {
     .desc = "Standard PC",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_12,
         {
@@ -644,7 +645,6 @@ static QEMUMachine pc_machine_v0_12 = {
         { /* end of list */ }
     },
     .hw_version = "0.12",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #define PC_COMPAT_0_11 \
@@ -664,6 +664,7 @@ static QEMUMachine pc_machine_v0_11 = {
     .desc = "Standard PC, qemu 0.11",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_11,
         {
@@ -678,7 +679,6 @@ static QEMUMachine pc_machine_v0_11 = {
         { /* end of list */ }
     },
     .hw_version = "0.11",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine pc_machine_v0_10 = {
@@ -686,6 +686,7 @@ static QEMUMachine pc_machine_v0_10 = {
     .desc = "Standard PC, qemu 0.10",
     .init = pc_init_pci_no_kvmclock,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_0_11,
         {
@@ -712,7 +713,6 @@ static QEMUMachine pc_machine_v0_10 = {
         { /* end of list */ }
     },
     .hw_version = "0.10",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine isapc_machine = {
@@ -720,6 +720,7 @@ static QEMUMachine isapc_machine = {
     .desc = "ISA-only PC",
     .init = pc_init_isa,
     .max_cpus = 1,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         {
             .driver   = "pc-sysfw",
@@ -733,7 +734,6 @@ static QEMUMachine isapc_machine = {
         },
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 #ifdef CONFIG_XEN
@@ -743,7 +743,7 @@ static QEMUMachine xenfv_machine = {
     .init = pc_xen_hvm_init,
     .max_cpus = HVM_MAX_VCPUS,
     .default_machine_opts = "accel=xen",
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "cad",
 };
 #endif
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 1657eb3..28ecf76 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -195,7 +195,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
                                     0xb100),
                       8, NULL, 0);
 
-    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
+    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
                  floppy, idebus[0], idebus[1], rtc_state);
 
     /* the rest devices to which pci devfn is automatically assigned */
@@ -230,7 +230,7 @@ static QEMUMachine pc_q35_machine_v1_6 = {
     .init = pc_q35_init,
     .hot_add_cpu = pc_hot_add_cpu,
     .max_cpus = 255,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "cad",
 };
 
 static QEMUMachine pc_q35_machine_v1_5 = {
@@ -239,11 +239,11 @@ static QEMUMachine pc_q35_machine_v1_5 = {
     .init = pc_q35_init_1_5,
     .hot_add_cpu = pc_hot_add_cpu,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_5,
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine pc_q35_machine_v1_4 = {
@@ -251,11 +251,11 @@ static QEMUMachine pc_q35_machine_v1_4 = {
     .desc = "Standard PC (Q35 + ICH9, 2009)",
     .init = pc_q35_init_1_4,
     .max_cpus = 255,
+    .default_boot_order = "cad",
     .compat_props = (GlobalProperty[]) {
         PC_COMPAT_1_4,
         { /* end of list */ }
     },
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void pc_q35_machine_init(void)
diff --git a/hw/i386/xen_machine_pv.c b/hw/i386/xen_machine_pv.c
index 9f2e291..9adb57f 100644
--- a/hw/i386/xen_machine_pv.c
+++ b/hw/i386/xen_machine_pv.c
@@ -99,7 +99,6 @@ static QEMUMachine xenpv_machine = {
     .init = xen_init_pv,
     .max_cpus = 1,
     .default_machine_opts = "accel=xen",
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void xenpv_machine_init(void)
diff --git a/hw/lm32/lm32_boards.c b/hw/lm32/lm32_boards.c
index 62003b8..c032bb8 100644
--- a/hw/lm32/lm32_boards.c
+++ b/hw/lm32/lm32_boards.c
@@ -289,7 +289,6 @@ static QEMUMachine lm32_evr_machine = {
     .desc = "LatticeMico32 EVR32 eval system",
     .init = lm32_evr_init,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine lm32_uclinux_machine = {
@@ -297,7 +296,6 @@ static QEMUMachine lm32_uclinux_machine = {
     .desc = "lm32 platform for uClinux and u-boot by Theobroma Systems",
     .init = lm32_uclinux_init,
     .is_default = 0,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void lm32_machine_init(void)
diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
index 7ceedb8..f1744ec 100644
--- a/hw/lm32/milkymist.c
+++ b/hw/lm32/milkymist.c
@@ -208,7 +208,6 @@ static QEMUMachine milkymist_machine = {
     .desc = "Milkymist One",
     .init = milkymist_init,
     .is_default = 0,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void milkymist_machine_init(void)
diff --git a/hw/m68k/an5206.c b/hw/m68k/an5206.c
index 0c03a87..a8eee44 100644
--- a/hw/m68k/an5206.c
+++ b/hw/m68k/an5206.c
@@ -89,7 +89,6 @@ static QEMUMachine an5206_machine = {
     .name = "an5206",
     .desc = "Arnewsh 5206",
     .init = an5206_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void an5206_machine_init(void)
diff --git a/hw/m68k/dummy_m68k.c b/hw/m68k/dummy_m68k.c
index f4ed7c6..86e2e6e 100644
--- a/hw/m68k/dummy_m68k.c
+++ b/hw/m68k/dummy_m68k.c
@@ -73,7 +73,6 @@ static QEMUMachine dummy_m68k_machine = {
     .name = "dummy",
     .desc = "Dummy board",
     .init = dummy_m68k_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void dummy_m68k_machine_init(void)
diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index 9cf000f..fb96fe8 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -295,7 +295,6 @@ static QEMUMachine mcf5208evb_machine = {
     .desc = "MCF5206EVB",
     .init = mcf5208evb_init,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void mcf5208evb_machine_init(void)
diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index 989da25..e003c7c 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -186,7 +186,6 @@ static QEMUMachine petalogix_ml605_machine = {
     .desc = "PetaLogix linux refdesign for xilinx ml605 little endian",
     .init = petalogix_ml605_init,
     .is_default = 0,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void petalogix_ml605_machine_init(void)
diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index a461494..00af2b5 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -116,7 +116,6 @@ static QEMUMachine petalogix_s3adsp1800_machine = {
     .desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800",
     .init = petalogix_s3adsp1800_init,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void petalogix_s3adsp1800_machine_init(void)
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 9e305d2..bf9ae4a 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -400,7 +400,6 @@ static QEMUMachine mips_fulong2e_machine = {
     .name = "fulong2e",
     .desc = "Fulong 2e mini pc",
     .init = mips_fulong2e_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void mips_fulong2e_machine_init(void)
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 31e138b..bf12faa 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -323,7 +323,6 @@ static QEMUMachine mips_magnum_machine = {
     .desc = "MIPS Magnum",
     .init = mips_magnum_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine mips_pica61_machine = {
@@ -331,7 +330,6 @@ static QEMUMachine mips_pica61_machine = {
     .desc = "Acer Pica 61",
     .init = mips_pica61_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void mips_jazz_machine_init(void)
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index de87241..cb22279 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1017,7 +1017,6 @@ static QEMUMachine mips_malta_machine = {
     .init = mips_malta_init,
     .max_cpus = 16,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void mips_malta_register_types(void)
diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index a10cf13..ea5868c 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -229,7 +229,6 @@ static QEMUMachine mips_mipssim_machine = {
     .name = "mipssim",
     .desc = "MIPS MIPSsim platform",
     .init = mips_mipssim_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void mips_mipssim_machine_init(void)
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index 22beb0a..cf569c1 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -302,7 +302,6 @@ static QEMUMachine mips_machine = {
     .name = "mips",
     .desc = "mips r4k platform",
     .init = mips_r4k_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void mips_machine_init(void)
diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 924438b..2e32bea 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -139,7 +139,6 @@ static QEMUMachine openrisc_sim_machine = {
     .init = openrisc_sim_init,
     .max_cpus = 1,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void openrisc_sim_machine_init(void)
diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index bf65b69..2e964b2 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -51,7 +51,6 @@ static QEMUMachine e500plat_machine = {
     .desc = "generic paravirt e500 platform",
     .init = e500plat_init,
     .max_cpus = 32,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void e500plat_machine_init(void)
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index fe80348..670c473 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -147,7 +147,7 @@ static void ppc_core99_init(QEMUMachineInitArgs *args)
     const char *kernel_filename = args->kernel_filename;
     const char *kernel_cmdline = args->kernel_cmdline;
     const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
+    const char *boot_device = args->boot_order;
     PowerPCCPU *cpu = NULL;
     CPUPPCState *env = NULL;
     char *filename;
@@ -474,7 +474,7 @@ static QEMUMachine core99_machine = {
     .desc = "Mac99 based PowerMAC",
     .init = ppc_core99_init,
     .max_cpus = MAX_CPUS,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "cd",
 };
 
 static void core99_machine_init(void)
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 8b8c6b9..a3ec9a5 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -78,7 +78,7 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args)
     const char *kernel_filename = args->kernel_filename;
     const char *kernel_cmdline = args->kernel_cmdline;
     const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
+    const char *boot_device = args->boot_order;
     MemoryRegion *sysmem = get_system_memory();
     PowerPCCPU *cpu = NULL;
     CPUPPCState *env = NULL;
@@ -347,7 +347,7 @@ static QEMUMachine heathrow_machine = {
 #ifndef TARGET_PPC64
     .is_default = 1,
 #endif
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "cd", /* TOFIX "cad" when Mac floppy is implemented */
 };
 
 static void heathrow_machine_init(void)
diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
index 1888e75..edcc0be 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -44,7 +44,6 @@ static QEMUMachine ppce500_machine = {
     .desc = "mpc8544ds",
     .init = mpc8544ds_init,
     .max_cpus = 15,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void ppce500_machine_init(void)
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index f74e5e5..43a83ca 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -362,7 +362,6 @@ static QEMUMachine ref405ep_machine = {
     .name = "ref405ep",
     .desc = "ref405ep",
     .init = ref405ep_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 /*****************************************************************************/
@@ -650,7 +649,6 @@ static QEMUMachine taihu_machine = {
     .name = "taihu",
     .desc = "taihu",
     .init = taihu_405ep_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void ppc405_machine_init(void)
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 5b039ab..698ec87 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -293,7 +293,6 @@ static QEMUMachine bamboo_machine = {
     .name = "bamboo",
     .desc = "bamboo",
     .init = bamboo_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void bamboo_machine_init(void)
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 19f2442..e884d3e 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -452,7 +452,7 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
     const char *kernel_filename = args->kernel_filename;
     const char *kernel_cmdline = args->kernel_cmdline;
     const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
+    const char *boot_device = args->boot_order;
     MemoryRegion *sysmem = get_system_memory();
     PowerPCCPU *cpu = NULL;
     CPUPPCState *env = NULL;
@@ -691,7 +691,7 @@ static QEMUMachine prep_machine = {
     .desc = "PowerPC PREP platform",
     .init = ppc_prep_init,
     .max_cpus = MAX_CPUS,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "cad",
 };
 
 static void prep_machine_init(void)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 48ae092..805261a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -718,7 +718,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
     const char *kernel_filename = args->kernel_filename;
     const char *kernel_cmdline = args->kernel_cmdline;
     const char *initrd_filename = args->initrd_filename;
-    const char *boot_device = args->boot_device;
+    const char *boot_device = args->boot_order;
     PowerPCCPU *cpu;
     CPUPPCState *env;
     PCIHostState *phb;
@@ -971,7 +971,7 @@ static QEMUMachine spapr_machine = {
     .block_default_type = IF_SCSI,
     .max_cpus = MAX_CPUS,
     .no_parallel = 1,
-    .boot_order = NULL,
+    .default_boot_order = NULL,
 };
 
 static void spapr_machine_init(void)
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index 08e77fb..7250f51 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -245,7 +245,6 @@ static QEMUMachine virtex_machine = {
     .name = "virtex-ml507",
     .desc = "Xilinx Virtex ML507 reference design",
     .init = virtex_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void virtex_machine_init(void)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index aebbbf1..e2681a6 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -126,7 +126,6 @@ static QEMUMachine ccw_machine = {
     .no_sdcard = 1,
     .use_sclp = 1,
     .max_cpus = 255,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void ccw_machine_init(void)
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index edbde00..df52f1f 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -293,7 +293,6 @@ static QEMUMachine s390_machine = {
     .use_virtcon = 1,
     .max_cpus = 255,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void s390_machine_init(void)
diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
index 98b3408..7b1de85 100644
--- a/hw/sh4/r2d.c
+++ b/hw/sh4/r2d.c
@@ -356,7 +356,6 @@ static QEMUMachine r2d_machine = {
     .name = "r2d",
     .desc = "r2d-plus board",
     .init = r2d_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void r2d_machine_init(void)
diff --git a/hw/sh4/shix.c b/hw/sh4/shix.c
index 84dd666..1ff37f5 100644
--- a/hw/sh4/shix.c
+++ b/hw/sh4/shix.c
@@ -96,7 +96,6 @@ static QEMUMachine shix_machine = {
     .desc = "shix card",
     .init = shix_init,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void shix_machine_init(void)
diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 5ef282f..390f3e4 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -216,7 +216,6 @@ static QEMUMachine leon3_generic_machine = {
     .name     = "leon3_generic",
     .desc     = "Leon-3 generic",
     .init     = leon3_generic_hw_init,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void leon3_machine_init(void)
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 97c3ee0..ad09312 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -976,7 +976,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
                                     args->ram_size);
 
     nvram_init(nvram, (uint8_t *)&nd_table[0].macaddr, args->kernel_cmdline,
-               args->boot_device, args->ram_size, kernel_size, graphic_width,
+               args->boot_order, args->ram_size, kernel_size, graphic_width,
                graphic_height, graphic_depth, hwdef->nvram_machine_id,
                "Sun4m");
 
@@ -1005,7 +1005,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     }
     fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
     fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, 0); // not used
-    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_device[0]);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_order[0]);
     qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
 }
 
@@ -1326,7 +1326,7 @@ static QEMUMachine ss5_machine = {
     .init = ss5_init,
     .block_default_type = IF_SCSI,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine ss10_machine = {
@@ -1335,7 +1335,7 @@ static QEMUMachine ss10_machine = {
     .init = ss10_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine ss600mp_machine = {
@@ -1344,7 +1344,7 @@ static QEMUMachine ss600mp_machine = {
     .init = ss600mp_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine ss20_machine = {
@@ -1353,7 +1353,7 @@ static QEMUMachine ss20_machine = {
     .init = ss20_init,
     .block_default_type = IF_SCSI,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine voyager_machine = {
@@ -1361,7 +1361,7 @@ static QEMUMachine voyager_machine = {
     .desc = "Sun4m platform, SPARCstation Voyager",
     .init = vger_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine ss_lx_machine = {
@@ -1369,7 +1369,7 @@ static QEMUMachine ss_lx_machine = {
     .desc = "Sun4m platform, SPARCstation LX",
     .init = ss_lx_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine ss4_machine = {
@@ -1377,7 +1377,7 @@ static QEMUMachine ss4_machine = {
     .desc = "Sun4m platform, SPARCstation 4",
     .init = ss4_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine scls_machine = {
@@ -1385,7 +1385,7 @@ static QEMUMachine scls_machine = {
     .desc = "Sun4m platform, SPARCClassic",
     .init = scls_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine sbook_machine = {
@@ -1393,7 +1393,7 @@ static QEMUMachine sbook_machine = {
     .desc = "Sun4m platform, SPARCbook",
     .init = sbook_init,
     .block_default_type = IF_SCSI,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static void sun4m_register_types(void)
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 22e9818..9f726b4 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -872,7 +872,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
                                     &kernel_addr, &kernel_entry);
 
     sun4u_NVRAM_set_params(nvram, NVRAM_SIZE, "Sun4u", args->ram_size,
-                           args->boot_device,
+                           args->boot_order,
                            kernel_addr, kernel_size,
                            args->kernel_cmdline,
                            initrd_addr, initrd_size,
@@ -897,7 +897,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     }
     fw_cfg_add_i64(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
     fw_cfg_add_i64(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
-    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_device[0]);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_order[0]);
 
     fw_cfg_add_i16(fw_cfg, FW_CFG_SPARC64_WIDTH, graphic_width);
     fw_cfg_add_i16(fw_cfg, FW_CFG_SPARC64_HEIGHT, graphic_height);
@@ -960,7 +960,7 @@ static QEMUMachine sun4u_machine = {
     .init = sun4u_init,
     .max_cpus = 1, // XXX for now
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine sun4v_machine = {
@@ -968,7 +968,7 @@ static QEMUMachine sun4v_machine = {
     .desc = "Sun4v platform",
     .init = sun4v_init,
     .max_cpus = 1, // XXX for now
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static QEMUMachine niagara_machine = {
@@ -976,7 +976,7 @@ static QEMUMachine niagara_machine = {
     .desc = "Sun4v platform, Niagara",
     .init = niagara_init,
     .max_cpus = 1, // XXX for now
-    DEFAULT_MACHINE_OPTIONS,
+    .default_boot_order = "c",
 };
 
 static void sun4u_register_types(void)
diff --git a/hw/unicore32/puv3.c b/hw/unicore32/puv3.c
index 5ff0dc9..a900061 100644
--- a/hw/unicore32/puv3.c
+++ b/hw/unicore32/puv3.c
@@ -128,7 +128,6 @@ static QEMUMachine puv3_machine = {
     .desc = "PKUnity Version-3 based on UniCore32",
     .init = puv3_init,
     .is_default = 1,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void puv3_machine_init(void)
diff --git a/hw/xtensa/xtensa_lx60.c b/hw/xtensa/xtensa_lx60.c
index 075daf1..3d2f990 100644
--- a/hw/xtensa/xtensa_lx60.c
+++ b/hw/xtensa/xtensa_lx60.c
@@ -295,7 +295,6 @@ static QEMUMachine xtensa_lx60_machine = {
     .desc = "lx60 EVB (" XTENSA_DEFAULT_CPU_MODEL ")",
     .init = xtensa_lx60_init,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static QEMUMachine xtensa_lx200_machine = {
@@ -303,7 +302,6 @@ static QEMUMachine xtensa_lx200_machine = {
     .desc = "lx200 EVB (" XTENSA_DEFAULT_CPU_MODEL ")",
     .init = xtensa_lx200_init,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void xtensa_lx_machines_init(void)
diff --git a/hw/xtensa/xtensa_sim.c b/hw/xtensa/xtensa_sim.c
index a88707e..af9af10 100644
--- a/hw/xtensa/xtensa_sim.c
+++ b/hw/xtensa/xtensa_sim.c
@@ -106,7 +106,6 @@ static QEMUMachine xtensa_sim_machine = {
     .is_default = true,
     .init = xtensa_sim_init,
     .max_cpus = 4,
-    DEFAULT_MACHINE_OPTIONS,
 };
 
 static void xtensa_sim_machine_init(void)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index fb7c6f1..5a7ae9f 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -6,12 +6,9 @@
 #include "sysemu/blockdev.h"
 #include "hw/qdev.h"
 
-#define DEFAULT_MACHINE_OPTIONS \
-    .boot_order = "cad"
-
 typedef struct QEMUMachineInitArgs {
     ram_addr_t ram_size;
-    const char *boot_device;
+    const char *boot_order;
     const char *kernel_filename;
     const char *kernel_cmdline;
     const char *initrd_filename;
@@ -42,7 +39,7 @@ typedef struct QEMUMachine {
         no_sdcard:1;
     int is_default;
     const char *default_machine_opts;
-    const char *boot_order;
+    const char *default_boot_order;
     GlobalProperty *compat_props;
     struct QEMUMachine *next;
     const char *hw_version;
diff --git a/vl.c b/vl.c
index 25b8f2f..dcd7661 100644
--- a/vl.c
+++ b/vl.c
@@ -4121,7 +4121,7 @@ int main(int argc, char **argv, char **envp)
     kernel_cmdline = qemu_opt_get(machine_opts, "append");
 
     if (!boot_order) {
-        boot_order = machine->boot_order;
+        boot_order = machine->default_boot_order;
     }
     opts = qemu_opts_find(qemu_find_opts("boot-opts"), NULL);
     if (opts) {
@@ -4309,7 +4309,7 @@ int main(int argc, char **argv, char **envp)
     qdev_machine_init();
 
     QEMUMachineInitArgs args = { .ram_size = ram_size,
-                                 .boot_device = boot_order,
+                                 .boot_order = boot_order,
                                  .kernel_filename = kernel_filename,
                                  .kernel_cmdline = kernel_cmdline,
                                  .initrd_filename = initrd_filename,
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH v3 6/6] hw: Clean up bogus default boot order

[Michael S. Tsirkin]

On Mon, Jul 22, 2013 at 01:38:47PM +0200, Markus Armbruster wrote:
> We set default boot order "cad" in every single machine definition
> except "pseries" and "moxiesim", even though very few boards actually
> care for boot order, and "cad" makes sense for even fewer.
> 
> Machines that care:
> 
> * pc and its variants
> 
>   Accept up to three letters 'a', 'b' (undocumented alias for 'a'),
>   'c', 'd' and 'n'.  Reject all others (fatal with -boot).
> 
> * nseries (n800, n810)
> 
>   Check whether order starts with 'n'.  Silently ignored otherwise.
> 
> * prep, g3beige, mac99
> 
>   Extract the first character the machine understands (subset of
>   'a'..'f').  Silently ignored otherwise.
> 
> * spapr
> 
>   Accept an arbitrary string (vl.c restricts it to contain only
>   'a'..'p', no duplicates).
> 
> * sun4[mdc]
> 
>   Use the first character.  Silently ignored otherwise.
> 
> Strip characters these machines ignore from their default boot order.
> 
> For all other machines, remove the unused default boot order
> alltogether.
> 
> Note that my rename of QEMUMachine member boot_order to
> default_boot_order and QEMUMachineInitArgs member boot_device to
> boot_order has a welcome side effect: it makes every use of boot
> orders visible in this patch, for easy review.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/alpha/dp264.c                         |  1 -
>  hw/arm/collie.c                          |  1 -
>  hw/arm/exynos4_boards.c                  |  2 --
>  hw/arm/gumstix.c                         |  2 --
>  hw/arm/highbank.c                        |  2 --
>  hw/arm/integratorcp.c                    |  1 -
>  hw/arm/kzm.c                             |  1 -
>  hw/arm/mainstone.c                       |  1 -
>  hw/arm/musicpal.c                        |  1 -
>  hw/arm/nseries.c                         |  6 +++---
>  hw/arm/omap_sx1.c                        |  2 --
>  hw/arm/palm.c                            |  1 -
>  hw/arm/realview.c                        |  4 ----
>  hw/arm/spitz.c                           |  4 ----
>  hw/arm/stellaris.c                       |  2 --
>  hw/arm/tosa.c                            |  1 -
>  hw/arm/versatilepb.c                     |  2 --
>  hw/arm/vexpress.c                        |  2 --
>  hw/arm/xilinx_zynq.c                     |  1 -
>  hw/arm/z2.c                              |  1 -
>  hw/core/null-machine.c                   |  1 -
>  hw/cris/axis_dev88.c                     |  1 -
>  hw/i386/pc_piix.c                        | 32 ++++++++++++++++----------------
>  hw/i386/pc_q35.c                         |  8 ++++----
>  hw/i386/xen_machine_pv.c                 |  1 -
>  hw/lm32/lm32_boards.c                    |  2 --
>  hw/lm32/milkymist.c                      |  1 -
>  hw/m68k/an5206.c                         |  1 -
>  hw/m68k/dummy_m68k.c                     |  1 -
>  hw/m68k/mcf5208.c                        |  1 -
>  hw/microblaze/petalogix_ml605_mmu.c      |  1 -
>  hw/microblaze/petalogix_s3adsp1800_mmu.c |  1 -
>  hw/mips/mips_fulong2e.c                  |  1 -
>  hw/mips/mips_jazz.c                      |  2 --
>  hw/mips/mips_malta.c                     |  1 -
>  hw/mips/mips_mipssim.c                   |  1 -
>  hw/mips/mips_r4k.c                       |  1 -
>  hw/openrisc/openrisc_sim.c               |  1 -
>  hw/ppc/e500plat.c                        |  1 -
>  hw/ppc/mac_newworld.c                    |  4 ++--
>  hw/ppc/mac_oldworld.c                    |  4 ++--
>  hw/ppc/mpc8544ds.c                       |  1 -
>  hw/ppc/ppc405_boards.c                   |  2 --
>  hw/ppc/ppc440_bamboo.c                   |  1 -
>  hw/ppc/prep.c                            |  4 ++--
>  hw/ppc/spapr.c                           |  4 ++--
>  hw/ppc/virtex_ml507.c                    |  1 -
>  hw/s390x/s390-virtio-ccw.c               |  1 -
>  hw/s390x/s390-virtio.c                   |  1 -
>  hw/sh4/r2d.c                             |  1 -
>  hw/sh4/shix.c                            |  1 -
>  hw/sparc/leon3.c                         |  1 -
>  hw/sparc/sun4m.c                         | 22 +++++++++++-----------
>  hw/sparc64/sun4u.c                       | 10 +++++-----
>  hw/unicore32/puv3.c                      |  1 -
>  hw/xtensa/xtensa_lx60.c                  |  2 --
>  hw/xtensa/xtensa_sim.c                   |  1 -
>  include/hw/boards.h                      |  7 ++-----
>  vl.c                                     |  4 ++--
>  59 files changed, 51 insertions(+), 119 deletions(-)
> 
> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
> index 95fde61..20795ac 100644
> --- a/hw/alpha/dp264.c
> +++ b/hw/alpha/dp264.c
> @@ -173,7 +173,6 @@ static QEMUMachine clipper_machine = {
>      .init = clipper_init,
>      .max_cpus = 4,
>      .is_default = 1,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void clipper_machine_init(void)
> diff --git a/hw/arm/collie.c b/hw/arm/collie.c
> index a19857a..8878b0e 100644
> --- a/hw/arm/collie.c
> +++ b/hw/arm/collie.c
> @@ -62,7 +62,6 @@ static QEMUMachine collie_machine = {
>      .name = "collie",
>      .desc = "Collie PDA (SA-1110)",
>      .init = collie_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void collie_machine_init(void)
> diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c
> index 7c90b2d..2929f9f 100644
> --- a/hw/arm/exynos4_boards.c
> +++ b/hw/arm/exynos4_boards.c
> @@ -150,14 +150,12 @@ static QEMUMachine exynos4_machines[EXYNOS4_NUM_OF_BOARDS] = {
>          .desc = "Samsung NURI board (Exynos4210)",
>          .init = nuri_init,
>          .max_cpus = EXYNOS4210_NCPUS,
> -        DEFAULT_MACHINE_OPTIONS,
>      },
>      [EXYNOS4_BOARD_SMDKC210] = {
>          .name = "smdkc210",
>          .desc = "Samsung SMDKC210 board (Exynos4210)",
>          .init = smdkc210_init,
>          .max_cpus = EXYNOS4210_NCPUS,
> -        DEFAULT_MACHINE_OPTIONS,
>      },
>  };
>  
> diff --git a/hw/arm/gumstix.c b/hw/arm/gumstix.c
> index b8cab10..e97fbbd 100644
> --- a/hw/arm/gumstix.c
> +++ b/hw/arm/gumstix.c
> @@ -122,14 +122,12 @@ static QEMUMachine connex_machine = {
>      .name = "connex",
>      .desc = "Gumstix Connex (PXA255)",
>      .init = connex_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine verdex_machine = {
>      .name = "verdex",
>      .desc = "Gumstix Verdex (PXA270)",
>      .init = verdex_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void gumstix_machine_init(void)
> diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
> index be264d3..4dfcfd0 100644
> --- a/hw/arm/highbank.c
> +++ b/hw/arm/highbank.c
> @@ -359,7 +359,6 @@ static QEMUMachine highbank_machine = {
>      .init = highbank_init,
>      .block_default_type = IF_SCSI,
>      .max_cpus = 4,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine midway_machine = {
> @@ -368,7 +367,6 @@ static QEMUMachine midway_machine = {
>      .init = midway_init,
>      .block_default_type = IF_SCSI,
>      .max_cpus = 4,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void calxeda_machines_init(void)
> diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
> index 249a430..5b20859 100644
> --- a/hw/arm/integratorcp.c
> +++ b/hw/arm/integratorcp.c
> @@ -513,7 +513,6 @@ static QEMUMachine integratorcp_machine = {
>      .desc = "ARM Integrator/CP (ARM926EJ-S)",
>      .init = integratorcp_init,
>      .is_default = 1,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void integratorcp_machine_init(void)
> diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
> index bd6c05c..0007923 100644
> --- a/hw/arm/kzm.c
> +++ b/hw/arm/kzm.c
> @@ -146,7 +146,6 @@ static QEMUMachine kzm_machine = {
>      .name = "kzm",
>      .desc = "ARM KZM Emulation Baseboard (ARM1136)",
>      .init = kzm_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void kzm_machine_init(void)
> diff --git a/hw/arm/mainstone.c b/hw/arm/mainstone.c
> index 8e5fc26..b244f7e 100644
> --- a/hw/arm/mainstone.c
> +++ b/hw/arm/mainstone.c
> @@ -179,7 +179,6 @@ static QEMUMachine mainstone2_machine = {
>      .name = "mainstone",
>      .desc = "Mainstone II (PXA27x)",
>      .init = mainstone_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void mainstone_machine_init(void)
> diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
> index b06d442..bed9e16 100644
> --- a/hw/arm/musicpal.c
> +++ b/hw/arm/musicpal.c
> @@ -1681,7 +1681,6 @@ static QEMUMachine musicpal_machine = {
>      .name = "musicpal",
>      .desc = "Marvell 88w8618 / MusicPal (ARM926EJ-S)",
>      .init = musicpal_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void musicpal_machine_init(void)
> diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
> index f6c9dc0..9ef31ca 100644
> --- a/hw/arm/nseries.c
> +++ b/hw/arm/nseries.c
> @@ -1340,7 +1340,7 @@ static void n8x0_init(QEMUMachineInitArgs *args,
>      }
>  
>      if (option_rom[0].name &&
> -        (args->boot_device[0] == 'n' || !args->kernel_filename)) {
> +        (args->boot_order[0] == 'n' || !args->kernel_filename)) {
>          uint8_t nolo_tags[0x10000];
>          /* No, wait, better start at the ROM.  */
>          s->mpu->cpu->env.regs[15] = OMAP2_Q2_BASE + 0x400000;
> @@ -1396,14 +1396,14 @@ static QEMUMachine n800_machine = {
>      .name = "n800",
>      .desc = "Nokia N800 tablet aka. RX-34 (OMAP2420)",
>      .init = n800_init,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "",
>  };
>  
>  static QEMUMachine n810_machine = {
>      .name = "n810",
>      .desc = "Nokia N810 tablet aka. RX-44 (OMAP2420)",
>      .init = n810_init,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "",
>  };
>  
>  static void nseries_machine_init(void)
> diff --git a/hw/arm/omap_sx1.c b/hw/arm/omap_sx1.c
> index 05b0353..b0f8664 100644
> --- a/hw/arm/omap_sx1.c
> +++ b/hw/arm/omap_sx1.c
> @@ -219,14 +219,12 @@ static QEMUMachine sx1_machine_v2 = {
>      .name = "sx1",
>      .desc = "Siemens SX1 (OMAP310) V2",
>      .init = sx1_init_v2,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine sx1_machine_v1 = {
>      .name = "sx1-v1",
>      .desc = "Siemens SX1 (OMAP310) V1",
>      .init = sx1_init_v1,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void sx1_machine_init(void)
> diff --git a/hw/arm/palm.c b/hw/arm/palm.c
> index cdc3c3a..3e39044 100644
> --- a/hw/arm/palm.c
> +++ b/hw/arm/palm.c
> @@ -273,7 +273,6 @@ static QEMUMachine palmte_machine = {
>      .name = "cheetah",
>      .desc = "Palm Tungsten|E aka. Cheetah PDA (OMAP310)",
>      .init = palmte_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void palmte_machine_init(void)
> diff --git a/hw/arm/realview.c b/hw/arm/realview.c
> index 3060f48..1f4e5b0 100644
> --- a/hw/arm/realview.c
> +++ b/hw/arm/realview.c
> @@ -371,7 +371,6 @@ static QEMUMachine realview_eb_machine = {
>      .desc = "ARM RealView Emulation Baseboard (ARM926EJ-S)",
>      .init = realview_eb_init,
>      .block_default_type = IF_SCSI,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine realview_eb_mpcore_machine = {
> @@ -380,14 +379,12 @@ static QEMUMachine realview_eb_mpcore_machine = {
>      .init = realview_eb_mpcore_init,
>      .block_default_type = IF_SCSI,
>      .max_cpus = 4,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine realview_pb_a8_machine = {
>      .name = "realview-pb-a8",
>      .desc = "ARM RealView Platform Baseboard for Cortex-A8",
>      .init = realview_pb_a8_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine realview_pbx_a9_machine = {
> @@ -396,7 +393,6 @@ static QEMUMachine realview_pbx_a9_machine = {
>      .init = realview_pbx_a9_init,
>      .block_default_type = IF_SCSI,
>      .max_cpus = 4,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void realview_machine_init(void)
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index 593b75e..5062020 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -959,28 +959,24 @@ static QEMUMachine akitapda_machine = {
>      .name = "akita",
>      .desc = "Akita PDA (PXA270)",
>      .init = akita_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine spitzpda_machine = {
>      .name = "spitz",
>      .desc = "Spitz PDA (PXA270)",
>      .init = spitz_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine borzoipda_machine = {
>      .name = "borzoi",
>      .desc = "Borzoi PDA (PXA270)",
>      .init = borzoi_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine terrierpda_machine = {
>      .name = "terrier",
>      .desc = "Terrier PDA (PXA270)",
>      .init = terrier_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void spitz_machine_init(void)
> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index a2b6b17..3b9bca6 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -1331,14 +1331,12 @@ static QEMUMachine lm3s811evb_machine = {
>      .name = "lm3s811evb",
>      .desc = "Stellaris LM3S811EVB",
>      .init = lm3s811evb_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine lm3s6965evb_machine = {
>      .name = "lm3s6965evb",
>      .desc = "Stellaris LM3S6965EVB",
>      .init = lm3s6965evb_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void stellaris_machine_init(void)
> diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
> index 47d1f4f..c00d8c2 100644
> --- a/hw/arm/tosa.c
> +++ b/hw/arm/tosa.c
> @@ -251,7 +251,6 @@ static QEMUMachine tosapda_machine = {
>      .name = "tosa",
>      .desc = "Tosa PDA (PXA255)",
>      .init = tosa_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void tosapda_machine_init(void)
> diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
> index 725f60f..326bf63 100644
> --- a/hw/arm/versatilepb.c
> +++ b/hw/arm/versatilepb.c
> @@ -363,7 +363,6 @@ static QEMUMachine versatilepb_machine = {
>      .desc = "ARM Versatile/PB (ARM926EJ-S)",
>      .init = vpb_init,
>      .block_default_type = IF_SCSI,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine versatileab_machine = {
> @@ -371,7 +370,6 @@ static QEMUMachine versatileab_machine = {
>      .desc = "ARM Versatile/AB (ARM926EJ-S)",
>      .init = vab_init,
>      .block_default_type = IF_SCSI,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void versatile_machine_init(void)
> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
> index 7d1b823..072e91c 100644
> --- a/hw/arm/vexpress.c
> +++ b/hw/arm/vexpress.c
> @@ -553,7 +553,6 @@ static QEMUMachine vexpress_a9_machine = {
>      .init = vexpress_a9_init,
>      .block_default_type = IF_SCSI,
>      .max_cpus = 4,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine vexpress_a15_machine = {
> @@ -562,7 +561,6 @@ static QEMUMachine vexpress_a15_machine = {
>      .init = vexpress_a15_init,
>      .block_default_type = IF_SCSI,
>      .max_cpus = 4,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void vexpress_machine_init(void)
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 3444823..608686a 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -236,7 +236,6 @@ static QEMUMachine zynq_machine = {
>      .block_default_type = IF_SCSI,
>      .max_cpus = 1,
>      .no_sdcard = 1,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void zynq_machine_init(void)
> diff --git a/hw/arm/z2.c b/hw/arm/z2.c
> index 07a127b..2e0d5d4 100644
> --- a/hw/arm/z2.c
> +++ b/hw/arm/z2.c
> @@ -373,7 +373,6 @@ static QEMUMachine z2_machine = {
>      .name = "z2",
>      .desc = "Zipit Z2 (PXA27x)",
>      .init = z2_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void z2_machine_init(void)
> diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c
> index bdf109f..d813c08 100644
> --- a/hw/core/null-machine.c
> +++ b/hw/core/null-machine.c
> @@ -24,7 +24,6 @@ static QEMUMachine machine_none = {
>      .desc = "empty machine",
>      .init = machine_none_init,
>      .max_cpus = 0,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void register_machines(void)
> diff --git a/hw/cris/axis_dev88.c b/hw/cris/axis_dev88.c
> index 9104d61..03058d3 100644
> --- a/hw/cris/axis_dev88.c
> +++ b/hw/cris/axis_dev88.c
> @@ -355,7 +355,6 @@ static QEMUMachine axisdev88_machine = {
>      .desc = "AXIS devboard 88",
>      .init = axisdev88_init,
>      .is_default = 1,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void axisdev88_machine_init(void)
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 9327ac1..3700bd5 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -216,7 +216,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
>          }
>      }
>  
> -    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
> +    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
>                   floppy, idebus[0], idebus[1], rtc_state);
>  
>      if (pci_enabled && usb_enabled(false)) {
> @@ -324,7 +324,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
>      .hot_add_cpu = pc_hot_add_cpu,
>      .max_cpus = 255,
>      .is_default = 1,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "cad",
>  };
>  
>  static QEMUMachine pc_i440fx_machine_v1_5 = {
> @@ -337,7 +337,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
>          PC_COMPAT_1_5,
>          { /* end of list */ }
>      },
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "cad",
>  };
>  
>  static QEMUMachine pc_i440fx_machine_v1_4 = {

So all PC machine types share this?
Can we set this in some common code, somehow?


> @@ -345,11 +345,11 @@ static QEMUMachine pc_i440fx_machine_v1_4 = {
>      .desc = "Standard PC (i440FX + PIIX, 1996)",
>      .init = pc_init_pci_1_4,
>      .max_cpus = 255,
> +    .default_boot_order = "cad",
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_1_4,
>          { /* end of list */ }
>      },
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  #define PC_COMPAT_1_3 \
> @@ -377,11 +377,11 @@ static QEMUMachine pc_machine_v1_3 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_1_3,
>      .max_cpus = 255,
> +    .default_boot_order = "cad",
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_1_3,
>          { /* end of list */ }
>      },
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  #define PC_COMPAT_1_2 \
> @@ -417,11 +417,11 @@ static QEMUMachine pc_machine_v1_2 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_1_2,
>      .max_cpus = 255,
> +    .default_boot_order = "cad",
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_1_2,
>          { /* end of list */ }
>      },
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  #define PC_COMPAT_1_1 \
> @@ -461,11 +461,11 @@ static QEMUMachine pc_machine_v1_1 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_1_2,
>      .max_cpus = 255,
> +    .default_boot_order = "cad",
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_1_1,
>          { /* end of list */ }
>      },
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  #define PC_COMPAT_1_0 \
> @@ -497,12 +497,12 @@ static QEMUMachine pc_machine_v1_0 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_1_0,
>      .max_cpus = 255,
> +    .default_boot_order = "cad",
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_1_0,
>          { /* end of list */ }
>      },
>      .hw_version = "1.0",
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  #define PC_COMPAT_0_15 \
> @@ -513,12 +513,12 @@ static QEMUMachine pc_machine_v0_15 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_1_0,
>      .max_cpus = 255,
> +    .default_boot_order = "cad",
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_0_15,
>          { /* end of list */ }
>      },
>      .hw_version = "0.15",
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  #define PC_COMPAT_0_14 \
> @@ -546,6 +546,7 @@ static QEMUMachine pc_machine_v0_14 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_1_0,
>      .max_cpus = 255,
> +    .default_boot_order = "cad",
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_0_14, 
>          {
> @@ -560,7 +561,6 @@ static QEMUMachine pc_machine_v0_14 = {
>          { /* end of list */ }
>      },
>      .hw_version = "0.14",
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  #define PC_COMPAT_0_13 \
> @@ -580,6 +580,7 @@ static QEMUMachine pc_machine_v0_13 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_no_kvmclock,
>      .max_cpus = 255,
> +    .default_boot_order = "cad",
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_0_13,
>          {
> @@ -598,7 +599,6 @@ static QEMUMachine pc_machine_v0_13 = {
>          { /* end of list */ }
>      },
>      .hw_version = "0.13",
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  #define PC_COMPAT_0_12 \
> @@ -630,6 +630,7 @@ static QEMUMachine pc_machine_v0_12 = {
>      .desc = "Standard PC",
>      .init = pc_init_pci_no_kvmclock,
>      .max_cpus = 255,
> +    .default_boot_order = "cad",
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_0_12,
>          {
> @@ -644,7 +645,6 @@ static QEMUMachine pc_machine_v0_12 = {
>          { /* end of list */ }
>      },
>      .hw_version = "0.12",
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  #define PC_COMPAT_0_11 \
> @@ -664,6 +664,7 @@ static QEMUMachine pc_machine_v0_11 = {
>      .desc = "Standard PC, qemu 0.11",
>      .init = pc_init_pci_no_kvmclock,
>      .max_cpus = 255,
> +    .default_boot_order = "cad",
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_0_11,
>          {
> @@ -678,7 +679,6 @@ static QEMUMachine pc_machine_v0_11 = {
>          { /* end of list */ }
>      },
>      .hw_version = "0.11",
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine pc_machine_v0_10 = {
> @@ -686,6 +686,7 @@ static QEMUMachine pc_machine_v0_10 = {
>      .desc = "Standard PC, qemu 0.10",
>      .init = pc_init_pci_no_kvmclock,
>      .max_cpus = 255,
> +    .default_boot_order = "cad",
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_0_11,
>          {
> @@ -712,7 +713,6 @@ static QEMUMachine pc_machine_v0_10 = {
>          { /* end of list */ }
>      },
>      .hw_version = "0.10",
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine isapc_machine = {
> @@ -720,6 +720,7 @@ static QEMUMachine isapc_machine = {
>      .desc = "ISA-only PC",
>      .init = pc_init_isa,
>      .max_cpus = 1,
> +    .default_boot_order = "cad",
>      .compat_props = (GlobalProperty[]) {
>          {
>              .driver   = "pc-sysfw",
> @@ -733,7 +734,6 @@ static QEMUMachine isapc_machine = {
>          },
>          { /* end of list */ }
>      },
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  #ifdef CONFIG_XEN
> @@ -743,7 +743,7 @@ static QEMUMachine xenfv_machine = {
>      .init = pc_xen_hvm_init,
>      .max_cpus = HVM_MAX_VCPUS,
>      .default_machine_opts = "accel=xen",
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "cad",
>  };
>  #endif
>  
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 1657eb3..28ecf76 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -195,7 +195,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
>                                      0xb100),
>                        8, NULL, 0);
>  
> -    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
> +    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
>                   floppy, idebus[0], idebus[1], rtc_state);
>  
>      /* the rest devices to which pci devfn is automatically assigned */
> @@ -230,7 +230,7 @@ static QEMUMachine pc_q35_machine_v1_6 = {
>      .init = pc_q35_init,
>      .hot_add_cpu = pc_hot_add_cpu,
>      .max_cpus = 255,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "cad",
>  };
>  
>  static QEMUMachine pc_q35_machine_v1_5 = {
> @@ -239,11 +239,11 @@ static QEMUMachine pc_q35_machine_v1_5 = {
>      .init = pc_q35_init_1_5,
>      .hot_add_cpu = pc_hot_add_cpu,
>      .max_cpus = 255,
> +    .default_boot_order = "cad",
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_1_5,
>          { /* end of list */ }
>      },
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine pc_q35_machine_v1_4 = {
> @@ -251,11 +251,11 @@ static QEMUMachine pc_q35_machine_v1_4 = {
>      .desc = "Standard PC (Q35 + ICH9, 2009)",
>      .init = pc_q35_init_1_4,
>      .max_cpus = 255,
> +    .default_boot_order = "cad",
>      .compat_props = (GlobalProperty[]) {
>          PC_COMPAT_1_4,
>          { /* end of list */ }
>      },
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void pc_q35_machine_init(void)
> diff --git a/hw/i386/xen_machine_pv.c b/hw/i386/xen_machine_pv.c
> index 9f2e291..9adb57f 100644
> --- a/hw/i386/xen_machine_pv.c
> +++ b/hw/i386/xen_machine_pv.c
> @@ -99,7 +99,6 @@ static QEMUMachine xenpv_machine = {
>      .init = xen_init_pv,
>      .max_cpus = 1,
>      .default_machine_opts = "accel=xen",
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void xenpv_machine_init(void)
> diff --git a/hw/lm32/lm32_boards.c b/hw/lm32/lm32_boards.c
> index 62003b8..c032bb8 100644
> --- a/hw/lm32/lm32_boards.c
> +++ b/hw/lm32/lm32_boards.c
> @@ -289,7 +289,6 @@ static QEMUMachine lm32_evr_machine = {
>      .desc = "LatticeMico32 EVR32 eval system",
>      .init = lm32_evr_init,
>      .is_default = 1,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine lm32_uclinux_machine = {
> @@ -297,7 +296,6 @@ static QEMUMachine lm32_uclinux_machine = {
>      .desc = "lm32 platform for uClinux and u-boot by Theobroma Systems",
>      .init = lm32_uclinux_init,
>      .is_default = 0,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void lm32_machine_init(void)
> diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
> index 7ceedb8..f1744ec 100644
> --- a/hw/lm32/milkymist.c
> +++ b/hw/lm32/milkymist.c
> @@ -208,7 +208,6 @@ static QEMUMachine milkymist_machine = {
>      .desc = "Milkymist One",
>      .init = milkymist_init,
>      .is_default = 0,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void milkymist_machine_init(void)
> diff --git a/hw/m68k/an5206.c b/hw/m68k/an5206.c
> index 0c03a87..a8eee44 100644
> --- a/hw/m68k/an5206.c
> +++ b/hw/m68k/an5206.c
> @@ -89,7 +89,6 @@ static QEMUMachine an5206_machine = {
>      .name = "an5206",
>      .desc = "Arnewsh 5206",
>      .init = an5206_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void an5206_machine_init(void)
> diff --git a/hw/m68k/dummy_m68k.c b/hw/m68k/dummy_m68k.c
> index f4ed7c6..86e2e6e 100644
> --- a/hw/m68k/dummy_m68k.c
> +++ b/hw/m68k/dummy_m68k.c
> @@ -73,7 +73,6 @@ static QEMUMachine dummy_m68k_machine = {
>      .name = "dummy",
>      .desc = "Dummy board",
>      .init = dummy_m68k_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void dummy_m68k_machine_init(void)
> diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
> index 9cf000f..fb96fe8 100644
> --- a/hw/m68k/mcf5208.c
> +++ b/hw/m68k/mcf5208.c
> @@ -295,7 +295,6 @@ static QEMUMachine mcf5208evb_machine = {
>      .desc = "MCF5206EVB",
>      .init = mcf5208evb_init,
>      .is_default = 1,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void mcf5208evb_machine_init(void)
> diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
> index 989da25..e003c7c 100644
> --- a/hw/microblaze/petalogix_ml605_mmu.c
> +++ b/hw/microblaze/petalogix_ml605_mmu.c
> @@ -186,7 +186,6 @@ static QEMUMachine petalogix_ml605_machine = {
>      .desc = "PetaLogix linux refdesign for xilinx ml605 little endian",
>      .init = petalogix_ml605_init,
>      .is_default = 0,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void petalogix_ml605_machine_init(void)
> diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> index a461494..00af2b5 100644
> --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
> +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
> @@ -116,7 +116,6 @@ static QEMUMachine petalogix_s3adsp1800_machine = {
>      .desc = "PetaLogix linux refdesign for xilinx Spartan 3ADSP1800",
>      .init = petalogix_s3adsp1800_init,
>      .is_default = 1,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void petalogix_s3adsp1800_machine_init(void)
> diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
> index 9e305d2..bf9ae4a 100644
> --- a/hw/mips/mips_fulong2e.c
> +++ b/hw/mips/mips_fulong2e.c
> @@ -400,7 +400,6 @@ static QEMUMachine mips_fulong2e_machine = {
>      .name = "fulong2e",
>      .desc = "Fulong 2e mini pc",
>      .init = mips_fulong2e_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void mips_fulong2e_machine_init(void)
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index 31e138b..bf12faa 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -323,7 +323,6 @@ static QEMUMachine mips_magnum_machine = {
>      .desc = "MIPS Magnum",
>      .init = mips_magnum_init,
>      .block_default_type = IF_SCSI,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine mips_pica61_machine = {
> @@ -331,7 +330,6 @@ static QEMUMachine mips_pica61_machine = {
>      .desc = "Acer Pica 61",
>      .init = mips_pica61_init,
>      .block_default_type = IF_SCSI,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void mips_jazz_machine_init(void)
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index de87241..cb22279 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -1017,7 +1017,6 @@ static QEMUMachine mips_malta_machine = {
>      .init = mips_malta_init,
>      .max_cpus = 16,
>      .is_default = 1,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void mips_malta_register_types(void)
> diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
> index a10cf13..ea5868c 100644
> --- a/hw/mips/mips_mipssim.c
> +++ b/hw/mips/mips_mipssim.c
> @@ -229,7 +229,6 @@ static QEMUMachine mips_mipssim_machine = {
>      .name = "mipssim",
>      .desc = "MIPS MIPSsim platform",
>      .init = mips_mipssim_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void mips_mipssim_machine_init(void)
> diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
> index 22beb0a..cf569c1 100644
> --- a/hw/mips/mips_r4k.c
> +++ b/hw/mips/mips_r4k.c
> @@ -302,7 +302,6 @@ static QEMUMachine mips_machine = {
>      .name = "mips",
>      .desc = "mips r4k platform",
>      .init = mips_r4k_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void mips_machine_init(void)
> diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
> index 924438b..2e32bea 100644
> --- a/hw/openrisc/openrisc_sim.c
> +++ b/hw/openrisc/openrisc_sim.c
> @@ -139,7 +139,6 @@ static QEMUMachine openrisc_sim_machine = {
>      .init = openrisc_sim_init,
>      .max_cpus = 1,
>      .is_default = 1,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void openrisc_sim_machine_init(void)
> diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
> index bf65b69..2e964b2 100644
> --- a/hw/ppc/e500plat.c
> +++ b/hw/ppc/e500plat.c
> @@ -51,7 +51,6 @@ static QEMUMachine e500plat_machine = {
>      .desc = "generic paravirt e500 platform",
>      .init = e500plat_init,
>      .max_cpus = 32,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void e500plat_machine_init(void)
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index fe80348..670c473 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -147,7 +147,7 @@ static void ppc_core99_init(QEMUMachineInitArgs *args)
>      const char *kernel_filename = args->kernel_filename;
>      const char *kernel_cmdline = args->kernel_cmdline;
>      const char *initrd_filename = args->initrd_filename;
> -    const char *boot_device = args->boot_device;
> +    const char *boot_device = args->boot_order;
>      PowerPCCPU *cpu = NULL;
>      CPUPPCState *env = NULL;
>      char *filename;
> @@ -474,7 +474,7 @@ static QEMUMachine core99_machine = {
>      .desc = "Mac99 based PowerMAC",
>      .init = ppc_core99_init,
>      .max_cpus = MAX_CPUS,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "cd",
>  };
>  
>  static void core99_machine_init(void)
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 8b8c6b9..a3ec9a5 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -78,7 +78,7 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args)
>      const char *kernel_filename = args->kernel_filename;
>      const char *kernel_cmdline = args->kernel_cmdline;
>      const char *initrd_filename = args->initrd_filename;
> -    const char *boot_device = args->boot_device;
> +    const char *boot_device = args->boot_order;
>      MemoryRegion *sysmem = get_system_memory();
>      PowerPCCPU *cpu = NULL;
>      CPUPPCState *env = NULL;
> @@ -347,7 +347,7 @@ static QEMUMachine heathrow_machine = {
>  #ifndef TARGET_PPC64
>      .is_default = 1,
>  #endif
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "cd", /* TOFIX "cad" when Mac floppy is implemented */
>  };
>  
>  static void heathrow_machine_init(void)
> diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
> index 1888e75..edcc0be 100644
> --- a/hw/ppc/mpc8544ds.c
> +++ b/hw/ppc/mpc8544ds.c
> @@ -44,7 +44,6 @@ static QEMUMachine ppce500_machine = {
>      .desc = "mpc8544ds",
>      .init = mpc8544ds_init,
>      .max_cpus = 15,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void ppce500_machine_init(void)
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index f74e5e5..43a83ca 100644
> --- a/hw/ppc/ppc405_boards.c
> +++ b/hw/ppc/ppc405_boards.c
> @@ -362,7 +362,6 @@ static QEMUMachine ref405ep_machine = {
>      .name = "ref405ep",
>      .desc = "ref405ep",
>      .init = ref405ep_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  /*****************************************************************************/
> @@ -650,7 +649,6 @@ static QEMUMachine taihu_machine = {
>      .name = "taihu",
>      .desc = "taihu",
>      .init = taihu_405ep_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void ppc405_machine_init(void)
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index 5b039ab..698ec87 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -293,7 +293,6 @@ static QEMUMachine bamboo_machine = {
>      .name = "bamboo",
>      .desc = "bamboo",
>      .init = bamboo_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void bamboo_machine_init(void)
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 19f2442..e884d3e 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -452,7 +452,7 @@ static void ppc_prep_init(QEMUMachineInitArgs *args)
>      const char *kernel_filename = args->kernel_filename;
>      const char *kernel_cmdline = args->kernel_cmdline;
>      const char *initrd_filename = args->initrd_filename;
> -    const char *boot_device = args->boot_device;
> +    const char *boot_device = args->boot_order;
>      MemoryRegion *sysmem = get_system_memory();
>      PowerPCCPU *cpu = NULL;
>      CPUPPCState *env = NULL;
> @@ -691,7 +691,7 @@ static QEMUMachine prep_machine = {
>      .desc = "PowerPC PREP platform",
>      .init = ppc_prep_init,
>      .max_cpus = MAX_CPUS,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "cad",
>  };
>  
>  static void prep_machine_init(void)
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 48ae092..805261a 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -718,7 +718,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>      const char *kernel_filename = args->kernel_filename;
>      const char *kernel_cmdline = args->kernel_cmdline;
>      const char *initrd_filename = args->initrd_filename;
> -    const char *boot_device = args->boot_device;
> +    const char *boot_device = args->boot_order;
>      PowerPCCPU *cpu;
>      CPUPPCState *env;
>      PCIHostState *phb;
> @@ -971,7 +971,7 @@ static QEMUMachine spapr_machine = {
>      .block_default_type = IF_SCSI,
>      .max_cpus = MAX_CPUS,
>      .no_parallel = 1,
> -    .boot_order = NULL,
> +    .default_boot_order = NULL,
>  };
>  
>  static void spapr_machine_init(void)
> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index 08e77fb..7250f51 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -245,7 +245,6 @@ static QEMUMachine virtex_machine = {
>      .name = "virtex-ml507",
>      .desc = "Xilinx Virtex ML507 reference design",
>      .init = virtex_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void virtex_machine_init(void)
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index aebbbf1..e2681a6 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -126,7 +126,6 @@ static QEMUMachine ccw_machine = {
>      .no_sdcard = 1,
>      .use_sclp = 1,
>      .max_cpus = 255,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void ccw_machine_init(void)
> diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
> index edbde00..df52f1f 100644
> --- a/hw/s390x/s390-virtio.c
> +++ b/hw/s390x/s390-virtio.c
> @@ -293,7 +293,6 @@ static QEMUMachine s390_machine = {
>      .use_virtcon = 1,
>      .max_cpus = 255,
>      .is_default = 1,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void s390_machine_init(void)
> diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
> index 98b3408..7b1de85 100644
> --- a/hw/sh4/r2d.c
> +++ b/hw/sh4/r2d.c
> @@ -356,7 +356,6 @@ static QEMUMachine r2d_machine = {
>      .name = "r2d",
>      .desc = "r2d-plus board",
>      .init = r2d_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void r2d_machine_init(void)
> diff --git a/hw/sh4/shix.c b/hw/sh4/shix.c
> index 84dd666..1ff37f5 100644
> --- a/hw/sh4/shix.c
> +++ b/hw/sh4/shix.c
> @@ -96,7 +96,6 @@ static QEMUMachine shix_machine = {
>      .desc = "shix card",
>      .init = shix_init,
>      .is_default = 1,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void shix_machine_init(void)
> diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
> index 5ef282f..390f3e4 100644
> --- a/hw/sparc/leon3.c
> +++ b/hw/sparc/leon3.c
> @@ -216,7 +216,6 @@ static QEMUMachine leon3_generic_machine = {
>      .name     = "leon3_generic",
>      .desc     = "Leon-3 generic",
>      .init     = leon3_generic_hw_init,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void leon3_machine_init(void)
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 97c3ee0..ad09312 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -976,7 +976,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>                                      args->ram_size);
>  
>      nvram_init(nvram, (uint8_t *)&nd_table[0].macaddr, args->kernel_cmdline,
> -               args->boot_device, args->ram_size, kernel_size, graphic_width,
> +               args->boot_order, args->ram_size, kernel_size, graphic_width,
>                 graphic_height, graphic_depth, hwdef->nvram_machine_id,
>                 "Sun4m");
>  
> @@ -1005,7 +1005,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      }
>      fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, INITRD_LOAD_ADDR);
>      fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, 0); // not used
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_device[0]);
> +    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_order[0]);
>      qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
>  }
>  
> @@ -1326,7 +1326,7 @@ static QEMUMachine ss5_machine = {
>      .init = ss5_init,
>      .block_default_type = IF_SCSI,
>      .is_default = 1,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "c",
>  };
>  
>  static QEMUMachine ss10_machine = {
> @@ -1335,7 +1335,7 @@ static QEMUMachine ss10_machine = {
>      .init = ss10_init,
>      .block_default_type = IF_SCSI,
>      .max_cpus = 4,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "c",
>  };
>  
>  static QEMUMachine ss600mp_machine = {
> @@ -1344,7 +1344,7 @@ static QEMUMachine ss600mp_machine = {
>      .init = ss600mp_init,
>      .block_default_type = IF_SCSI,
>      .max_cpus = 4,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "c",
>  };
>  
>  static QEMUMachine ss20_machine = {
> @@ -1353,7 +1353,7 @@ static QEMUMachine ss20_machine = {
>      .init = ss20_init,
>      .block_default_type = IF_SCSI,
>      .max_cpus = 4,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "c",
>  };
>  
>  static QEMUMachine voyager_machine = {
> @@ -1361,7 +1361,7 @@ static QEMUMachine voyager_machine = {
>      .desc = "Sun4m platform, SPARCstation Voyager",
>      .init = vger_init,
>      .block_default_type = IF_SCSI,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "c",
>  };
>  
>  static QEMUMachine ss_lx_machine = {
> @@ -1369,7 +1369,7 @@ static QEMUMachine ss_lx_machine = {
>      .desc = "Sun4m platform, SPARCstation LX",
>      .init = ss_lx_init,
>      .block_default_type = IF_SCSI,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "c",
>  };
>  
>  static QEMUMachine ss4_machine = {
> @@ -1377,7 +1377,7 @@ static QEMUMachine ss4_machine = {
>      .desc = "Sun4m platform, SPARCstation 4",
>      .init = ss4_init,
>      .block_default_type = IF_SCSI,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "c",
>  };
>  
>  static QEMUMachine scls_machine = {
> @@ -1385,7 +1385,7 @@ static QEMUMachine scls_machine = {
>      .desc = "Sun4m platform, SPARCClassic",
>      .init = scls_init,
>      .block_default_type = IF_SCSI,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "c",
>  };
>  
>  static QEMUMachine sbook_machine = {
> @@ -1393,7 +1393,7 @@ static QEMUMachine sbook_machine = {
>      .desc = "Sun4m platform, SPARCbook",
>      .init = sbook_init,
>      .block_default_type = IF_SCSI,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "c",
>  };
>  
>  static void sun4m_register_types(void)
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 22e9818..9f726b4 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -872,7 +872,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>                                      &kernel_addr, &kernel_entry);
>  
>      sun4u_NVRAM_set_params(nvram, NVRAM_SIZE, "Sun4u", args->ram_size,
> -                           args->boot_device,
> +                           args->boot_order,
>                             kernel_addr, kernel_size,
>                             args->kernel_cmdline,
>                             initrd_addr, initrd_size,
> @@ -897,7 +897,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      }
>      fw_cfg_add_i64(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
>      fw_cfg_add_i64(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
> -    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_device[0]);
> +    fw_cfg_add_i16(fw_cfg, FW_CFG_BOOT_DEVICE, args->boot_order[0]);
>  
>      fw_cfg_add_i16(fw_cfg, FW_CFG_SPARC64_WIDTH, graphic_width);
>      fw_cfg_add_i16(fw_cfg, FW_CFG_SPARC64_HEIGHT, graphic_height);
> @@ -960,7 +960,7 @@ static QEMUMachine sun4u_machine = {
>      .init = sun4u_init,
>      .max_cpus = 1, // XXX for now
>      .is_default = 1,
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "c",
>  };
>  
>  static QEMUMachine sun4v_machine = {
> @@ -968,7 +968,7 @@ static QEMUMachine sun4v_machine = {
>      .desc = "Sun4v platform",
>      .init = sun4v_init,
>      .max_cpus = 1, // XXX for now
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "c",
>  };
>  
>  static QEMUMachine niagara_machine = {
> @@ -976,7 +976,7 @@ static QEMUMachine niagara_machine = {
>      .desc = "Sun4v platform, Niagara",
>      .init = niagara_init,
>      .max_cpus = 1, // XXX for now
> -    DEFAULT_MACHINE_OPTIONS,
> +    .default_boot_order = "c",
>  };
>  
>  static void sun4u_register_types(void)
> diff --git a/hw/unicore32/puv3.c b/hw/unicore32/puv3.c
> index 5ff0dc9..a900061 100644
> --- a/hw/unicore32/puv3.c
> +++ b/hw/unicore32/puv3.c
> @@ -128,7 +128,6 @@ static QEMUMachine puv3_machine = {
>      .desc = "PKUnity Version-3 based on UniCore32",
>      .init = puv3_init,
>      .is_default = 1,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void puv3_machine_init(void)
> diff --git a/hw/xtensa/xtensa_lx60.c b/hw/xtensa/xtensa_lx60.c
> index 075daf1..3d2f990 100644
> --- a/hw/xtensa/xtensa_lx60.c
> +++ b/hw/xtensa/xtensa_lx60.c
> @@ -295,7 +295,6 @@ static QEMUMachine xtensa_lx60_machine = {
>      .desc = "lx60 EVB (" XTENSA_DEFAULT_CPU_MODEL ")",
>      .init = xtensa_lx60_init,
>      .max_cpus = 4,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static QEMUMachine xtensa_lx200_machine = {
> @@ -303,7 +302,6 @@ static QEMUMachine xtensa_lx200_machine = {
>      .desc = "lx200 EVB (" XTENSA_DEFAULT_CPU_MODEL ")",
>      .init = xtensa_lx200_init,
>      .max_cpus = 4,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void xtensa_lx_machines_init(void)
> diff --git a/hw/xtensa/xtensa_sim.c b/hw/xtensa/xtensa_sim.c
> index a88707e..af9af10 100644
> --- a/hw/xtensa/xtensa_sim.c
> +++ b/hw/xtensa/xtensa_sim.c
> @@ -106,7 +106,6 @@ static QEMUMachine xtensa_sim_machine = {
>      .is_default = true,
>      .init = xtensa_sim_init,
>      .max_cpus = 4,
> -    DEFAULT_MACHINE_OPTIONS,
>  };
>  
>  static void xtensa_sim_machine_init(void)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index fb7c6f1..5a7ae9f 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -6,12 +6,9 @@
>  #include "sysemu/blockdev.h"
>  #include "hw/qdev.h"
>  
> -#define DEFAULT_MACHINE_OPTIONS \
> -    .boot_order = "cad"
> -
>  typedef struct QEMUMachineInitArgs {
>      ram_addr_t ram_size;
> -    const char *boot_device;
> +    const char *boot_order;
>      const char *kernel_filename;
>      const char *kernel_cmdline;
>      const char *initrd_filename;
> @@ -42,7 +39,7 @@ typedef struct QEMUMachine {
>          no_sdcard:1;
>      int is_default;
>      const char *default_machine_opts;
> -    const char *boot_order;
> +    const char *default_boot_order;
>      GlobalProperty *compat_props;
>      struct QEMUMachine *next;
>      const char *hw_version;
> diff --git a/vl.c b/vl.c
> index 25b8f2f..dcd7661 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4121,7 +4121,7 @@ int main(int argc, char **argv, char **envp)
>      kernel_cmdline = qemu_opt_get(machine_opts, "append");
>  
>      if (!boot_order) {
> -        boot_order = machine->boot_order;
> +        boot_order = machine->default_boot_order;
>      }
>      opts = qemu_opts_find(qemu_find_opts("boot-opts"), NULL);
>      if (opts) {
> @@ -4309,7 +4309,7 @@ int main(int argc, char **argv, char **envp)
>      qdev_machine_init();
>  
>      QEMUMachineInitArgs args = { .ram_size = ram_size,
> -                                 .boot_device = boot_order,
> +                                 .boot_order = boot_order,
>                                   .kernel_filename = kernel_filename,
>                                   .kernel_cmdline = kernel_cmdline,
>                                   .initrd_filename = initrd_filename,
> -- 
> 1.7.11.7
> 

Re: [Qemu-devel] [PATCH v3 6/6] hw: Clean up bogus default boot order

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Mon, Jul 22, 2013 at 01:38:47PM +0200, Markus Armbruster wrote:
>> We set default boot order "cad" in every single machine definition
>> except "pseries" and "moxiesim", even though very few boards actually
>> care for boot order, and "cad" makes sense for even fewer.
>> 
>> Machines that care:
>> 
>> * pc and its variants
>> 
>>   Accept up to three letters 'a', 'b' (undocumented alias for 'a'),
>>   'c', 'd' and 'n'.  Reject all others (fatal with -boot).
>> 
>> * nseries (n800, n810)
>> 
>>   Check whether order starts with 'n'.  Silently ignored otherwise.
>> 
>> * prep, g3beige, mac99
>> 
>>   Extract the first character the machine understands (subset of
>>   'a'..'f').  Silently ignored otherwise.
>> 
>> * spapr
>> 
>>   Accept an arbitrary string (vl.c restricts it to contain only
>>   'a'..'p', no duplicates).
>> 
>> * sun4[mdc]
>> 
>>   Use the first character.  Silently ignored otherwise.
>> 
>> Strip characters these machines ignore from their default boot order.
>> 
>> For all other machines, remove the unused default boot order
>> alltogether.
>> 
>> Note that my rename of QEMUMachine member boot_order to
>> default_boot_order and QEMUMachineInitArgs member boot_device to
>> boot_order has a welcome side effect: it makes every use of boot
>> orders visible in this patch, for easy review.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
[...]
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 9327ac1..3700bd5 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -216,7 +216,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
>>          }
>>      }
>>  
>> -    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
>> +    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
>>                   floppy, idebus[0], idebus[1], rtc_state);
>>  
>>      if (pci_enabled && usb_enabled(false)) {
>> @@ -324,7 +324,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
>>      .hot_add_cpu = pc_hot_add_cpu,
>>      .max_cpus = 255,
>>      .is_default = 1,
>> -    DEFAULT_MACHINE_OPTIONS,
>> +    .default_boot_order = "cad",
>>  };
>>  
>>  static QEMUMachine pc_i440fx_machine_v1_5 = {
>> @@ -337,7 +337,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
>>          PC_COMPAT_1_5,
>>          { /* end of list */ }
>>      },
>> -    DEFAULT_MACHINE_OPTIONS,
>> +    .default_boot_order = "cad",
>>  };
>>  
>>  static QEMUMachine pc_i440fx_machine_v1_4 = {
>
> So all PC machine types share this?

Correct, just like they share DEFAULT_MACHINE_OPTIONS before my patch.
Which is defined as

    #define DEFAULT_MACHINE_OPTIONS \
        .boot_order = "cad"

I.e. my patch merely peels off a layer of obfuscation :)

> Can we set this in some common code, somehow?

We don't have an inheritance notion for machine types.

vl.c uses machine->boot_order before calling one of its methods, so
monkey-patching .boot_order from a method won't do.  Besides, that cure
looks much worse than the disease to me.

Can't think of anything else offhand.

[...]

Re: [Qemu-devel] [PATCH v3 6/6] hw: Clean up bogus default boot order

[Michael S. Tsirkin]

On Wed, Aug 21, 2013 at 05:10:56PM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Mon, Jul 22, 2013 at 01:38:47PM +0200, Markus Armbruster wrote:
> >> We set default boot order "cad" in every single machine definition
> >> except "pseries" and "moxiesim", even though very few boards actually
> >> care for boot order, and "cad" makes sense for even fewer.
> >> 
> >> Machines that care:
> >> 
> >> * pc and its variants
> >> 
> >>   Accept up to three letters 'a', 'b' (undocumented alias for 'a'),
> >>   'c', 'd' and 'n'.  Reject all others (fatal with -boot).
> >> 
> >> * nseries (n800, n810)
> >> 
> >>   Check whether order starts with 'n'.  Silently ignored otherwise.
> >> 
> >> * prep, g3beige, mac99
> >> 
> >>   Extract the first character the machine understands (subset of
> >>   'a'..'f').  Silently ignored otherwise.
> >> 
> >> * spapr
> >> 
> >>   Accept an arbitrary string (vl.c restricts it to contain only
> >>   'a'..'p', no duplicates).
> >> 
> >> * sun4[mdc]
> >> 
> >>   Use the first character.  Silently ignored otherwise.
> >> 
> >> Strip characters these machines ignore from their default boot order.
> >> 
> >> For all other machines, remove the unused default boot order
> >> alltogether.
> >> 
> >> Note that my rename of QEMUMachine member boot_order to
> >> default_boot_order and QEMUMachineInitArgs member boot_device to
> >> boot_order has a welcome side effect: it makes every use of boot
> >> orders visible in this patch, for easy review.
> >> 
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> [...]
> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> >> index 9327ac1..3700bd5 100644
> >> --- a/hw/i386/pc_piix.c
> >> +++ b/hw/i386/pc_piix.c
> >> @@ -216,7 +216,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
> >>          }
> >>      }
> >>  
> >> -    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
> >> +    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
> >>                   floppy, idebus[0], idebus[1], rtc_state);
> >>  
> >>      if (pci_enabled && usb_enabled(false)) {
> >> @@ -324,7 +324,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
> >>      .hot_add_cpu = pc_hot_add_cpu,
> >>      .max_cpus = 255,
> >>      .is_default = 1,
> >> -    DEFAULT_MACHINE_OPTIONS,
> >> +    .default_boot_order = "cad",
> >>  };
> >>  
> >>  static QEMUMachine pc_i440fx_machine_v1_5 = {
> >> @@ -337,7 +337,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
> >>          PC_COMPAT_1_5,
> >>          { /* end of list */ }
> >>      },
> >> -    DEFAULT_MACHINE_OPTIONS,
> >> +    .default_boot_order = "cad",
> >>  };
> >>  
> >>  static QEMUMachine pc_i440fx_machine_v1_4 = {
> >
> > So all PC machine types share this?
> 
> Correct, just like they share DEFAULT_MACHINE_OPTIONS before my patch.
> Which is defined as
> 
>     #define DEFAULT_MACHINE_OPTIONS \
>         .boot_order = "cad"
> 
> I.e. my patch merely peels off a layer of obfuscation :)

Using a macro in multiple places, instead of a hard-coded constant is
not obfuscation.

> > Can we set this in some common code, somehow?
> 
> We don't have an inheritance notion for machine types.
> 
> vl.c uses machine->boot_order before calling one of its methods, so
> monkey-patching .boot_order from a method won't do.  Besides, that cure
> looks much worse than the disease to me.
> 
> Can't think of anything else offhand.
> 
> [...]

Set this in pc_init_pci somehow?
Set DEFAULT_MACHINE_OPTIONS locally in this file?

-- 
MST

Re: [Qemu-devel] [PATCH v3 6/6] hw: Clean up bogus default boot order

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Wed, Aug 21, 2013 at 05:10:56PM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Mon, Jul 22, 2013 at 01:38:47PM +0200, Markus Armbruster wrote:
>> >> We set default boot order "cad" in every single machine definition
>> >> except "pseries" and "moxiesim", even though very few boards actually
>> >> care for boot order, and "cad" makes sense for even fewer.
>> >> 
>> >> Machines that care:
>> >> 
>> >> * pc and its variants
>> >> 
>> >>   Accept up to three letters 'a', 'b' (undocumented alias for 'a'),
>> >>   'c', 'd' and 'n'.  Reject all others (fatal with -boot).
>> >> 
>> >> * nseries (n800, n810)
>> >> 
>> >>   Check whether order starts with 'n'.  Silently ignored otherwise.
>> >> 
>> >> * prep, g3beige, mac99
>> >> 
>> >>   Extract the first character the machine understands (subset of
>> >>   'a'..'f').  Silently ignored otherwise.
>> >> 
>> >> * spapr
>> >> 
>> >>   Accept an arbitrary string (vl.c restricts it to contain only
>> >>   'a'..'p', no duplicates).
>> >> 
>> >> * sun4[mdc]
>> >> 
>> >>   Use the first character.  Silently ignored otherwise.
>> >> 
>> >> Strip characters these machines ignore from their default boot order.
>> >> 
>> >> For all other machines, remove the unused default boot order
>> >> alltogether.
>> >> 
>> >> Note that my rename of QEMUMachine member boot_order to
>> >> default_boot_order and QEMUMachineInitArgs member boot_device to
>> >> boot_order has a welcome side effect: it makes every use of boot
>> >> orders visible in this patch, for easy review.
>> >> 
>> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> >> ---
>> [...]
>> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> >> index 9327ac1..3700bd5 100644
>> >> --- a/hw/i386/pc_piix.c
>> >> +++ b/hw/i386/pc_piix.c
>> >> @@ -216,7 +216,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
>> >>          }
>> >>      }
>> >>  
>> >> -    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
>> >> +    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
>> >>                   floppy, idebus[0], idebus[1], rtc_state);
>> >>  
>> >>      if (pci_enabled && usb_enabled(false)) {
>> >> @@ -324,7 +324,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
>> >>      .hot_add_cpu = pc_hot_add_cpu,
>> >>      .max_cpus = 255,
>> >>      .is_default = 1,
>> >> -    DEFAULT_MACHINE_OPTIONS,
>> >> +    .default_boot_order = "cad",
>> >>  };
>> >>  
>> >>  static QEMUMachine pc_i440fx_machine_v1_5 = {
>> >> @@ -337,7 +337,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
>> >>          PC_COMPAT_1_5,
>> >>          { /* end of list */ }
>> >>      },
>> >> -    DEFAULT_MACHINE_OPTIONS,
>> >> +    .default_boot_order = "cad",
>> >>  };
>> >>  
>> >>  static QEMUMachine pc_i440fx_machine_v1_4 = {
>> >
>> > So all PC machine types share this?
>> 
>> Correct, just like they share DEFAULT_MACHINE_OPTIONS before my patch.
>> Which is defined as
>> 
>>     #define DEFAULT_MACHINE_OPTIONS \
>>         .boot_order = "cad"
>> 
>> I.e. my patch merely peels off a layer of obfuscation :)
>
> Using a macro in multiple places, instead of a hard-coded constant is
> not obfuscation.
>
>> > Can we set this in some common code, somehow?
>> 
>> We don't have an inheritance notion for machine types.
>> 
>> vl.c uses machine->boot_order before calling one of its methods, so
>> monkey-patching .boot_order from a method won't do.  Besides, that cure
>> looks much worse than the disease to me.
>> 
>> Can't think of anything else offhand.
>> 
>> [...]
>
> Set this in pc_init_pci somehow?

Too late, see "vl.c uses..." above.

> Set DEFAULT_MACHINE_OPTIONS locally in this file?

I can do #define CAD "cad" for you ;)

Seriously, I'd be okay with ".bootorder = DEFAULT_PC_BOOT_ORDER", with
#define DEFAULT_PC_BOOT_ORDER either locally, or in
include/hw/i386/pc.h.

Hiding the complete initialization in a macro would be bad style, in my
opinion.

Re: [Qemu-devel] [PATCH v3 6/6] hw: Clean up bogus default boot order

[Michael S. Tsirkin]

On Wed, Aug 21, 2013 at 06:01:45PM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Wed, Aug 21, 2013 at 05:10:56PM +0200, Markus Armbruster wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> 
> >> > On Mon, Jul 22, 2013 at 01:38:47PM +0200, Markus Armbruster wrote:
> >> >> We set default boot order "cad" in every single machine definition
> >> >> except "pseries" and "moxiesim", even though very few boards actually
> >> >> care for boot order, and "cad" makes sense for even fewer.
> >> >> 
> >> >> Machines that care:
> >> >> 
> >> >> * pc and its variants
> >> >> 
> >> >>   Accept up to three letters 'a', 'b' (undocumented alias for 'a'),
> >> >>   'c', 'd' and 'n'.  Reject all others (fatal with -boot).
> >> >> 
> >> >> * nseries (n800, n810)
> >> >> 
> >> >>   Check whether order starts with 'n'.  Silently ignored otherwise.
> >> >> 
> >> >> * prep, g3beige, mac99
> >> >> 
> >> >>   Extract the first character the machine understands (subset of
> >> >>   'a'..'f').  Silently ignored otherwise.
> >> >> 
> >> >> * spapr
> >> >> 
> >> >>   Accept an arbitrary string (vl.c restricts it to contain only
> >> >>   'a'..'p', no duplicates).
> >> >> 
> >> >> * sun4[mdc]
> >> >> 
> >> >>   Use the first character.  Silently ignored otherwise.
> >> >> 
> >> >> Strip characters these machines ignore from their default boot order.
> >> >> 
> >> >> For all other machines, remove the unused default boot order
> >> >> alltogether.
> >> >> 
> >> >> Note that my rename of QEMUMachine member boot_order to
> >> >> default_boot_order and QEMUMachineInitArgs member boot_device to
> >> >> boot_order has a welcome side effect: it makes every use of boot
> >> >> orders visible in this patch, for easy review.
> >> >> 
> >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> >> ---
> >> [...]
> >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> >> >> index 9327ac1..3700bd5 100644
> >> >> --- a/hw/i386/pc_piix.c
> >> >> +++ b/hw/i386/pc_piix.c
> >> >> @@ -216,7 +216,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
> >> >>          }
> >> >>      }
> >> >>  
> >> >> -    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
> >> >> +    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
> >> >>                   floppy, idebus[0], idebus[1], rtc_state);
> >> >>  
> >> >>      if (pci_enabled && usb_enabled(false)) {
> >> >> @@ -324,7 +324,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
> >> >>      .hot_add_cpu = pc_hot_add_cpu,
> >> >>      .max_cpus = 255,
> >> >>      .is_default = 1,
> >> >> -    DEFAULT_MACHINE_OPTIONS,
> >> >> +    .default_boot_order = "cad",
> >> >>  };
> >> >>  
> >> >>  static QEMUMachine pc_i440fx_machine_v1_5 = {
> >> >> @@ -337,7 +337,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
> >> >>          PC_COMPAT_1_5,
> >> >>          { /* end of list */ }
> >> >>      },
> >> >> -    DEFAULT_MACHINE_OPTIONS,
> >> >> +    .default_boot_order = "cad",
> >> >>  };
> >> >>  
> >> >>  static QEMUMachine pc_i440fx_machine_v1_4 = {
> >> >
> >> > So all PC machine types share this?
> >> 
> >> Correct, just like they share DEFAULT_MACHINE_OPTIONS before my patch.
> >> Which is defined as
> >> 
> >>     #define DEFAULT_MACHINE_OPTIONS \
> >>         .boot_order = "cad"
> >> 
> >> I.e. my patch merely peels off a layer of obfuscation :)
> >
> > Using a macro in multiple places, instead of a hard-coded constant is
> > not obfuscation.
> >
> >> > Can we set this in some common code, somehow?
> >> 
> >> We don't have an inheritance notion for machine types.
> >> 
> >> vl.c uses machine->boot_order before calling one of its methods, so
> >> monkey-patching .boot_order from a method won't do.  Besides, that cure
> >> looks much worse than the disease to me.
> >> 
> >> Can't think of anything else offhand.
> >> 
> >> [...]
> >
> > Set this in pc_init_pci somehow?
> 
> Too late, see "vl.c uses..." above.

Ah, missed it.
Can we fix that?

> > Set DEFAULT_MACHINE_OPTIONS locally in this file?
> 
> I can do #define CAD "cad" for you ;)
> 
> Seriously, I'd be okay with ".bootorder = DEFAULT_PC_BOOT_ORDER", with
> #define DEFAULT_PC_BOOT_ORDER either locally, or in
> include/hw/i386/pc.h.
> 
> Hiding the complete initialization in a macro would be bad style, in my
> opinion.

Re: [Qemu-devel] [PATCH v3 6/6] hw: Clean up bogus default boot order

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Wed, Aug 21, 2013 at 06:01:45PM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Wed, Aug 21, 2013 at 05:10:56PM +0200, Markus Armbruster wrote:
>> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> >> 
>> >> > On Mon, Jul 22, 2013 at 01:38:47PM +0200, Markus Armbruster wrote:
>> >> >> We set default boot order "cad" in every single machine definition
>> >> >> except "pseries" and "moxiesim", even though very few boards actually
>> >> >> care for boot order, and "cad" makes sense for even fewer.
>> >> >> 
>> >> >> Machines that care:
>> >> >> 
>> >> >> * pc and its variants
>> >> >> 
>> >> >>   Accept up to three letters 'a', 'b' (undocumented alias for 'a'),
>> >> >>   'c', 'd' and 'n'.  Reject all others (fatal with -boot).
>> >> >> 
>> >> >> * nseries (n800, n810)
>> >> >> 
>> >> >>   Check whether order starts with 'n'.  Silently ignored otherwise.
>> >> >> 
>> >> >> * prep, g3beige, mac99
>> >> >> 
>> >> >>   Extract the first character the machine understands (subset of
>> >> >>   'a'..'f').  Silently ignored otherwise.
>> >> >> 
>> >> >> * spapr
>> >> >> 
>> >> >>   Accept an arbitrary string (vl.c restricts it to contain only
>> >> >>   'a'..'p', no duplicates).
>> >> >> 
>> >> >> * sun4[mdc]
>> >> >> 
>> >> >>   Use the first character.  Silently ignored otherwise.
>> >> >> 
>> >> >> Strip characters these machines ignore from their default boot order.
>> >> >> 
>> >> >> For all other machines, remove the unused default boot order
>> >> >> alltogether.
>> >> >> 
>> >> >> Note that my rename of QEMUMachine member boot_order to
>> >> >> default_boot_order and QEMUMachineInitArgs member boot_device to
>> >> >> boot_order has a welcome side effect: it makes every use of boot
>> >> >> orders visible in this patch, for easy review.
>> >> >> 
>> >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> >> >> ---
>> >> [...]
>> >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> >> >> index 9327ac1..3700bd5 100644
>> >> >> --- a/hw/i386/pc_piix.c
>> >> >> +++ b/hw/i386/pc_piix.c
>> >> >> @@ -216,7 +216,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
>> >> >>          }
>> >> >>      }
>> >> >>  
>> >> >> -    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
>> >> >> +    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
>> >> >>                   floppy, idebus[0], idebus[1], rtc_state);
>> >> >>  
>> >> >>      if (pci_enabled && usb_enabled(false)) {
>> >> >> @@ -324,7 +324,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
>> >> >>      .hot_add_cpu = pc_hot_add_cpu,
>> >> >>      .max_cpus = 255,
>> >> >>      .is_default = 1,
>> >> >> -    DEFAULT_MACHINE_OPTIONS,
>> >> >> +    .default_boot_order = "cad",
>> >> >>  };
>> >> >>  
>> >> >>  static QEMUMachine pc_i440fx_machine_v1_5 = {
>> >> >> @@ -337,7 +337,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
>> >> >>          PC_COMPAT_1_5,
>> >> >>          { /* end of list */ }
>> >> >>      },
>> >> >> -    DEFAULT_MACHINE_OPTIONS,
>> >> >> +    .default_boot_order = "cad",
>> >> >>  };
>> >> >>  
>> >> >>  static QEMUMachine pc_i440fx_machine_v1_4 = {
>> >> >
>> >> > So all PC machine types share this?
>> >> 
>> >> Correct, just like they share DEFAULT_MACHINE_OPTIONS before my patch.
>> >> Which is defined as
>> >> 
>> >>     #define DEFAULT_MACHINE_OPTIONS \
>> >>         .boot_order = "cad"
>> >> 
>> >> I.e. my patch merely peels off a layer of obfuscation :)
>> >
>> > Using a macro in multiple places, instead of a hard-coded constant is
>> > not obfuscation.
>> >
>> >> > Can we set this in some common code, somehow?
>> >> 
>> >> We don't have an inheritance notion for machine types.
>> >> 
>> >> vl.c uses machine->boot_order before calling one of its methods, so
>> >> monkey-patching .boot_order from a method won't do.  Besides, that cure
>> >> looks much worse than the disease to me.
>> >> 
>> >> Can't think of anything else offhand.
>> >> 
>> >> [...]
>> >
>> > Set this in pc_init_pci somehow?
>> 
>> Too late, see "vl.c uses..." above.
>
> Ah, missed it.
> Can we fix that?

If I understand you correctly, you want to monkey-patch
machine->boot_order from machine->init().  Issues:

* machine->init() does not have access to machine.  Fixable.

* machine is read-only, except for a few places in vl.c (one is managing
  the list of registered machines, the other monkey-patches
  machine->max_cpus to one when it's zero).  I don't want *more*
  monkey-patching, I want *less*, so we can make machines const.  In
  fact, we can make current_machine const right away, and I think we
  should (patch coming).

* If machine->init() can change the default boot order, we get a data
  dependency cycle.  Current data dependency chain:

  0. Initialize QEMUMachine *machine to the default machine.

  1. Option parsing sets machine, and collects "boot-opts" options.

  2. Evaluation of "boot-opts": find normal boot order (from
     machine->boot_order and option parameter "boot") and one-time boot
     order (if option parameter "once" is given).

     Set boot_order to the initial boot order.

     Register a reset handler to revert the boot order from one-time to
     normal, if necessary.

  3. machine->init()

     Gets passed boot_order via QEMUMachineInitArgs.  Currently doesn't
     have access to machine.

  4. Set global variable current_machine = machine.

  Cycle: step 2 uses default boot order and defines boot order, step 3
  uses boot order and defines default boot order.

  I guess we could break this cycle by some sufficiently ingenious code
  shuffling.  But I'm pretty sure that would only complicate matters.
  Right now, boot order data flows down the program text, which makes it
  easy to understand.

>> > Set DEFAULT_MACHINE_OPTIONS locally in this file?
>> 
>> I can do #define CAD "cad" for you ;)
>> 
>> Seriously, I'd be okay with ".bootorder = DEFAULT_PC_BOOT_ORDER", with
>> #define DEFAULT_PC_BOOT_ORDER either locally, or in
>> include/hw/i386/pc.h.
>> 
>> Hiding the complete initialization in a macro would be bad style, in my
>> opinion.

Would you accept #define DEFAULT_PC_BOOT_ORDER in pc.h?

Re: [Qemu-devel] [PATCH v3 6/6] hw: Clean up bogus default boot order

[Michael S. Tsirkin]

On Thu, Aug 22, 2013 at 10:39:34AM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Wed, Aug 21, 2013 at 06:01:45PM +0200, Markus Armbruster wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> 
> >> > On Wed, Aug 21, 2013 at 05:10:56PM +0200, Markus Armbruster wrote:
> >> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> >> 
> >> >> > On Mon, Jul 22, 2013 at 01:38:47PM +0200, Markus Armbruster wrote:
> >> >> >> We set default boot order "cad" in every single machine definition
> >> >> >> except "pseries" and "moxiesim", even though very few boards actually
> >> >> >> care for boot order, and "cad" makes sense for even fewer.
> >> >> >> 
> >> >> >> Machines that care:
> >> >> >> 
> >> >> >> * pc and its variants
> >> >> >> 
> >> >> >>   Accept up to three letters 'a', 'b' (undocumented alias for 'a'),
> >> >> >>   'c', 'd' and 'n'.  Reject all others (fatal with -boot).
> >> >> >> 
> >> >> >> * nseries (n800, n810)
> >> >> >> 
> >> >> >>   Check whether order starts with 'n'.  Silently ignored otherwise.
> >> >> >> 
> >> >> >> * prep, g3beige, mac99
> >> >> >> 
> >> >> >>   Extract the first character the machine understands (subset of
> >> >> >>   'a'..'f').  Silently ignored otherwise.
> >> >> >> 
> >> >> >> * spapr
> >> >> >> 
> >> >> >>   Accept an arbitrary string (vl.c restricts it to contain only
> >> >> >>   'a'..'p', no duplicates).
> >> >> >> 
> >> >> >> * sun4[mdc]
> >> >> >> 
> >> >> >>   Use the first character.  Silently ignored otherwise.
> >> >> >> 
> >> >> >> Strip characters these machines ignore from their default boot order.
> >> >> >> 
> >> >> >> For all other machines, remove the unused default boot order
> >> >> >> alltogether.
> >> >> >> 
> >> >> >> Note that my rename of QEMUMachine member boot_order to
> >> >> >> default_boot_order and QEMUMachineInitArgs member boot_device to
> >> >> >> boot_order has a welcome side effect: it makes every use of boot
> >> >> >> orders visible in this patch, for easy review.
> >> >> >> 
> >> >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> >> >> ---
> >> >> [...]
> >> >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> >> >> >> index 9327ac1..3700bd5 100644
> >> >> >> --- a/hw/i386/pc_piix.c
> >> >> >> +++ b/hw/i386/pc_piix.c
> >> >> >> @@ -216,7 +216,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
> >> >> >>          }
> >> >> >>      }
> >> >> >>  
> >> >> >> -    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
> >> >> >> +    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
> >> >> >>                   floppy, idebus[0], idebus[1], rtc_state);
> >> >> >>  
> >> >> >>      if (pci_enabled && usb_enabled(false)) {
> >> >> >> @@ -324,7 +324,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
> >> >> >>      .hot_add_cpu = pc_hot_add_cpu,
> >> >> >>      .max_cpus = 255,
> >> >> >>      .is_default = 1,
> >> >> >> -    DEFAULT_MACHINE_OPTIONS,
> >> >> >> +    .default_boot_order = "cad",
> >> >> >>  };
> >> >> >>  
> >> >> >>  static QEMUMachine pc_i440fx_machine_v1_5 = {
> >> >> >> @@ -337,7 +337,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
> >> >> >>          PC_COMPAT_1_5,
> >> >> >>          { /* end of list */ }
> >> >> >>      },
> >> >> >> -    DEFAULT_MACHINE_OPTIONS,
> >> >> >> +    .default_boot_order = "cad",
> >> >> >>  };
> >> >> >>  
> >> >> >>  static QEMUMachine pc_i440fx_machine_v1_4 = {
> >> >> >
> >> >> > So all PC machine types share this?
> >> >> 
> >> >> Correct, just like they share DEFAULT_MACHINE_OPTIONS before my patch.
> >> >> Which is defined as
> >> >> 
> >> >>     #define DEFAULT_MACHINE_OPTIONS \
> >> >>         .boot_order = "cad"
> >> >> 
> >> >> I.e. my patch merely peels off a layer of obfuscation :)
> >> >
> >> > Using a macro in multiple places, instead of a hard-coded constant is
> >> > not obfuscation.
> >> >
> >> >> > Can we set this in some common code, somehow?
> >> >> 
> >> >> We don't have an inheritance notion for machine types.
> >> >> 
> >> >> vl.c uses machine->boot_order before calling one of its methods, so
> >> >> monkey-patching .boot_order from a method won't do.  Besides, that cure
> >> >> looks much worse than the disease to me.
> >> >> 
> >> >> Can't think of anything else offhand.
> >> >> 
> >> >> [...]
> >> >
> >> > Set this in pc_init_pci somehow?
> >> 
> >> Too late, see "vl.c uses..." above.
> >
> > Ah, missed it.
> > Can we fix that?
> 
> If I understand you correctly, you want to monkey-patch
> machine->boot_order from machine->init().  Issues:
> 
> * machine->init() does not have access to machine.  Fixable.
> 
> * machine is read-only, except for a few places in vl.c (one is managing
>   the list of registered machines, the other monkey-patches
>   machine->max_cpus to one when it's zero).  I don't want *more*
>   monkey-patching, I want *less*, so we can make machines const.  In
>   fact, we can make current_machine const right away, and I think we
>   should (patch coming).
> 
> * If machine->init() can change the default boot order, we get a data
>   dependency cycle.  Current data dependency chain:
> 
>   0. Initialize QEMUMachine *machine to the default machine.
> 
>   1. Option parsing sets machine, and collects "boot-opts" options.
> 
>   2. Evaluation of "boot-opts": find normal boot order (from
>      machine->boot_order and option parameter "boot") and one-time boot
>      order (if option parameter "once" is given).
> 
>      Set boot_order to the initial boot order.
> 
>      Register a reset handler to revert the boot order from one-time to
>      normal, if necessary.
> 
>   3. machine->init()
> 
>      Gets passed boot_order via QEMUMachineInitArgs.  Currently doesn't
>      have access to machine.
> 
>   4. Set global variable current_machine = machine.
> 
>   Cycle: step 2 uses default boot order and defines boot order, step 3
>   uses boot order and defines default boot order.
> 
>   I guess we could break this cycle by some sufficiently ingenious code
>   shuffling.  But I'm pretty sure that would only complicate matters.
>   Right now, boot order data flows down the program text, which makes it
>   easy to understand.

I agree, it's a concern.

> >> > Set DEFAULT_MACHINE_OPTIONS locally in this file?
> >> 
> >> I can do #define CAD "cad" for you ;)
> >> 
> >> Seriously, I'd be okay with ".bootorder = DEFAULT_PC_BOOT_ORDER", with
> >> #define DEFAULT_PC_BOOT_ORDER either locally, or in
> >> include/hw/i386/pc.h.
> >> 
> >> Hiding the complete initialization in a macro would be bad style, in my
> >> opinion.
> 
> Would you accept #define DEFAULT_PC_BOOT_ORDER in pc.h?


Here's what bothers me.
static QEMUMachine pc_i440fx_machine_v1_6 = {
    .name = "pc-i440fx-1.6",
    .alias = "pc", 
    .desc = "Standard PC (i440FX + PIIX, 1996)", <-- mostly copied over,
				but different for 1.3 - this is likely a bug
    .init = pc_init_pci_1_6,
    .hot_add_cpu = pc_hot_add_cpu, <- has to be copied over at least for
					newer PCs. Not sure about older ones -
					could be a bug or intentional
    .max_cpus = 255,		<- always 255 except isapc
    DEFAULT_MACHINE_OPTIONS, <- always copied
};


So there's a lot of boiler plate eahc time we add
a machine type.

DEFAULT_MACHINE_OPTIONS kind of offered a way to address
that, maybe with per-version inheritance like we do
for compat properties.

Now you want to remove that for style reasons, so we'll
have to stay with duplicated code :(

I'm not nacking this, but I think you'll agree it's not
a clear-cut improvement - if we are cleaning this up
it would be better to do something that fixes the
code duplication.

-- 
MST

Re: [Qemu-devel] [PATCH v3 6/6] hw: Clean up bogus default boot order

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Aug 22, 2013 at 10:39:34AM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Wed, Aug 21, 2013 at 06:01:45PM +0200, Markus Armbruster wrote:
>> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> >> 
>> >> > On Wed, Aug 21, 2013 at 05:10:56PM +0200, Markus Armbruster wrote:
>> >> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> >> >> 
>> >> >> > On Mon, Jul 22, 2013 at 01:38:47PM +0200, Markus Armbruster wrote:
>> >> >> >> We set default boot order "cad" in every single machine definition
>> >> >> >> except "pseries" and "moxiesim", even though very few boards actually
>> >> >> >> care for boot order, and "cad" makes sense for even fewer.
>> >> >> >> 
>> >> >> >> Machines that care:
>> >> >> >> 
>> >> >> >> * pc and its variants
>> >> >> >> 
>> >> >> >>   Accept up to three letters 'a', 'b' (undocumented alias for 'a'),
>> >> >> >>   'c', 'd' and 'n'.  Reject all others (fatal with -boot).
>> >> >> >> 
>> >> >> >> * nseries (n800, n810)
>> >> >> >> 
>> >> >> >>   Check whether order starts with 'n'.  Silently ignored otherwise.
>> >> >> >> 
>> >> >> >> * prep, g3beige, mac99
>> >> >> >> 
>> >> >> >>   Extract the first character the machine understands (subset of
>> >> >> >>   'a'..'f').  Silently ignored otherwise.
>> >> >> >> 
>> >> >> >> * spapr
>> >> >> >> 
>> >> >> >>   Accept an arbitrary string (vl.c restricts it to contain only
>> >> >> >>   'a'..'p', no duplicates).
>> >> >> >> 
>> >> >> >> * sun4[mdc]
>> >> >> >> 
>> >> >> >>   Use the first character.  Silently ignored otherwise.
>> >> >> >> 
>> >> >> >> Strip characters these machines ignore from their default boot order.
>> >> >> >> 
>> >> >> >> For all other machines, remove the unused default boot order
>> >> >> >> alltogether.
>> >> >> >> 
>> >> >> >> Note that my rename of QEMUMachine member boot_order to
>> >> >> >> default_boot_order and QEMUMachineInitArgs member boot_device to
>> >> >> >> boot_order has a welcome side effect: it makes every use of boot
>> >> >> >> orders visible in this patch, for easy review.
>> >> >> >> 
>> >> >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> >> >> >> ---
>> >> >> [...]
>> >> >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> >> >> >> index 9327ac1..3700bd5 100644
>> >> >> >> --- a/hw/i386/pc_piix.c
>> >> >> >> +++ b/hw/i386/pc_piix.c
>> >> >> >> @@ -216,7 +216,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
>> >> >> >>          }
>> >> >> >>      }
>> >> >> >>  
>> >> >> >> -    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
>> >> >> >> +    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
>> >> >> >>                   floppy, idebus[0], idebus[1], rtc_state);
>> >> >> >>  
>> >> >> >>      if (pci_enabled && usb_enabled(false)) {
>> >> >> >> @@ -324,7 +324,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
>> >> >> >>      .hot_add_cpu = pc_hot_add_cpu,
>> >> >> >>      .max_cpus = 255,
>> >> >> >>      .is_default = 1,
>> >> >> >> -    DEFAULT_MACHINE_OPTIONS,
>> >> >> >> +    .default_boot_order = "cad",
>> >> >> >>  };
>> >> >> >>  
>> >> >> >>  static QEMUMachine pc_i440fx_machine_v1_5 = {
>> >> >> >> @@ -337,7 +337,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
>> >> >> >>          PC_COMPAT_1_5,
>> >> >> >>          { /* end of list */ }
>> >> >> >>      },
>> >> >> >> -    DEFAULT_MACHINE_OPTIONS,
>> >> >> >> +    .default_boot_order = "cad",
>> >> >> >>  };
>> >> >> >>  
>> >> >> >>  static QEMUMachine pc_i440fx_machine_v1_4 = {
>> >> >> >
>> >> >> > So all PC machine types share this?
>> >> >> 
>> >> >> Correct, just like they share DEFAULT_MACHINE_OPTIONS before my patch.
>> >> >> Which is defined as
>> >> >> 
>> >> >>     #define DEFAULT_MACHINE_OPTIONS \
>> >> >>         .boot_order = "cad"
>> >> >> 
>> >> >> I.e. my patch merely peels off a layer of obfuscation :)
>> >> >
>> >> > Using a macro in multiple places, instead of a hard-coded constant is
>> >> > not obfuscation.
>> >> >
>> >> >> > Can we set this in some common code, somehow?
>> >> >> 
>> >> >> We don't have an inheritance notion for machine types.
>> >> >> 
>> >> >> vl.c uses machine->boot_order before calling one of its methods, so
>> >> >> monkey-patching .boot_order from a method won't do.  Besides, that cure
>> >> >> looks much worse than the disease to me.
>> >> >> 
>> >> >> Can't think of anything else offhand.
>> >> >> 
>> >> >> [...]
>> >> >
>> >> > Set this in pc_init_pci somehow?
>> >> 
>> >> Too late, see "vl.c uses..." above.
>> >
>> > Ah, missed it.
>> > Can we fix that?
>> 
>> If I understand you correctly, you want to monkey-patch
>> machine->boot_order from machine->init().  Issues:
>> 
>> * machine->init() does not have access to machine.  Fixable.
>> 
>> * machine is read-only, except for a few places in vl.c (one is managing
>>   the list of registered machines, the other monkey-patches
>>   machine->max_cpus to one when it's zero).  I don't want *more*
>>   monkey-patching, I want *less*, so we can make machines const.  In
>>   fact, we can make current_machine const right away, and I think we
>>   should (patch coming).
>> 
>> * If machine->init() can change the default boot order, we get a data
>>   dependency cycle.  Current data dependency chain:
>> 
>>   0. Initialize QEMUMachine *machine to the default machine.
>> 
>>   1. Option parsing sets machine, and collects "boot-opts" options.
>> 
>>   2. Evaluation of "boot-opts": find normal boot order (from
>>      machine->boot_order and option parameter "boot") and one-time boot
>>      order (if option parameter "once" is given).
>> 
>>      Set boot_order to the initial boot order.
>> 
>>      Register a reset handler to revert the boot order from one-time to
>>      normal, if necessary.
>> 
>>   3. machine->init()
>> 
>>      Gets passed boot_order via QEMUMachineInitArgs.  Currently doesn't
>>      have access to machine.
>> 
>>   4. Set global variable current_machine = machine.
>> 
>>   Cycle: step 2 uses default boot order and defines boot order, step 3
>>   uses boot order and defines default boot order.
>> 
>>   I guess we could break this cycle by some sufficiently ingenious code
>>   shuffling.  But I'm pretty sure that would only complicate matters.
>>   Right now, boot order data flows down the program text, which makes it
>>   easy to understand.
>
> I agree, it's a concern.
>
>> >> > Set DEFAULT_MACHINE_OPTIONS locally in this file?
>> >> 
>> >> I can do #define CAD "cad" for you ;)
>> >> 
>> >> Seriously, I'd be okay with ".bootorder = DEFAULT_PC_BOOT_ORDER", with
>> >> #define DEFAULT_PC_BOOT_ORDER either locally, or in
>> >> include/hw/i386/pc.h.
>> >> 
>> >> Hiding the complete initialization in a macro would be bad style, in my
>> >> opinion.
>> 
>> Would you accept #define DEFAULT_PC_BOOT_ORDER in pc.h?
>
>
> Here's what bothers me.
> static QEMUMachine pc_i440fx_machine_v1_6 = {
>     .name = "pc-i440fx-1.6",
>     .alias = "pc", 
>     .desc = "Standard PC (i440FX + PIIX, 1996)", <-- mostly copied over,
> 				but different for 1.3 - this is likely a bug
>     .init = pc_init_pci_1_6,
>     .hot_add_cpu = pc_hot_add_cpu, <- has to be copied over at least for
> 					newer PCs. Not sure about older ones -
> 					could be a bug or intentional
>     .max_cpus = 255,		<- always 255 except isapc
>     DEFAULT_MACHINE_OPTIONS, <- always copied
> };
>
>
> So there's a lot of boiler plate eahc time we add
> a machine type.
>
> DEFAULT_MACHINE_OPTIONS kind of offered a way to address
> that, maybe with per-version inheritance like we do
> for compat properties.
>
> Now you want to remove that for style reasons, so we'll
> have to stay with duplicated code :(

DEFAULT_MACHINE_OPTIONS are copied to *every* machine.  I can't see why
anything that's common to every machine should be duplicated in every
machine type definition.

Turns out the stuff DEFAULT_MACHINE_OPTIONS copies shouldn't be copied:
it's bogus for most machines.  My patch cleans that up, no more, no
less.

There are groups of related machines, such as the PC machines, which
have stuff in common legitimately.  Avoiding duplicating this stuff in
their machine initializers may be worthwhile.  However, that's not my
patch's aim.  Nothing in my patch precludes future de-duplication.

> I'm not nacking this, but I think you'll agree it's not
> a clear-cut improvement

I agree de-duplication may be worthwhile.  I further agree my patch
makes the existing duplication of one initializer (default boot order) a
bit more visible than it was before (in addition to removing its
existing duplication from lots of places where it makes no sense).  It
doesn't affect the existing duplication of all the other initializers.

>                         - if we are cleaning this up
> it would be better to do something that fixes the
> code duplication.

The patch is not about cleaning up code duplication in related machine
initializers.  It's about cleaning up bogus default boot orders.

I'm wary of patch series mission creep :)

Re: [Qemu-devel] [PATCH v3 6/6] hw: Clean up bogus default boot order

[Michael S. Tsirkin]

On Thu, Aug 22, 2013 at 01:24:50PM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Thu, Aug 22, 2013 at 10:39:34AM +0200, Markus Armbruster wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> 
> >> > On Wed, Aug 21, 2013 at 06:01:45PM +0200, Markus Armbruster wrote:
> >> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> >> 
> >> >> > On Wed, Aug 21, 2013 at 05:10:56PM +0200, Markus Armbruster wrote:
> >> >> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> >> >> 
> >> >> >> > On Mon, Jul 22, 2013 at 01:38:47PM +0200, Markus Armbruster wrote:
> >> >> >> >> We set default boot order "cad" in every single machine definition
> >> >> >> >> except "pseries" and "moxiesim", even though very few boards actually
> >> >> >> >> care for boot order, and "cad" makes sense for even fewer.
> >> >> >> >> 
> >> >> >> >> Machines that care:
> >> >> >> >> 
> >> >> >> >> * pc and its variants
> >> >> >> >> 
> >> >> >> >>   Accept up to three letters 'a', 'b' (undocumented alias for 'a'),
> >> >> >> >>   'c', 'd' and 'n'.  Reject all others (fatal with -boot).
> >> >> >> >> 
> >> >> >> >> * nseries (n800, n810)
> >> >> >> >> 
> >> >> >> >>   Check whether order starts with 'n'.  Silently ignored otherwise.
> >> >> >> >> 
> >> >> >> >> * prep, g3beige, mac99
> >> >> >> >> 
> >> >> >> >>   Extract the first character the machine understands (subset of
> >> >> >> >>   'a'..'f').  Silently ignored otherwise.
> >> >> >> >> 
> >> >> >> >> * spapr
> >> >> >> >> 
> >> >> >> >>   Accept an arbitrary string (vl.c restricts it to contain only
> >> >> >> >>   'a'..'p', no duplicates).
> >> >> >> >> 
> >> >> >> >> * sun4[mdc]
> >> >> >> >> 
> >> >> >> >>   Use the first character.  Silently ignored otherwise.
> >> >> >> >> 
> >> >> >> >> Strip characters these machines ignore from their default boot order.
> >> >> >> >> 
> >> >> >> >> For all other machines, remove the unused default boot order
> >> >> >> >> alltogether.
> >> >> >> >> 
> >> >> >> >> Note that my rename of QEMUMachine member boot_order to
> >> >> >> >> default_boot_order and QEMUMachineInitArgs member boot_device to
> >> >> >> >> boot_order has a welcome side effect: it makes every use of boot
> >> >> >> >> orders visible in this patch, for easy review.
> >> >> >> >> 
> >> >> >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> >> >> >> ---
> >> >> >> [...]
> >> >> >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> >> >> >> >> index 9327ac1..3700bd5 100644
> >> >> >> >> --- a/hw/i386/pc_piix.c
> >> >> >> >> +++ b/hw/i386/pc_piix.c
> >> >> >> >> @@ -216,7 +216,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
> >> >> >> >>          }
> >> >> >> >>      }
> >> >> >> >>  
> >> >> >> >> -    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
> >> >> >> >> +    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
> >> >> >> >>                   floppy, idebus[0], idebus[1], rtc_state);
> >> >> >> >>  
> >> >> >> >>      if (pci_enabled && usb_enabled(false)) {
> >> >> >> >> @@ -324,7 +324,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
> >> >> >> >>      .hot_add_cpu = pc_hot_add_cpu,
> >> >> >> >>      .max_cpus = 255,
> >> >> >> >>      .is_default = 1,
> >> >> >> >> -    DEFAULT_MACHINE_OPTIONS,
> >> >> >> >> +    .default_boot_order = "cad",
> >> >> >> >>  };
> >> >> >> >>  
> >> >> >> >>  static QEMUMachine pc_i440fx_machine_v1_5 = {
> >> >> >> >> @@ -337,7 +337,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
> >> >> >> >>          PC_COMPAT_1_5,
> >> >> >> >>          { /* end of list */ }
> >> >> >> >>      },
> >> >> >> >> -    DEFAULT_MACHINE_OPTIONS,
> >> >> >> >> +    .default_boot_order = "cad",
> >> >> >> >>  };
> >> >> >> >>  
> >> >> >> >>  static QEMUMachine pc_i440fx_machine_v1_4 = {
> >> >> >> >
> >> >> >> > So all PC machine types share this?
> >> >> >> 
> >> >> >> Correct, just like they share DEFAULT_MACHINE_OPTIONS before my patch.
> >> >> >> Which is defined as
> >> >> >> 
> >> >> >>     #define DEFAULT_MACHINE_OPTIONS \
> >> >> >>         .boot_order = "cad"
> >> >> >> 
> >> >> >> I.e. my patch merely peels off a layer of obfuscation :)
> >> >> >
> >> >> > Using a macro in multiple places, instead of a hard-coded constant is
> >> >> > not obfuscation.
> >> >> >
> >> >> >> > Can we set this in some common code, somehow?
> >> >> >> 
> >> >> >> We don't have an inheritance notion for machine types.
> >> >> >> 
> >> >> >> vl.c uses machine->boot_order before calling one of its methods, so
> >> >> >> monkey-patching .boot_order from a method won't do.  Besides, that cure
> >> >> >> looks much worse than the disease to me.
> >> >> >> 
> >> >> >> Can't think of anything else offhand.
> >> >> >> 
> >> >> >> [...]
> >> >> >
> >> >> > Set this in pc_init_pci somehow?
> >> >> 
> >> >> Too late, see "vl.c uses..." above.
> >> >
> >> > Ah, missed it.
> >> > Can we fix that?
> >> 
> >> If I understand you correctly, you want to monkey-patch
> >> machine->boot_order from machine->init().  Issues:
> >> 
> >> * machine->init() does not have access to machine.  Fixable.
> >> 
> >> * machine is read-only, except for a few places in vl.c (one is managing
> >>   the list of registered machines, the other monkey-patches
> >>   machine->max_cpus to one when it's zero).  I don't want *more*
> >>   monkey-patching, I want *less*, so we can make machines const.  In
> >>   fact, we can make current_machine const right away, and I think we
> >>   should (patch coming).
> >> 
> >> * If machine->init() can change the default boot order, we get a data
> >>   dependency cycle.  Current data dependency chain:
> >> 
> >>   0. Initialize QEMUMachine *machine to the default machine.
> >> 
> >>   1. Option parsing sets machine, and collects "boot-opts" options.
> >> 
> >>   2. Evaluation of "boot-opts": find normal boot order (from
> >>      machine->boot_order and option parameter "boot") and one-time boot
> >>      order (if option parameter "once" is given).
> >> 
> >>      Set boot_order to the initial boot order.
> >> 
> >>      Register a reset handler to revert the boot order from one-time to
> >>      normal, if necessary.
> >> 
> >>   3. machine->init()
> >> 
> >>      Gets passed boot_order via QEMUMachineInitArgs.  Currently doesn't
> >>      have access to machine.
> >> 
> >>   4. Set global variable current_machine = machine.
> >> 
> >>   Cycle: step 2 uses default boot order and defines boot order, step 3
> >>   uses boot order and defines default boot order.
> >> 
> >>   I guess we could break this cycle by some sufficiently ingenious code
> >>   shuffling.  But I'm pretty sure that would only complicate matters.
> >>   Right now, boot order data flows down the program text, which makes it
> >>   easy to understand.
> >
> > I agree, it's a concern.
> >
> >> >> > Set DEFAULT_MACHINE_OPTIONS locally in this file?
> >> >> 
> >> >> I can do #define CAD "cad" for you ;)
> >> >> 
> >> >> Seriously, I'd be okay with ".bootorder = DEFAULT_PC_BOOT_ORDER", with
> >> >> #define DEFAULT_PC_BOOT_ORDER either locally, or in
> >> >> include/hw/i386/pc.h.
> >> >> 
> >> >> Hiding the complete initialization in a macro would be bad style, in my
> >> >> opinion.
> >> 
> >> Would you accept #define DEFAULT_PC_BOOT_ORDER in pc.h?
> >
> >
> > Here's what bothers me.
> > static QEMUMachine pc_i440fx_machine_v1_6 = {
> >     .name = "pc-i440fx-1.6",
> >     .alias = "pc", 
> >     .desc = "Standard PC (i440FX + PIIX, 1996)", <-- mostly copied over,
> > 				but different for 1.3 - this is likely a bug
> >     .init = pc_init_pci_1_6,
> >     .hot_add_cpu = pc_hot_add_cpu, <- has to be copied over at least for
> > 					newer PCs. Not sure about older ones -
> > 					could be a bug or intentional
> >     .max_cpus = 255,		<- always 255 except isapc
> >     DEFAULT_MACHINE_OPTIONS, <- always copied
> > };
> >
> >
> > So there's a lot of boiler plate eahc time we add
> > a machine type.
> >
> > DEFAULT_MACHINE_OPTIONS kind of offered a way to address
> > that, maybe with per-version inheritance like we do
> > for compat properties.
> >
> > Now you want to remove that for style reasons, so we'll
> > have to stay with duplicated code :(
> 
> DEFAULT_MACHINE_OPTIONS are copied to *every* machine.  I can't see why
> anything that's common to every machine should be duplicated in every
> machine type definition.
> 
> Turns out the stuff DEFAULT_MACHINE_OPTIONS copies shouldn't be copied:
> it's bogus for most machines.  My patch cleans that up, no more, no
> less.
> 
> There are groups of related machines, such as the PC machines, which
> have stuff in common legitimately.  Avoiding duplicating this stuff in
> their machine initializers may be worthwhile.  However, that's not my
> patch's aim.  Nothing in my patch precludes future de-duplication.
> 
> > I'm not nacking this, but I think you'll agree it's not
> > a clear-cut improvement
> 
> I agree de-duplication may be worthwhile.  I further agree my patch
> makes the existing duplication of one initializer (default boot order) a
> bit more visible than it was before (in addition to removing its
> existing duplication from lots of places where it makes no sense).  It
> doesn't affect the existing duplication of all the other initializers.

Now that it's visible, maybe you can be persuaded to fix it?
If it were not for code duplication, your patch would have
been much smaller, right?

> >                         - if we are cleaning this up
> > it would be better to do something that fixes the
> > code duplication.
> 
> The patch is not about cleaning up code duplication in related machine
> initializers.  It's about cleaning up bogus default boot orders.
> 
> I'm wary of patch series mission creep :)

My point is that once we have that cleanup, it's possible
that you will want to tweak 6/6.

-- 
MST

Re: [Qemu-devel] [PATCH v3 6/6] hw: Clean up bogus default boot order

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Aug 22, 2013 at 01:24:50PM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Thu, Aug 22, 2013 at 10:39:34AM +0200, Markus Armbruster wrote:
>> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> >> 
>> >> > On Wed, Aug 21, 2013 at 06:01:45PM +0200, Markus Armbruster wrote:
>> >> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> >> >> 
>> >> >> > On Wed, Aug 21, 2013 at 05:10:56PM +0200, Markus Armbruster wrote:
>> >> >> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> >> >> >> 
>> >> >> >> > On Mon, Jul 22, 2013 at 01:38:47PM +0200, Markus Armbruster wrote:
>> >> >> >> >> We set default boot order "cad" in every single machine definition
>> >> >> >> >> except "pseries" and "moxiesim", even though very few
>> >> >> >> >> boards actually
>> >> >> >> >> care for boot order, and "cad" makes sense for even fewer.
>> >> >> >> >> 
>> >> >> >> >> Machines that care:
>> >> >> >> >> 
>> >> >> >> >> * pc and its variants
>> >> >> >> >> 
>> >> >> >> >>   Accept up to three letters 'a', 'b' (undocumented
>> >> >> >> >> alias for 'a'),
>> >> >> >> >>   'c', 'd' and 'n'.  Reject all others (fatal with -boot).
>> >> >> >> >> 
>> >> >> >> >> * nseries (n800, n810)
>> >> >> >> >> 
>> >> >> >> >>   Check whether order starts with 'n'.  Silently ignored
>> >> >> >> >> otherwise.
>> >> >> >> >> 
>> >> >> >> >> * prep, g3beige, mac99
>> >> >> >> >> 
>> >> >> >> >>   Extract the first character the machine understands (subset of
>> >> >> >> >>   'a'..'f').  Silently ignored otherwise.
>> >> >> >> >> 
>> >> >> >> >> * spapr
>> >> >> >> >> 
>> >> >> >> >>   Accept an arbitrary string (vl.c restricts it to contain only
>> >> >> >> >>   'a'..'p', no duplicates).
>> >> >> >> >> 
>> >> >> >> >> * sun4[mdc]
>> >> >> >> >> 
>> >> >> >> >>   Use the first character.  Silently ignored otherwise.
>> >> >> >> >> 
>> >> >> >> >> Strip characters these machines ignore from their
>> >> >> >> >> default boot order.
>> >> >> >> >> 
>> >> >> >> >> For all other machines, remove the unused default boot order
>> >> >> >> >> alltogether.
>> >> >> >> >> 
>> >> >> >> >> Note that my rename of QEMUMachine member boot_order to
>> >> >> >> >> default_boot_order and QEMUMachineInitArgs member boot_device to
>> >> >> >> >> boot_order has a welcome side effect: it makes every use of boot
>> >> >> >> >> orders visible in this patch, for easy review.
>> >> >> >> >> 
>> >> >> >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> >> >> >> >> ---
>> >> >> >> [...]
>> >> >> >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> >> >> >> >> index 9327ac1..3700bd5 100644
>> >> >> >> >> --- a/hw/i386/pc_piix.c
>> >> >> >> >> +++ b/hw/i386/pc_piix.c
>> >> >> >> >> @@ -216,7 +216,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
>> >> >> >> >>          }
>> >> >> >> >>      }
>> >> >> >> >>  
>> >> >> >> >> -    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
>> >> >> >> >> +    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
>> >> >> >> >>                   floppy, idebus[0], idebus[1], rtc_state);
>> >> >> >> >>  
>> >> >> >> >>      if (pci_enabled && usb_enabled(false)) {
>> >> >> >> >> @@ -324,7 +324,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
>> >> >> >> >>      .hot_add_cpu = pc_hot_add_cpu,
>> >> >> >> >>      .max_cpus = 255,
>> >> >> >> >>      .is_default = 1,
>> >> >> >> >> -    DEFAULT_MACHINE_OPTIONS,
>> >> >> >> >> +    .default_boot_order = "cad",
>> >> >> >> >>  };
>> >> >> >> >>  
>> >> >> >> >>  static QEMUMachine pc_i440fx_machine_v1_5 = {
>> >> >> >> >> @@ -337,7 +337,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
>> >> >> >> >>          PC_COMPAT_1_5,
>> >> >> >> >>          { /* end of list */ }
>> >> >> >> >>      },
>> >> >> >> >> -    DEFAULT_MACHINE_OPTIONS,
>> >> >> >> >> +    .default_boot_order = "cad",
>> >> >> >> >>  };
>> >> >> >> >>  
>> >> >> >> >>  static QEMUMachine pc_i440fx_machine_v1_4 = {
>> >> >> >> >
>> >> >> >> > So all PC machine types share this?
>> >> >> >> 
>> >> >> >> Correct, just like they share DEFAULT_MACHINE_OPTIONS before my patch.
>> >> >> >> Which is defined as
>> >> >> >> 
>> >> >> >>     #define DEFAULT_MACHINE_OPTIONS \
>> >> >> >>         .boot_order = "cad"
>> >> >> >> 
>> >> >> >> I.e. my patch merely peels off a layer of obfuscation :)
>> >> >> >
>> >> >> > Using a macro in multiple places, instead of a hard-coded constant is
>> >> >> > not obfuscation.
>> >> >> >
>> >> >> >> > Can we set this in some common code, somehow?
>> >> >> >> 
>> >> >> >> We don't have an inheritance notion for machine types.
>> >> >> >> 
>> >> >> >> vl.c uses machine->boot_order before calling one of its methods, so
>> >> >> >> monkey-patching .boot_order from a method won't do.
>> >> >> >> Besides, that cure
>> >> >> >> looks much worse than the disease to me.
>> >> >> >> 
>> >> >> >> Can't think of anything else offhand.
>> >> >> >> 
>> >> >> >> [...]
>> >> >> >
>> >> >> > Set this in pc_init_pci somehow?
>> >> >> 
>> >> >> Too late, see "vl.c uses..." above.
>> >> >
>> >> > Ah, missed it.
>> >> > Can we fix that?
>> >> 
>> >> If I understand you correctly, you want to monkey-patch
>> >> machine->boot_order from machine->init().  Issues:
>> >> 
>> >> * machine->init() does not have access to machine.  Fixable.
>> >> 
>> >> * machine is read-only, except for a few places in vl.c (one is managing
>> >>   the list of registered machines, the other monkey-patches
>> >>   machine->max_cpus to one when it's zero).  I don't want *more*
>> >>   monkey-patching, I want *less*, so we can make machines const.  In
>> >>   fact, we can make current_machine const right away, and I think we
>> >>   should (patch coming).
>> >> 
>> >> * If machine->init() can change the default boot order, we get a data
>> >>   dependency cycle.  Current data dependency chain:
>> >> 
>> >>   0. Initialize QEMUMachine *machine to the default machine.
>> >> 
>> >>   1. Option parsing sets machine, and collects "boot-opts" options.
>> >> 
>> >>   2. Evaluation of "boot-opts": find normal boot order (from
>> >>      machine->boot_order and option parameter "boot") and one-time boot
>> >>      order (if option parameter "once" is given).
>> >> 
>> >>      Set boot_order to the initial boot order.
>> >> 
>> >>      Register a reset handler to revert the boot order from one-time to
>> >>      normal, if necessary.
>> >> 
>> >>   3. machine->init()
>> >> 
>> >>      Gets passed boot_order via QEMUMachineInitArgs.  Currently doesn't
>> >>      have access to machine.
>> >> 
>> >>   4. Set global variable current_machine = machine.
>> >> 
>> >>   Cycle: step 2 uses default boot order and defines boot order, step 3
>> >>   uses boot order and defines default boot order.
>> >> 
>> >>   I guess we could break this cycle by some sufficiently ingenious code
>> >>   shuffling.  But I'm pretty sure that would only complicate matters.
>> >>   Right now, boot order data flows down the program text, which makes it
>> >>   easy to understand.
>> >
>> > I agree, it's a concern.
>> >
>> >> >> > Set DEFAULT_MACHINE_OPTIONS locally in this file?
>> >> >> 
>> >> >> I can do #define CAD "cad" for you ;)
>> >> >> 
>> >> >> Seriously, I'd be okay with ".bootorder = DEFAULT_PC_BOOT_ORDER", with
>> >> >> #define DEFAULT_PC_BOOT_ORDER either locally, or in
>> >> >> include/hw/i386/pc.h.
>> >> >> 
>> >> >> Hiding the complete initialization in a macro would be bad style, in my
>> >> >> opinion.
>> >> 
>> >> Would you accept #define DEFAULT_PC_BOOT_ORDER in pc.h?
>> >
>> >
>> > Here's what bothers me.
>> > static QEMUMachine pc_i440fx_machine_v1_6 = {
>> >     .name = "pc-i440fx-1.6",
>> >     .alias = "pc", 
>> >     .desc = "Standard PC (i440FX + PIIX, 1996)", <-- mostly copied over,
>> > 				but different for 1.3 - this is likely a bug
>> >     .init = pc_init_pci_1_6,
>> >     .hot_add_cpu = pc_hot_add_cpu, <- has to be copied over at least for
>> > 					newer PCs. Not sure about older ones -
>> > 					could be a bug or intentional
>> >     .max_cpus = 255,		<- always 255 except isapc
>> >     DEFAULT_MACHINE_OPTIONS, <- always copied
>> > };
>> >
>> >
>> > So there's a lot of boiler plate eahc time we add
>> > a machine type.
>> >
>> > DEFAULT_MACHINE_OPTIONS kind of offered a way to address
>> > that, maybe with per-version inheritance like we do
>> > for compat properties.
>> >
>> > Now you want to remove that for style reasons, so we'll
>> > have to stay with duplicated code :(
>> 
>> DEFAULT_MACHINE_OPTIONS are copied to *every* machine.  I can't see why
>> anything that's common to every machine should be duplicated in every
>> machine type definition.
>> 
>> Turns out the stuff DEFAULT_MACHINE_OPTIONS copies shouldn't be copied:
>> it's bogus for most machines.  My patch cleans that up, no more, no
>> less.
>> 
>> There are groups of related machines, such as the PC machines, which
>> have stuff in common legitimately.  Avoiding duplicating this stuff in
>> their machine initializers may be worthwhile.  However, that's not my
>> patch's aim.  Nothing in my patch precludes future de-duplication.
>> 
>> > I'm not nacking this, but I think you'll agree it's not
>> > a clear-cut improvement
>> 
>> I agree de-duplication may be worthwhile.  I further agree my patch
>> makes the existing duplication of one initializer (default boot order) a
>> bit more visible than it was before (in addition to removing its
>> existing duplication from lots of places where it makes no sense).  It
>> doesn't affect the existing duplication of all the other initializers.
>
> Now that it's visible, maybe you can be persuaded to fix it?
> If it were not for code duplication, your patch would have
> been much smaller, right?

Not really.

The patch touches all 100 machine types.  For 65 types,
DEFAULT_MACHINE_OPTIONS is simply dropped without replacement (patch
can't get smaller than that).  The remaining 35 can be grouped as
follows:

* nseries (n800, n810)

  Two machines get .default_boot_order = ""

* prep, g3beige, mac99

  Three machines either .default_boot_order = "cd" or "cad".  Really
  depends on the machine, so no duplication here.

* spapr

  Doesn't use DEFAULT_MACHINE_OPTIONS, still touched because I rename
  .boot_order to .default_boot_order.

* sun4[mdc]

  Twelve machines get .default_boot_order = "c".  Some or all may be
  related closely enough to make this duplication, but I don't know.

* pc and its variants

  Of 19 PC types, 18 get .default_boot_order = "cad", and one (xenfv)
  gets nothing.

Total diffstat is 59 files changed, 51 insertions(+), 119 deletions(-).

If we accomplished perfect de-duplication of PC other than xenfv, we'd
save 17 insertions.  If we can do the same for sun4[mdc], we'd save
another 11.

The machinery enabling de-duplication would likely change more than 28
lines, and insert *code* rather than data.

>> >                         - if we are cleaning this up
>> > it would be better to do something that fixes the
>> > code duplication.
>> 
>> The patch is not about cleaning up code duplication in related machine
>> initializers.  It's about cleaning up bogus default boot orders.
>> 
>> I'm wary of patch series mission creep :)
>
> My point is that once we have that cleanup, it's possible
> that you will want to tweak 6/6.

I definitely would not want to tweak 6/6 for that, because it's hard
enough to review as it is, and it already got competent review.  Any
further cleanup should be done on top.

The cleanup could drop up to 30 lines of trivial code changed in this
patch.  That's not nearly enough churn to make invalidating a review
that "wasn't easy" according to Laszlo worthwhile.

Re: [Qemu-devel] [PATCH v3 6/6] hw: Clean up bogus default boot order

[Michael S. Tsirkin]

On Mon, Aug 26, 2013 at 09:12:41AM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Thu, Aug 22, 2013 at 01:24:50PM +0200, Markus Armbruster wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> 
> >> > On Thu, Aug 22, 2013 at 10:39:34AM +0200, Markus Armbruster wrote:
> >> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> >> 
> >> >> > On Wed, Aug 21, 2013 at 06:01:45PM +0200, Markus Armbruster wrote:
> >> >> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> >> >> 
> >> >> >> > On Wed, Aug 21, 2013 at 05:10:56PM +0200, Markus Armbruster wrote:
> >> >> >> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> >> >> >> 
> >> >> >> >> > On Mon, Jul 22, 2013 at 01:38:47PM +0200, Markus Armbruster wrote:
> >> >> >> >> >> We set default boot order "cad" in every single machine definition
> >> >> >> >> >> except "pseries" and "moxiesim", even though very few
> >> >> >> >> >> boards actually
> >> >> >> >> >> care for boot order, and "cad" makes sense for even fewer.
> >> >> >> >> >> 
> >> >> >> >> >> Machines that care:
> >> >> >> >> >> 
> >> >> >> >> >> * pc and its variants
> >> >> >> >> >> 
> >> >> >> >> >>   Accept up to three letters 'a', 'b' (undocumented
> >> >> >> >> >> alias for 'a'),
> >> >> >> >> >>   'c', 'd' and 'n'.  Reject all others (fatal with -boot).
> >> >> >> >> >> 
> >> >> >> >> >> * nseries (n800, n810)
> >> >> >> >> >> 
> >> >> >> >> >>   Check whether order starts with 'n'.  Silently ignored
> >> >> >> >> >> otherwise.
> >> >> >> >> >> 
> >> >> >> >> >> * prep, g3beige, mac99
> >> >> >> >> >> 
> >> >> >> >> >>   Extract the first character the machine understands (subset of
> >> >> >> >> >>   'a'..'f').  Silently ignored otherwise.
> >> >> >> >> >> 
> >> >> >> >> >> * spapr
> >> >> >> >> >> 
> >> >> >> >> >>   Accept an arbitrary string (vl.c restricts it to contain only
> >> >> >> >> >>   'a'..'p', no duplicates).
> >> >> >> >> >> 
> >> >> >> >> >> * sun4[mdc]
> >> >> >> >> >> 
> >> >> >> >> >>   Use the first character.  Silently ignored otherwise.
> >> >> >> >> >> 
> >> >> >> >> >> Strip characters these machines ignore from their
> >> >> >> >> >> default boot order.
> >> >> >> >> >> 
> >> >> >> >> >> For all other machines, remove the unused default boot order
> >> >> >> >> >> alltogether.
> >> >> >> >> >> 
> >> >> >> >> >> Note that my rename of QEMUMachine member boot_order to
> >> >> >> >> >> default_boot_order and QEMUMachineInitArgs member boot_device to
> >> >> >> >> >> boot_order has a welcome side effect: it makes every use of boot
> >> >> >> >> >> orders visible in this patch, for easy review.
> >> >> >> >> >> 
> >> >> >> >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> >> >> >> >> ---
> >> >> >> >> [...]
> >> >> >> >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> >> >> >> >> >> index 9327ac1..3700bd5 100644
> >> >> >> >> >> --- a/hw/i386/pc_piix.c
> >> >> >> >> >> +++ b/hw/i386/pc_piix.c
> >> >> >> >> >> @@ -216,7 +216,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
> >> >> >> >> >>          }
> >> >> >> >> >>      }
> >> >> >> >> >>  
> >> >> >> >> >> -    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_device,
> >> >> >> >> >> +    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
> >> >> >> >> >>                   floppy, idebus[0], idebus[1], rtc_state);
> >> >> >> >> >>  
> >> >> >> >> >>      if (pci_enabled && usb_enabled(false)) {
> >> >> >> >> >> @@ -324,7 +324,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
> >> >> >> >> >>      .hot_add_cpu = pc_hot_add_cpu,
> >> >> >> >> >>      .max_cpus = 255,
> >> >> >> >> >>      .is_default = 1,
> >> >> >> >> >> -    DEFAULT_MACHINE_OPTIONS,
> >> >> >> >> >> +    .default_boot_order = "cad",
> >> >> >> >> >>  };
> >> >> >> >> >>  
> >> >> >> >> >>  static QEMUMachine pc_i440fx_machine_v1_5 = {
> >> >> >> >> >> @@ -337,7 +337,7 @@ static QEMUMachine pc_i440fx_machine_v1_5 = {
> >> >> >> >> >>          PC_COMPAT_1_5,
> >> >> >> >> >>          { /* end of list */ }
> >> >> >> >> >>      },
> >> >> >> >> >> -    DEFAULT_MACHINE_OPTIONS,
> >> >> >> >> >> +    .default_boot_order = "cad",
> >> >> >> >> >>  };
> >> >> >> >> >>  
> >> >> >> >> >>  static QEMUMachine pc_i440fx_machine_v1_4 = {
> >> >> >> >> >
> >> >> >> >> > So all PC machine types share this?
> >> >> >> >> 
> >> >> >> >> Correct, just like they share DEFAULT_MACHINE_OPTIONS before my patch.
> >> >> >> >> Which is defined as
> >> >> >> >> 
> >> >> >> >>     #define DEFAULT_MACHINE_OPTIONS \
> >> >> >> >>         .boot_order = "cad"
> >> >> >> >> 
> >> >> >> >> I.e. my patch merely peels off a layer of obfuscation :)
> >> >> >> >
> >> >> >> > Using a macro in multiple places, instead of a hard-coded constant is
> >> >> >> > not obfuscation.
> >> >> >> >
> >> >> >> >> > Can we set this in some common code, somehow?
> >> >> >> >> 
> >> >> >> >> We don't have an inheritance notion for machine types.
> >> >> >> >> 
> >> >> >> >> vl.c uses machine->boot_order before calling one of its methods, so
> >> >> >> >> monkey-patching .boot_order from a method won't do.
> >> >> >> >> Besides, that cure
> >> >> >> >> looks much worse than the disease to me.
> >> >> >> >> 
> >> >> >> >> Can't think of anything else offhand.
> >> >> >> >> 
> >> >> >> >> [...]
> >> >> >> >
> >> >> >> > Set this in pc_init_pci somehow?
> >> >> >> 
> >> >> >> Too late, see "vl.c uses..." above.
> >> >> >
> >> >> > Ah, missed it.
> >> >> > Can we fix that?
> >> >> 
> >> >> If I understand you correctly, you want to monkey-patch
> >> >> machine->boot_order from machine->init().  Issues:
> >> >> 
> >> >> * machine->init() does not have access to machine.  Fixable.
> >> >> 
> >> >> * machine is read-only, except for a few places in vl.c (one is managing
> >> >>   the list of registered machines, the other monkey-patches
> >> >>   machine->max_cpus to one when it's zero).  I don't want *more*
> >> >>   monkey-patching, I want *less*, so we can make machines const.  In
> >> >>   fact, we can make current_machine const right away, and I think we
> >> >>   should (patch coming).
> >> >> 
> >> >> * If machine->init() can change the default boot order, we get a data
> >> >>   dependency cycle.  Current data dependency chain:
> >> >> 
> >> >>   0. Initialize QEMUMachine *machine to the default machine.
> >> >> 
> >> >>   1. Option parsing sets machine, and collects "boot-opts" options.
> >> >> 
> >> >>   2. Evaluation of "boot-opts": find normal boot order (from
> >> >>      machine->boot_order and option parameter "boot") and one-time boot
> >> >>      order (if option parameter "once" is given).
> >> >> 
> >> >>      Set boot_order to the initial boot order.
> >> >> 
> >> >>      Register a reset handler to revert the boot order from one-time to
> >> >>      normal, if necessary.
> >> >> 
> >> >>   3. machine->init()
> >> >> 
> >> >>      Gets passed boot_order via QEMUMachineInitArgs.  Currently doesn't
> >> >>      have access to machine.
> >> >> 
> >> >>   4. Set global variable current_machine = machine.
> >> >> 
> >> >>   Cycle: step 2 uses default boot order and defines boot order, step 3
> >> >>   uses boot order and defines default boot order.
> >> >> 
> >> >>   I guess we could break this cycle by some sufficiently ingenious code
> >> >>   shuffling.  But I'm pretty sure that would only complicate matters.
> >> >>   Right now, boot order data flows down the program text, which makes it
> >> >>   easy to understand.
> >> >
> >> > I agree, it's a concern.
> >> >
> >> >> >> > Set DEFAULT_MACHINE_OPTIONS locally in this file?
> >> >> >> 
> >> >> >> I can do #define CAD "cad" for you ;)
> >> >> >> 
> >> >> >> Seriously, I'd be okay with ".bootorder = DEFAULT_PC_BOOT_ORDER", with
> >> >> >> #define DEFAULT_PC_BOOT_ORDER either locally, or in
> >> >> >> include/hw/i386/pc.h.
> >> >> >> 
> >> >> >> Hiding the complete initialization in a macro would be bad style, in my
> >> >> >> opinion.
> >> >> 
> >> >> Would you accept #define DEFAULT_PC_BOOT_ORDER in pc.h?
> >> >
> >> >
> >> > Here's what bothers me.
> >> > static QEMUMachine pc_i440fx_machine_v1_6 = {
> >> >     .name = "pc-i440fx-1.6",
> >> >     .alias = "pc", 
> >> >     .desc = "Standard PC (i440FX + PIIX, 1996)", <-- mostly copied over,
> >> > 				but different for 1.3 - this is likely a bug
> >> >     .init = pc_init_pci_1_6,
> >> >     .hot_add_cpu = pc_hot_add_cpu, <- has to be copied over at least for
> >> > 					newer PCs. Not sure about older ones -
> >> > 					could be a bug or intentional
> >> >     .max_cpus = 255,		<- always 255 except isapc
> >> >     DEFAULT_MACHINE_OPTIONS, <- always copied
> >> > };
> >> >
> >> >
> >> > So there's a lot of boiler plate eahc time we add
> >> > a machine type.
> >> >
> >> > DEFAULT_MACHINE_OPTIONS kind of offered a way to address
> >> > that, maybe with per-version inheritance like we do
> >> > for compat properties.
> >> >
> >> > Now you want to remove that for style reasons, so we'll
> >> > have to stay with duplicated code :(
> >> 
> >> DEFAULT_MACHINE_OPTIONS are copied to *every* machine.  I can't see why
> >> anything that's common to every machine should be duplicated in every
> >> machine type definition.
> >> 
> >> Turns out the stuff DEFAULT_MACHINE_OPTIONS copies shouldn't be copied:
> >> it's bogus for most machines.  My patch cleans that up, no more, no
> >> less.
> >> 
> >> There are groups of related machines, such as the PC machines, which
> >> have stuff in common legitimately.  Avoiding duplicating this stuff in
> >> their machine initializers may be worthwhile.  However, that's not my
> >> patch's aim.  Nothing in my patch precludes future de-duplication.
> >> 
> >> > I'm not nacking this, but I think you'll agree it's not
> >> > a clear-cut improvement
> >> 
> >> I agree de-duplication may be worthwhile.  I further agree my patch
> >> makes the existing duplication of one initializer (default boot order) a
> >> bit more visible than it was before (in addition to removing its
> >> existing duplication from lots of places where it makes no sense).  It
> >> doesn't affect the existing duplication of all the other initializers.
> >
> > Now that it's visible, maybe you can be persuaded to fix it?
> > If it were not for code duplication, your patch would have
> > been much smaller, right?
> 
> Not really.
> 
> The patch touches all 100 machine types.  For 65 types,
> DEFAULT_MACHINE_OPTIONS is simply dropped without replacement (patch
> can't get smaller than that).  The remaining 35 can be grouped as
> follows:
> 
> * nseries (n800, n810)
> 
>   Two machines get .default_boot_order = ""
> 
> * prep, g3beige, mac99
> 
>   Three machines either .default_boot_order = "cd" or "cad".  Really
>   depends on the machine, so no duplication here.
> 
> * spapr
> 
>   Doesn't use DEFAULT_MACHINE_OPTIONS, still touched because I rename
>   .boot_order to .default_boot_order.
> 
> * sun4[mdc]
> 
>   Twelve machines get .default_boot_order = "c".  Some or all may be
>   related closely enough to make this duplication, but I don't know.
> 
> * pc and its variants
> 
>   Of 19 PC types, 18 get .default_boot_order = "cad", and one (xenfv)
>   gets nothing.
> 
> Total diffstat is 59 files changed, 51 insertions(+), 119 deletions(-).
> 
> If we accomplished perfect de-duplication of PC other than xenfv, we'd
> save 17 insertions.  If we can do the same for sun4[mdc], we'd save
> another 11.
> 
> The machinery enabling de-duplication would likely change more than 28
> lines, and insert *code* rather than data.
> 
> >> >                         - if we are cleaning this up
> >> > it would be better to do something that fixes the
> >> > code duplication.
> >> 
> >> The patch is not about cleaning up code duplication in related machine
> >> initializers.  It's about cleaning up bogus default boot orders.
> >> 
> >> I'm wary of patch series mission creep :)
> >
> > My point is that once we have that cleanup, it's possible
> > that you will want to tweak 6/6.
> 
> I definitely would not want to tweak 6/6 for that, because it's hard
> enough to review as it is, and it already got competent review.  Any
> further cleanup should be done on top.
> 
> The cleanup could drop up to 30 lines of trivial code changed in this
> patch.  That's not nearly enough churn to make invalidating a review
> that "wasn't easy" according to Laszlo worthwhile.

I'm fine with cleanup being on top if this helps.

Re: [Qemu-devel] [PATCH v3 6/6] hw: Clean up bogus default boot order

[Paolo Bonzini]

Il 26/08/2013 09:12, Markus Armbruster ha scritto:
> * pc and its variants
> 
>   Of 19 PC types, 18 get .default_boot_order = "cad", and one (xenfv)
>   gets nothing.

I think you mean xenpv---which isn't a PC machine, in principle (or so
Stefano told me) it could be used for ARM as well.

Paolo

Re: [Qemu-devel] [PATCH v3 6/6] hw: Clean up bogus default boot order

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 26/08/2013 09:12, Markus Armbruster ha scritto:
>> * pc and its variants
>> 
>>   Of 19 PC types, 18 get .default_boot_order = "cad", and one (xenfv)
>>   gets nothing.
>
> I think you mean xenpv---which isn't a PC machine, in principle (or so
> Stefano told me) it could be used for ARM as well.


Yes, I slipped in the alphabet soup.  Thanks for the correction.

Re: [Qemu-devel] [PATCH v3 0/6] Clean up bogus default boot order

[Markus Armbruster]

Any chance for this to make 1.6?  It first hit the list mid-June...