[PATCH v3] discover/grub: Add blscfg command support to parse BootLoaderSpec files
brett.grandbois at opengear.com
Wed Mar 21 11:50:10 AEDT 2018
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
> 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)
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.
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.
> So I think it should be
> if title
> else if machine_id && version
> else if version
> else if dev->device->id && state->image
>> I know I originally suggested it, but looking at the implementation I
>> can see that having option->name be a mash up of machine_id and version
>> isn't the way to go. It looks much cleaner to just have:
>> if title
>> else if machine_id
> But the different BLS fragments will have the same machine_id, so this
> can't be used alone.
Ah sorry, you're right there. I had misinterpreted that key.
>> else if version
>> which then makes the default check easy has you can do it based on an
>> option->name comparison afterwards.
> Best regards,
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Petitboot