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

[Javier Martinez Canillas]

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.

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.

> else if version
> 
> else
> 
> 
> which then makes the default check easy has you can do it based on an 
> option->name comparison afterwards.
> 
> Brett
> 

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