Re: [Qemu-devel] [PATCH 2/2] fw_cfg: set the get_boot_devices_list() ignore_suffixes parameter from machine property

[Laszlo Ersek]

On 08/06/18 07:50, Thomas Huth wrote:
> On 08/05/2018 01:28 PM, Mark Cave-Ayland wrote:
>> For the older machines (such as Mac and SPARC) the DT nodes representing
>> bootdevices for disk nodes are irregular for mainly historical reasons, and
>> should be handled on an individual basis via a custom FWPathProvider.
>>
>> Since the majority of bootdevice nodes for these machines either do not have 
>> a
>> separate disk node or require different (custom) names then it is much easier
>> to allow the ignore_suffixes parameter to be set on a per-machine basis via
>> a machine property.
>>
>> The default value for this new fwcfg_bootdevice_ignore_suffixes machine
>> property is false to preserve compatibility for existing machines.
>>
>> Signed-off-by: Mark Cave-Ayland <address@hidden>
>> ---
>>  hw/core/machine.c   | 3 +++
>>  hw/nvram/fw_cfg.c   | 5 ++++-
>>  include/hw/boards.h | 1 +
>>  3 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index a9aeb22f03..fbadb35865 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -525,6 +525,9 @@ static void machine_class_init(ObjectClass *oc, void 
>> *data)
>>      mc->default_ram_size = 128 * MiB;
>>      mc->rom_file_has_mr = true;
>>  
>> +    /* Default to using fwcfg bootdevice suffixes */
>> +    mc->fwcfg_bootdevice_ignore_suffixes = false;
> 
> I guess you could omit this line since the memory for the machine class
> is pre-initialized to zero.

I was about to make the same recommendation.

I believe the patch should be respun for this; while the assignment is
correct / harmless, I believe we should stay consistent with the rest of
the code, and assign machine class fields when really necessary.


Another remark: I think the subject line is a bit too long (87
characters). How about:

  fw_cfg: ignore suffixes in the bootdev list dependent on machine class

(70 chars -- hopefully still precise enough)

Apologies about the bike-shedding, of course.

Thanks!
Laszlo

> Anyway:
> 
> Reviewed-by: Thomas Huth <address@hidden>
>