Re: [PATCH] util/grub.d/30_os-prober.in: Fix GRUB_OS_PROBER_SKIP_LIST for non-EFI

[Glenn Washburn]

On Fri, 19 Jan 2024 11:28:55 +0100
Pascal Hambourg <pascal@plouf.fr.eu.org> wrote:

> GRUB documentation states:
> 
> 'GRUB_OS_PROBER_SKIP_LIST'
>   List of space-separated FS UUIDs of filesystems to be ignored from
>   os-prober output. For efi chainloaders it's <UUID>@<EFI FILE>
> 
> But the actual behaviour does not match this description. Setting
> 
>   GRUB_OS_PROBER_SKIP_LIST="<UUID>"
> 
> does not ignore non-EFI boot loaders detected on filesystem <UUID>.
> In order to skip non-EFI boot loaders, you must set
> 
>   GRUB_OS_PROBER_SKIP_LIST="<UUID>@<DEVICE>"
> 
> which is both absurd (UUID and device are redundant) and wrong
> (device names such as /dev/sd* may not be persistent across boots).
> 
> This patch fixes the detection of "@<EFI FILE>" in the device string
> reported by os-prober.
> 
> Fixes: 55e706c9 (Add GRUB_OS_PROBER_SKIP_LIST to selectively skipping systems)
> 
> Note: the UUID matching regex uses word boundaries '\b' but '@' is
> not a word character. As a consequence, setting
> 
>   GRUB_OS_PROBER_SKIP_LIST="<UUID>@<ANYTHING>"
> 
> will ignore non-EFI boot loaders detected on filesystem <UUID> even
> though the goal is to ignore a given EFI boot loader only.
> However I think this is desirable to preserve the behaviour of
> existing setups which use GRUB_OS_PROBER_SKIP_LIST="<UUID>@<DEVICE>".

It would be good to have this in a comment in an appropriate place in
the code. I know of no complaints about this, but I think it would be
good to work on transitioning to what should be done. Perhaps the
correct behavior should be available with a envvar and wait a full
release to to enable by default giving people time to transition.

Glenn

> 
> Signed-off-by: Pascal Hambourg <pascal@plouf.fr.eu.org>
> ---
>  util/grub.d/30_os-prober.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util/grub.d/30_os-prober.in b/util/grub.d/30_os-prober.in
> index e9e217208..a24c01334 100644
> --- a/util/grub.d/30_os-prober.in
> +++ b/util/grub.d/30_os-prober.in
> @@ -125,7 +125,7 @@ for OS in ${OSPROBED} ; do
>    if UUID="`${grub_probe} --target=fs_uuid --device ${DEVICE%@*}`"; then
>      EXPUUID="$UUID"
>  
> -    if [ x"${DEVICE#*@}" != x ] ; then
> +    if [ x"${DEVICE%@*}" != x"${DEVICE}" ] ; then
>        EXPUUID="${EXPUUID}@${DEVICE#*@}"
>      fi
>