On Wed, Aug 21, 2013 at 05:10:56PM +0200, Markus Armbruster wrote: > "Michael S. Tsirkin" <m...@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 <arm...@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