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