Peter Maydell <[email protected]> writes:
> On 25 October 2012 15:38, Avik Sil <[email protected]> wrote:
>> @@ -171,6 +171,7 @@ static QEMUMachine clipper_machine = {
>> .init = clipper_init,
>> .max_cpus = 4,
>> .is_default = 1,
>> + .default_machine_opts = DEFAULT_BOOT_ORDER,
>> };
>
>> @@ -86,6 +86,7 @@ static QEMUMachine an5206_machine = {
>> .name = "an5206",
>> .desc = "Arnewsh 5206",
>> .init = an5206_init,
>> + .default_machine_opts = DEFAULT_BOOT_ORDER,
>> };
>
>> +++ b/hw/axis_dev88.c
>> @@ -355,6 +355,7 @@ static QEMUMachine axisdev88_machine = {
>> .desc = "AXIS devboard 88",
>> .init = axisdev88_init,
>> .is_default = 1,
>> + .default_machine_opts = DEFAULT_BOOT_ORDER,
>> };
>
> The thing about a default is that you shouldn't have to
> explicitly say it in every QEMUMachine struct... Please
> make the code handle a NULL in the struct in a way
> that means you don't need to touch every board.
I hate to spin a lot on a touch everything patch, but I also agree that
this is the wrong approach.
I think there are two reasonable approaches to this. One would be a
macro to provide default initialization. Something along the lines of:
#define DEFAULT_MACHINE_OPTIONS \
.boot_order = "cad"
And then:
static QEMUMachine axisdev88_machine = {
DEFAULT_MACHINE_OPTIONS,
...
};
static QEMUMachine pseries_machine = {
DEFAULT_MACHINE_OPTIONS,
/* no default boot order so we can detect absence of boot option */
.boot_order = NULL,
};
Baking this into .default_machines_opts is too much overloading.
The other approach to this would be:
static QEMUMachine pseries_machine = {
.no_boot_order = 1,
};
Which I think is what Peter is suggesting. I'm not a huge fan of this
because it's backwards logic but we already do this for a bunch of other
things so I can't object too strongly to it.
Regards,
Anthony Liguori
>
> -- PMM