[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [GRUB PARTUUID PATCH V9 4/5] Update grub script template files
From: |
Nick Vinson |
Subject: |
Re: [GRUB PARTUUID PATCH V9 4/5] Update grub script template files |
Date: |
Wed, 11 Apr 2018 07:45:01 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 |
On 04/11/2018 01:31 AM, Daniel Kiper wrote:
> 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.
Ack. I will update the commit comment.
>
>> 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?
We should.
I split it so that if #5 proved controversial for some reason and
received a lot of resistance, it could be dropped and the remainder of
the patchset could be merged.
There does not seem to be any issue with setting
GRUB_DISABLE_LINUX_PARTUUID to a default value, so there is no reason to
keep it separate.
Thanks,
Nicholas Vinson
>
> Daniel
>
- [GRUB PARTUUID PATCH V9 0/5] Add PARTUUID detection support, Nicholas Vinson, 2018/04/07
- [GRUB PARTUUID PATCH V9 1/5] Centralize guid prints, Nicholas Vinson, 2018/04/07
- [GRUB PARTUUID PATCH V9 2/5] Update grub_gpt_partentry, Nicholas Vinson, 2018/04/07
- [GRUB PARTUUID PATCH V9 3/5] Add PARTUUID detection support to grub-probe, Nicholas Vinson, 2018/04/07
- [GRUB PARTUUID PATCH V9 5/5] Default to disabling partition UUID support, Nicholas Vinson, 2018/04/07
- [GRUB PARTUUID PATCH V9 4/5] Update grub script template files, Nicholas Vinson, 2018/04/07
Re: [GRUB PARTUUID PATCH V9 0/5] Add PARTUUID detection support, Daniel Kiper, 2018/04/10