<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 21/03/18 09:59, Javier Martinez
Canillas wrote:<br>
</div>
<blockquote type="cite"
cite="mid:b55c1cc9-23fc-5a33-4661-b859ae53834b@redhat.com">
<pre wrap="">On 03/20/2018 11:45 PM, Brett Grandbois wrote:
</pre>
<blockquote type="cite">
<pre wrap="">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
</pre>
</blockquote>
<pre wrap="">
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.
</pre>
<blockquote type="cite">
<pre wrap="">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.
</pre>
</blockquote>
<pre wrap="">
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.</pre>
</blockquote>
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.<br>
<br>
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. <br>
<br>
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:<br>
<br>
<blockquote type="cite"><span style="color: rgb(0, 0, 0);
font-family: "Times New Roman"; font-size: 16.16px;
font-style: normal; font-variant-ligatures: normal;
font-variant-caps: normal; font-weight: 400; letter-spacing:
normal; orphans: 2; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: 2;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); text-decoration-style:
initial; text-decoration-color: initial; display: inline
!important; float: none;">The file name of the file is used for
identification of the boot item</span></blockquote>
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.<br>
<br>
<br>
<br>
<blockquote type="cite"
cite="mid:b55c1cc9-23fc-5a33-4661-b859ae53834b@redhat.com">
<pre wrap="">
So I think it should be
if title
else if machine_id && version
else if version
else if dev->device->id && state->image
</pre>
<blockquote type="cite">
<pre wrap="">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
</pre>
</blockquote>
<pre wrap="">
But the different BLS fragments will have the same machine_id, so this
can't be used alone.
</pre>
</blockquote>
Ah sorry, you're right there. I had misinterpreted that key.<br>
<blockquote type="cite"
cite="mid:b55c1cc9-23fc-5a33-4661-b859ae53834b@redhat.com">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">else if version
else
which then makes the default check easy has you can do it based on an
option->name comparison afterwards.
Brett
</pre>
</blockquote>
<pre wrap="">
Best regards,
</pre>
</blockquote>
<br>
</body>
</html>