Re: [GRUB PARTUUID PATCH V9 4/5] Update grub script template files

[Daniel Kiper]

On Tue, Apr 10, 2018 at 08:00:04PM -0700, Nick Vinson wrote:
> On 04/10/2018 01:52 PM, Daniel Kiper wrote:
> > On Sat, Apr 07, 2018 at 04:28:13PM -0700, Nicholas Vinson wrote:
> >> Update grub-mkconfig.in and 10_linux.in to support grub-probe's new
> >> partuuid target.  Update grub.texi documentation.  The following table
> >> shows how GRUB_DISABLE_LINUX_UUID, GRUB_DISABLE_LINUX_PARTUUID, and
> >> initramfs detection interact:
> >>
> >> Initramfs  GRUB_DISABLE_LINUX_PARTUUID  GRUB_DISABLE_LINUX_UUID  Linux Root
> >> detected   Set                          Set                      ID Method
> >>
> >> False      False                        False                    part UUID
> >> False      False                        True                     part UUID
> >> False      True                         False                    dev name
> >> False      True                         True                     dev name
> >> True       False                        False                    fs UUID
> >> True       False                        True                     part UUID
> >> True       True                         False                    fs UUID
> >> True       True                         True                     dev name
> >
> > What will happen if GRUB_DISABLE_LINUX_PARTUUID and/or 
> > GRUB_DISABLE_LINUX_UUID
> > are not set? I think that you can avoid that by setting defaults. You do 
> > that
> > for GRUB_DISABLE_LINUX_PARTUUID in next patch but GRUB_DISABLE_LINUX_UUID
> > does not have any default.
> >
>
> If they're not set, then that's the same as them being set to 'False'.
> I should have worded my table above a bit differently and used Yes/No
> instead of True/False as that is really what it is trying to convey.

IMO it will be more confusing. I think that I would use lowercase
false/true as it is used in the script and below the table I would
add a note that <VARIABLE_UNSET> == false or something like that.

> I.E. If there is no initramfs detected and GRUB_DISABLE_LINUX_PARTUUID
> is not set and GRUB_DISABLE_LINUX_UUID is not set, set the kernel root
> variable to "root=PARTUUID=...".
>
> In the scripts, the only value explicitly checked for is 'true'.
> Anything else (including unset) is considered false.

OK, perfect!

> >> Signed-off-by: Nicholas Vinson <address@hidden>
> >> ---
> >>  docs/grub.texi          | 10 ++++++++++
> >>  util/grub-mkconfig.in   |  3 +++
> >>  util/grub.d/10_linux.in | 18 +++++++++++++++---
> >
> > Could you update util/grub.d/20_linux_xen.in too?
>
> Let me see what I can do.  I have never worked with xen before, so I do
> not know much about it.

This should be easy. However, if you are not sure please drop me a line.

> >>  3 files changed, 28 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/docs/grub.texi b/docs/grub.texi
> >> index 65b4bbeda..06f0afe45 100644
> >> --- a/docs/grub.texi
> >> +++ b/docs/grub.texi
> >> @@ -1424,6 +1424,16 @@ the Linux kernel, using a @samp{root=UUID=...} 
> >> kernel parameter.  This is
> >>  usually more reliable, but in some cases it may not be appropriate.  To
> >>  disable the use of UUIDs, set this option to @samp{true}.
> >>
> >> address@hidden GRUB_DISABLE_LINUX_PARTUUID
> >> +If @command{grub-mkconfig} cannot identify the root filesystem via its
> >> +universally-unique indentifier (UUID), @command{grub-mkconfig} will use 
> >> the UUID
> >> +of the partition containing the filesystem to identify the root 
> >> filesystem to
> >> +the Linux kernel via a @samp{root=PARTUUID=...} kernel parameter.  This 
> >> is not
> >> +as reliable as using the filesystem UUID, but is more reliable than using 
> >> the
> >> +Linux device names.  When enabled, this option requires the Linux kernel 
> >> version
> >
> > s/When enabled, /When enabled, GRUB_DISABLE_LINUX_PARTUUID set to false, /
> > or something like that. Otherwise it is unclear.
>
> I'll clean up the wording in the next release.

Thanks a lot!

By the way, should not we merge patches 4 and 5 into one patch?

Daniel