[PATCH v2] Add GRUB_DISABLE_UUID

[PATCH v2] Add GRUB_DISABLE_UUID

[Javier Martinez Canillas]

From: Peter Jones <pjones@redhat.com>

The grub-mkconfig and 10_linux scripts by default attempt to use a UUID to
set the root kernel command line parameter and the $root GRUB environment
variable.

The former can be disabled by setting the GRUB_DISABLE_LINUX_UUID variable
to "true", but there is currently no way to disable the latter.

The generated grub config uses the search command with the --fs-uuid option
to find the device that has to be set as $root, i.e:

 search --no-floppy --fs-uuid --set=root ...

This is usually more reliable but in some cases it may not be appropriate,
so this patch introduces a new GRUB_DISABLE_UUID variable that can be used
to disable searching for the $root device by filesystem UUID.

When disabled, the $root device will be set to the value specified in the
device.map as found by the grub-probe --target=compatibility_hint option.

When setting GRUB_DISABLE_UUID=true, the GRUB_DISABLE_LINUX_UUID and
GRUB_DISABLE_LINUX_PARTUUID variables will also be set to "true" unless
these have been explicitly set to "false".

That way, the GRUB_DISABLE_UUID variable can be used to force using the
device names for both GRUB and Linux.

Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>


---

Changes since v1 (suggested by Daniel Kiper):

- Explain better in the commit message why the GRUB_DISABLE_UUID variable
  is needed and the difference with the existing GRUB_DISABLE_LINUX_UUID/
  GRUB_DISABLE_LINUX_PARTUUID variables.
- Remove logic that disabled setting the root cmdline param to either the
  filesystem UUID or partition UUID and instead use the existing variables
  to disable this.

 docs/grub.texi            |  9 +++++++++
 util/grub-mkconfig.in     | 10 ++++++++++
 util/grub-mkconfig_lib.in |  4 ++--
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/docs/grub.texi b/docs/grub.texi
index 5ac61c09d1b..2fe0e1b83bb 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -1441,6 +1441,15 @@ enable the use of partition UUIDs, set this option to @samp{false}.
 If this option is set to @samp{true}, disable the generation of recovery
 mode menu entries.
 
+@item GRUB_DISABLE_UUID
+Normally, @command{grub-mkconfig} will generate menu entries that use
+universally-unique identifiers (UUIDs) to identify various filesystems to
+search for files.  This is usually more reliable, but in some cases it may
+not be appropriate.  To disable this use of UUIDs, set this option to
+@samp{true}. Setting this option to @samp{true}, will also set the options
+@samp{GRUB_DISABLE_LINUX_UUID} and @samp{GRUB_DISABLE_LINUX_PARTUUID} to
+@samp{true}, unless they have been explicilty set to @samp{false}.
+
 @item GRUB_VIDEO_BACKEND
 If graphical video support is required, either because the @samp{gfxterm}
 graphical terminal is in use or because @samp{GRUB_GFXPAYLOAD_LINUX} is set,
diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
index a6ce375ed18..2537dbca2f9 100644
--- a/util/grub-mkconfig.in
+++ b/util/grub-mkconfig.in
@@ -158,6 +158,15 @@ if test -f ${sysconfdir}/default/grub ; then
   . ${sysconfdir}/default/grub
 fi
 
+if [ "x${GRUB_DISABLE_UUID}" = "xtrue" ]; then
+  if [ "x${GRUB_DISABLE_LINUX_UUID}" != "xfalse" ]; then
+    GRUB_DISABLE_LINUX_UUID="true"
+  fi
+  if [ "x${GRUB_DISABLE_LINUX_PARTUUID}" != "xfalse" ]; then
+    GRUB_DISABLE_LINUX_PARTUUID="true"
+  fi
+fi
+
 # XXX: should this be deprecated at some point?
 if [ "x${GRUB_TERMINAL}" != "x" ] ; then
   GRUB_TERMINAL_INPUT="${GRUB_TERMINAL}"
@@ -227,6 +236,7 @@ export GRUB_DEFAULT \
   GRUB_DISABLE_LINUX_UUID \
   GRUB_DISABLE_LINUX_PARTUUID \
   GRUB_DISABLE_RECOVERY \
+  GRUB_DISABLE_UUID \
   GRUB_VIDEO_BACKEND \
   GRUB_GFXMODE \
   GRUB_BACKGROUND \
diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
index 0f801cab3e4..3e12b2c0e6a 100644
--- a/util/grub-mkconfig_lib.in
+++ b/util/grub-mkconfig_lib.in
@@ -156,7 +156,7 @@ prepare_grub_to_access_device ()
   if [ "x$fs_hint" != x ]; then
     echo "set root='$fs_hint'"
   fi
-  if fs_uuid="`"${grub_probe}" --device $@ --target=fs_uuid 2> /dev/null`" ; then
+  if [ "x${GRUB_DISABLE_UUID}" != "xtrue" ] && fs_uuid="`"${grub_probe}" --device $@ --target=fs_uuid 2> /dev/null`" ; then
     hints="`"${grub_probe}" --device $@ --target=hints_string 2> /dev/null`" || hints=
     echo "if [ x\$feature_platform_search_hint = xy ]; then"
     echo "  search --no-floppy --fs-uuid --set=root ${hints} ${fs_uuid}"
@@ -173,7 +173,7 @@ grub_get_device_id ()
   IFS='
 '
   device="$1"
-  if fs_uuid="`"${grub_probe}" --device ${device} --target=fs_uuid 2> /dev/null`" ; then
+  if [ "x${GRUB_DISABLE_UUID}" != "xtrue" ] && fs_uuid="`"${grub_probe}" --device ${device} --target=fs_uuid 2> /dev/null`" ; then
     echo "$fs_uuid";
   else
     echo $device |sed 's, ,_,g'
-- 
2.21.0

Re: [PATCH v2] Add GRUB_DISABLE_UUID

[Daniel Kiper]

Adding Nicholas...

On Mon, Sep 30, 2019 at 04:59:12PM +0200, Javier Martinez Canillas wrote:
> From: Peter Jones <pjones@redhat.com>
>
> The grub-mkconfig and 10_linux scripts by default attempt to use a UUID to
> set the root kernel command line parameter and the $root GRUB environment
> variable.
>
> The former can be disabled by setting the GRUB_DISABLE_LINUX_UUID variable
> to "true", but there is currently no way to disable the latter.
>
> The generated grub config uses the search command with the --fs-uuid option
> to find the device that has to be set as $root, i.e:
>
>  search --no-floppy --fs-uuid --set=root ...
>
> This is usually more reliable but in some cases it may not be appropriate,
> so this patch introduces a new GRUB_DISABLE_UUID variable that can be used
> to disable searching for the $root device by filesystem UUID.
>
> When disabled, the $root device will be set to the value specified in the
> device.map as found by the grub-probe --target=compatibility_hint option.
>
> When setting GRUB_DISABLE_UUID=true, the GRUB_DISABLE_LINUX_UUID and
> GRUB_DISABLE_LINUX_PARTUUID variables will also be set to "true" unless
> these have been explicitly set to "false".
>
> That way, the GRUB_DISABLE_UUID variable can be used to force using the
> device names for both GRUB and Linux.
>
> Signed-off-by: Peter Jones <pjones@redhat.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

In general "Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>" but I would
like to hear Nicholas opinions here too.

Daniel

> ---
>
> Changes since v1 (suggested by Daniel Kiper):
>
> - Explain better in the commit message why the GRUB_DISABLE_UUID variable
>   is needed and the difference with the existing GRUB_DISABLE_LINUX_UUID/
>   GRUB_DISABLE_LINUX_PARTUUID variables.
> - Remove logic that disabled setting the root cmdline param to either the
>   filesystem UUID or partition UUID and instead use the existing variables
>   to disable this.
>
>  docs/grub.texi            |  9 +++++++++
>  util/grub-mkconfig.in     | 10 ++++++++++
>  util/grub-mkconfig_lib.in |  4 ++--
>  3 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 5ac61c09d1b..2fe0e1b83bb 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -1441,6 +1441,15 @@ enable the use of partition UUIDs, set this option to @samp{false}.
>  If this option is set to @samp{true}, disable the generation of recovery
>  mode menu entries.
>
> +@item GRUB_DISABLE_UUID
> +Normally, @command{grub-mkconfig} will generate menu entries that use
> +universally-unique identifiers (UUIDs) to identify various filesystems to
> +search for files.  This is usually more reliable, but in some cases it may
> +not be appropriate.  To disable this use of UUIDs, set this option to
> +@samp{true}. Setting this option to @samp{true}, will also set the options
> +@samp{GRUB_DISABLE_LINUX_UUID} and @samp{GRUB_DISABLE_LINUX_PARTUUID} to
> +@samp{true}, unless they have been explicilty set to @samp{false}.
> +
>  @item GRUB_VIDEO_BACKEND
>  If graphical video support is required, either because the @samp{gfxterm}
>  graphical terminal is in use or because @samp{GRUB_GFXPAYLOAD_LINUX} is set,
> diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
> index a6ce375ed18..2537dbca2f9 100644
> --- a/util/grub-mkconfig.in
> +++ b/util/grub-mkconfig.in
> @@ -158,6 +158,15 @@ if test -f ${sysconfdir}/default/grub ; then
>    . ${sysconfdir}/default/grub
>  fi
>
> +if [ "x${GRUB_DISABLE_UUID}" = "xtrue" ]; then
> +  if [ "x${GRUB_DISABLE_LINUX_UUID}" != "xfalse" ]; then
> +    GRUB_DISABLE_LINUX_UUID="true"
> +  fi
> +  if [ "x${GRUB_DISABLE_LINUX_PARTUUID}" != "xfalse" ]; then
> +    GRUB_DISABLE_LINUX_PARTUUID="true"
> +  fi
> +fi
> +
>  # XXX: should this be deprecated at some point?
>  if [ "x${GRUB_TERMINAL}" != "x" ] ; then
>    GRUB_TERMINAL_INPUT="${GRUB_TERMINAL}"
> @@ -227,6 +236,7 @@ export GRUB_DEFAULT \
>    GRUB_DISABLE_LINUX_UUID \
>    GRUB_DISABLE_LINUX_PARTUUID \
>    GRUB_DISABLE_RECOVERY \
> +  GRUB_DISABLE_UUID \
>    GRUB_VIDEO_BACKEND \
>    GRUB_GFXMODE \
>    GRUB_BACKGROUND \
> diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
> index 0f801cab3e4..3e12b2c0e6a 100644
> --- a/util/grub-mkconfig_lib.in
> +++ b/util/grub-mkconfig_lib.in
> @@ -156,7 +156,7 @@ prepare_grub_to_access_device ()
>    if [ "x$fs_hint" != x ]; then
>      echo "set root='$fs_hint'"
>    fi
> -  if fs_uuid="`"${grub_probe}" --device $@ --target=fs_uuid 2> /dev/null`" ; then
> +  if [ "x${GRUB_DISABLE_UUID}" != "xtrue" ] && fs_uuid="`"${grub_probe}" --device $@ --target=fs_uuid 2> /dev/null`" ; then
>      hints="`"${grub_probe}" --device $@ --target=hints_string 2> /dev/null`" || hints=
>      echo "if [ x\$feature_platform_search_hint = xy ]; then"
>      echo "  search --no-floppy --fs-uuid --set=root ${hints} ${fs_uuid}"
> @@ -173,7 +173,7 @@ grub_get_device_id ()
>    IFS='
>  '
>    device="$1"
> -  if fs_uuid="`"${grub_probe}" --device ${device} --target=fs_uuid 2> /dev/null`" ; then
> +  if [ "x${GRUB_DISABLE_UUID}" != "xtrue" ] && fs_uuid="`"${grub_probe}" --device ${device} --target=fs_uuid 2> /dev/null`" ; then
>      echo "$fs_uuid";
>    else
>      echo $device |sed 's, ,_,g'
> --
> 2.21.0

Re: [PATCH v2] Add GRUB_DISABLE_UUID

[Nicholas Vinson]

On 10/2/19 08:09, Daniel Kiper wrote:
> Adding Nicholas...
> 
> On Mon, Sep 30, 2019 at 04:59:12PM +0200, Javier Martinez Canillas wrote:
>> From: Peter Jones <pjones@redhat.com>
>>
>> The grub-mkconfig and 10_linux scripts by default attempt to use a UUID to
>> set the root kernel command line parameter and the $root GRUB environment
>> variable.
>>
>> The former can be disabled by setting the GRUB_DISABLE_LINUX_UUID variable
>> to "true", but there is currently no way to disable the latter.
>>
>> The generated grub config uses the search command with the --fs-uuid option
>> to find the device that has to be set as $root, i.e:
>>
>>   search --no-floppy --fs-uuid --set=root ...
>>
>> This is usually more reliable but in some cases it may not be appropriate,
>> so this patch introduces a new GRUB_DISABLE_UUID variable that can be used
>> to disable searching for the $root device by filesystem UUID.
>>
>> When disabled, the $root device will be set to the value specified in the
>> device.map as found by the grub-probe --target=compatibility_hint option.
>>
>> When setting GRUB_DISABLE_UUID=true, the GRUB_DISABLE_LINUX_UUID and
>> GRUB_DISABLE_LINUX_PARTUUID variables will also be set to "true" unless
>> these have been explicitly set to "false".
>>
>> That way, the GRUB_DISABLE_UUID variable can be used to force using the
>> device names for both GRUB and Linux.
>>
>> Signed-off-by: Peter Jones <pjones@redhat.com>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> 
> In general "Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>" but I would
> like to hear Nicholas opinions here too >
> Daniel
> 
>> ---
>>
>> Changes since v1 (suggested by Daniel Kiper):
>>
>> - Explain better in the commit message why the GRUB_DISABLE_UUID variable
>>    is needed and the difference with the existing GRUB_DISABLE_LINUX_UUID/
>>    GRUB_DISABLE_LINUX_PARTUUID variables.
>> - Remove logic that disabled setting the root cmdline param to either the
>>    filesystem UUID or partition UUID and instead use the existing variables
>>    to disable this.
>>
>>   docs/grub.texi            |  9 +++++++++
>>   util/grub-mkconfig.in     | 10 ++++++++++
>>   util/grub-mkconfig_lib.in |  4 ++--
>>   3 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/docs/grub.texi b/docs/grub.texi
>> index 5ac61c09d1b..2fe0e1b83bb 100644
>> --- a/docs/grub.texi
>> +++ b/docs/grub.texi
>> @@ -1441,6 +1441,15 @@ enable the use of partition UUIDs, set this option to @samp{false}.
>>   If this option is set to @samp{true}, disable the generation of recovery
>>   mode menu entries.
>>
>> +@item GRUB_DISABLE_UUID
>> +Normally, @command{grub-mkconfig} will generate menu entries that use
>> +universally-unique identifiers (UUIDs) to identify various filesystems to
>> +search for files.  This is usually more reliable, but in some cases it may
>> +not be appropriate.  To disable this use of UUIDs, set this option to
>> +@samp{true}. Setting this option to @samp{true}, will also set the options
>> +@samp{GRUB_DISABLE_LINUX_UUID} and @samp{GRUB_DISABLE_LINUX_PARTUUID} to
>> +@samp{true}, unless they have been explicilty set to @samp{false}.
>> +
>>   @item GRUB_VIDEO_BACKEND
>>   If graphical video support is required, either because the @samp{gfxterm}
>>   graphical terminal is in use or because @samp{GRUB_GFXPAYLOAD_LINUX} is set,
>> diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
>> index a6ce375ed18..2537dbca2f9 100644
>> --- a/util/grub-mkconfig.in
>> +++ b/util/grub-mkconfig.in
>> @@ -158,6 +158,15 @@ if test -f ${sysconfdir}/default/grub ; then
>>     . ${sysconfdir}/default/grub
>>   fi
>>
>> +if [ "x${GRUB_DISABLE_UUID}" = "xtrue" ]; then
>> +  if [ "x${GRUB_DISABLE_LINUX_UUID}" != "xfalse" ]; then
>> +    GRUB_DISABLE_LINUX_UUID="true"
>> +  fi
>> +  if [ "x${GRUB_DISABLE_LINUX_PARTUUID}" != "xfalse" ]; then
>> +    GRUB_DISABLE_LINUX_PARTUUID="true"
>> +  fi
>> +fi

I don't think this logic is exactly consistent with 
51be3372ec8ba07ef68a409956ea0eefa89fe7c5.  That commit assumes any value 
other than 'true' is false.  It also sets the default values for 
GRUB_DISABLE_LINUX_PARTUUID in 10_linux.in and 20_linux_xen.in.

Both minor issues, but I do wonder if they should be addressed.

Thanks,
Nicholas Vinson

>> +
>>   # XXX: should this be deprecated at some point?
>>   if [ "x${GRUB_TERMINAL}" != "x" ] ; then
>>     GRUB_TERMINAL_INPUT="${GRUB_TERMINAL}"
>> @@ -227,6 +236,7 @@ export GRUB_DEFAULT \
>>     GRUB_DISABLE_LINUX_UUID \
>>     GRUB_DISABLE_LINUX_PARTUUID \
>>     GRUB_DISABLE_RECOVERY \
>> +  GRUB_DISABLE_UUID \
>>     GRUB_VIDEO_BACKEND \
>>     GRUB_GFXMODE \
>>     GRUB_BACKGROUND \
>> diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
>> index 0f801cab3e4..3e12b2c0e6a 100644
>> --- a/util/grub-mkconfig_lib.in
>> +++ b/util/grub-mkconfig_lib.in
>> @@ -156,7 +156,7 @@ prepare_grub_to_access_device ()
>>     if [ "x$fs_hint" != x ]; then
>>       echo "set root='$fs_hint'"
>>     fi
>> -  if fs_uuid="`"${grub_probe}" --device $@ --target=fs_uuid 2> /dev/null`" ; then
>> +  if [ "x${GRUB_DISABLE_UUID}" != "xtrue" ] && fs_uuid="`"${grub_probe}" --device $@ --target=fs_uuid 2> /dev/null`" ; then
>>       hints="`"${grub_probe}" --device $@ --target=hints_string 2> /dev/null`" || hints=
>>       echo "if [ x\$feature_platform_search_hint = xy ]; then"
>>       echo "  search --no-floppy --fs-uuid --set=root ${hints} ${fs_uuid}"
>> @@ -173,7 +173,7 @@ grub_get_device_id ()
>>     IFS='
>>   '
>>     device="$1"
>> -  if fs_uuid="`"${grub_probe}" --device ${device} --target=fs_uuid 2> /dev/null`" ; then
>> +  if [ "x${GRUB_DISABLE_UUID}" != "xtrue" ] && fs_uuid="`"${grub_probe}" --device ${device} --target=fs_uuid 2> /dev/null`" ; then
>>       echo "$fs_uuid";
>>     else
>>       echo $device |sed 's, ,_,g'
>> --
>> 2.21.0

Re: [PATCH v2] Add GRUB_DISABLE_UUID

[Javier Martinez Canillas]

Hello Nicolas,

Thanks a lot for the feedback.

On 10/3/19 4:29 AM, Nicholas Vinson wrote:

[snip]

>>>
>>> +if [ "x${GRUB_DISABLE_UUID}" = "xtrue" ]; then
>>> +  if [ "x${GRUB_DISABLE_LINUX_UUID}" != "xfalse" ]; then
>>> +    GRUB_DISABLE_LINUX_UUID="true"
>>> +  fi
>>> +  if [ "x${GRUB_DISABLE_LINUX_PARTUUID}" != "xfalse" ]; then
>>> +    GRUB_DISABLE_LINUX_PARTUUID="true"
>>> +  fi
>>> +fi
> 
> I don't think this logic is exactly consistent with 
> 51be3372ec8ba07ef68a409956ea0eefa89fe7c5.  That commit assumes any value
> other than 'true' is false.  It also sets the default values for 

I'm not sure how we could make it more consistent with the mentioned commit
while only setting it to 'true' if the user haven't explicitly set the var
as Dan asked in the previous version of the patch.

> GRUB_DISABLE_LINUX_PARTUUID in 10_linux.in and 20_linux_xen.in.
>

This isn't an issue since that's only when GRUB_DISABLE_LINUX_PARTUUID
hasn't been set, but it will be set to 'true' if GRUB_DISABLE_UUID=true.

> Both minor issues, but I do wonder if they should be addressed.
>
> Thanks,
> Nicholas Vinson
> 

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

Re: [PATCH v2] Add GRUB_DISABLE_UUID

[Javier Martinez Canillas]

Hello Daniel and Nicolas,

On 10/4/19 12:57 PM, Javier Martinez Canillas wrote:
> Hello Nicolas,
> 
> Thanks a lot for the feedback.
> 
> On 10/3/19 4:29 AM, Nicholas Vinson wrote:
> 
> [snip]
> 
>>>>
>>>> +if [ "x${GRUB_DISABLE_UUID}" = "xtrue" ]; then
>>>> +  if [ "x${GRUB_DISABLE_LINUX_UUID}" != "xfalse" ]; then
>>>> +    GRUB_DISABLE_LINUX_UUID="true"
>>>> +  fi
>>>> +  if [ "x${GRUB_DISABLE_LINUX_PARTUUID}" != "xfalse" ]; then
>>>> +    GRUB_DISABLE_LINUX_PARTUUID="true"
>>>> +  fi
>>>> +fi
>>
>> I don't think this logic is exactly consistent with 
>> 51be3372ec8ba07ef68a409956ea0eefa89fe7c5.  That commit assumes any value
>> other than 'true' is false.  It also sets the default values for 
> 
> I'm not sure how we could make it more consistent with the mentioned commit
> while only setting it to 'true' if the user haven't explicitly set the var
> as Dan asked in the previous version of the patch.
>

Any comments on this?
 
>> GRUB_DISABLE_LINUX_PARTUUID in 10_linux.in and 20_linux_xen.in.
>>
> 
> This isn't an issue since that's only when GRUB_DISABLE_LINUX_PARTUUID
> hasn't been set, but it will be set to 'true' if GRUB_DISABLE_UUID=true.
> 
>> Both minor issues, but I do wonder if they should be addressed.
>>
>> Thanks,
>> Nicholas Vinson
>>
> 
> Best regards,
> 

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

Re: [PATCH v2] Add GRUB_DISABLE_UUID

[Daniel Kiper]

On Fri, Oct 04, 2019 at 12:57:44PM +0200, Javier Martinez Canillas wrote:
> Hello Nicolas,
>
> Thanks a lot for the feedback.
>
> On 10/3/19 4:29 AM, Nicholas Vinson wrote:
>
> [snip]
>
> >>>
> >>> +if [ "x${GRUB_DISABLE_UUID}" = "xtrue" ]; then
> >>> +  if [ "x${GRUB_DISABLE_LINUX_UUID}" != "xfalse" ]; then
> >>> +    GRUB_DISABLE_LINUX_UUID="true"
> >>> +  fi
> >>> +  if [ "x${GRUB_DISABLE_LINUX_PARTUUID}" != "xfalse" ]; then
> >>> +    GRUB_DISABLE_LINUX_PARTUUID="true"
> >>> +  fi
> >>> +fi
> >
> > I don't think this logic is exactly consistent with
> > 51be3372ec8ba07ef68a409956ea0eefa89fe7c5.  That commit assumes any value
> > other than 'true' is false.  It also sets the default values for
>
> I'm not sure how we could make it more consistent with the mentioned commit
> while only setting it to 'true' if the user haven't explicitly set the var
> as Dan asked in the previous version of the patch.

It seems to me that it can be easily fixed, e.g.:

if [ -z "${GRUB_DISABLE_LINUX_UUID}" ]; then
  GRUB_DISABLE_LINUX_UUID="true"
fi

Same for GRUB_DISABLE_LINUX_PARTUUID.

Does it make sense?

Daniel

Re: [PATCH v2] Add GRUB_DISABLE_UUID

[Javier Martinez Canillas]

Hello Daniel,

On 10/21/19 4:26 PM, Daniel Kiper wrote:
> On Fri, Oct 04, 2019 at 12:57:44PM +0200, Javier Martinez Canillas wrote:
>> Hello Nicolas,
>>
>> Thanks a lot for the feedback.
>>
>> On 10/3/19 4:29 AM, Nicholas Vinson wrote:
>>
>> [snip]
>>
>>>>>
>>>>> +if [ "x${GRUB_DISABLE_UUID}" = "xtrue" ]; then
>>>>> +  if [ "x${GRUB_DISABLE_LINUX_UUID}" != "xfalse" ]; then
>>>>> +    GRUB_DISABLE_LINUX_UUID="true"
>>>>> +  fi
>>>>> +  if [ "x${GRUB_DISABLE_LINUX_PARTUUID}" != "xfalse" ]; then
>>>>> +    GRUB_DISABLE_LINUX_PARTUUID="true"
>>>>> +  fi
>>>>> +fi
>>>
>>> I don't think this logic is exactly consistent with
>>> 51be3372ec8ba07ef68a409956ea0eefa89fe7c5.  That commit assumes any value
>>> other than 'true' is false.  It also sets the default values for
>>
>> I'm not sure how we could make it more consistent with the mentioned commit
>> while only setting it to 'true' if the user haven't explicitly set the var
>> as Dan asked in the previous version of the patch.
> 
> It seems to me that it can be easily fixed, e.g.:
> 
> if [ -z "${GRUB_DISABLE_LINUX_UUID}" ]; then
>   GRUB_DISABLE_LINUX_UUID="true"
> fi
> 
> Same for GRUB_DISABLE_LINUX_PARTUUID.
> 
> Does it make sense?
>

Right, that makes sense. Thanks.
 
> Daniel
> 

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