[PATCH v3] discover/grub: Add blscfg command support to parse BootLoaderSpec files

[Javier Martinez Canillas]

On 03/21/2018 01:50 AM, Brett Grandbois wrote:
> 
> 
> On 21/03/18 09:59, Javier Martinez Canillas wrote:
>> On 03/20/2018 11:45 PM, Brett Grandbois wrote:
>>> Tested it out and it works.  I just noticed however that there is no
>>> support for default in it so somewhere in bls_finish it would be nice to
>> Right, I wrongly assumed that this would already work by just setting a
>> default env var in grub.cfg, but now see that was missing support for it.
>>
>>> have an option->is_default check.  The concept of index doesn't seem to
>>> apply in BLS like it does in a list of menuentries so probably the best
>>> way to go is to do the default env comparison on title or machine_id if
>>> either exist.
>>>
>> There's already an option_is_default() in discover/grub2/script.c so I
>> think we should use it for consistency. The logic there checks if there
>> is a default env var and if it's a number, then uses it as an index. If
>> isn't a number, then it compares first with opt->id or as a fallback to
>> opt->name.
>>
>> So since users expects to set default to the menu entry name as shown
>> in the UI (as is the case for grub2 without BLS) then we should either
>> set opt->id to opt->name or not set opt->id at all.
> But option_is_default is called from within the menuentry parser and 
> expects that syntax environment where BLS is a different syntax so I 
> don't think consistency is required here if it turns out 
> option_is_default is unsuitable.
> 
> Do all grub users expect to set the default to the UI menu display entry 
> name?  For systems with more automated startup scripting (like ours) 

Well, from my point of view BLS should be transparent to the user so if
they already are used to have something like:

saved_entry=Fedora (4.15.9-300.fc27.x86_64) 27 (Workstation Edition)

in their grubenv and then set default="${saved_entry}" in the grub.cfg, it
should still work after switching to use BLS to populate the menu entries.

> using a unique --id (or equivalent) field makes automatic boot 
> determination in the grub.cfg simpler.  In the current menuentry parser
> the option->id is first populated to the --id field if present and only 
> falls back to name if id isn't present.
>

Right, there's no equivalent of --id in BLS though. We do have an id field
in our generated bls.conf but I didn't add it since is a slightly deviation
from the spec:

https://src.fedoraproject.org/rpms/kernel/blob/master/f/generate_bls_conf.sh

But if you think is useful, I can add it as another optional field and set
opt->name to it as another fallback.

> For BLS as I mentioned in my follow-up email, I actually now think the 
> id should be the filename minus the .conf extension as the spec states:
>
>> The file name of the file is used for identification of the boot item
> Which sounds to me like the perfect candidate for the option->id field.  
> Then any combination of title/machine-id/version can be displayed to the 
> UI without any fear of an id name collision.
>

Yes, sorry for missing that follow-up email. I don't mind to use the file
name to set option->id, but as mentioned I really think we should use the
option->name to check the default option to avoid changing grub2 behavior.

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat