[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] search: new --efidisk-only option on EFI systems
|
From: |
Renaud Métrich |
|
Subject: |
Re: [PATCH] search: new --efidisk-only option on EFI systems |
|
Date: |
Mon, 7 Feb 2022 12:12:14 +0100 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.0 |
Please find inline the new patch integrating Glenn's comments (new
"flags" option instead of "no-floppy" / "efidisk-only").
When using 'search' on EFI systems, we sometimes want to exclude devices
that are not EFI disks (e.g. md, lvm).
This is typically used when wanting to chainload when having a software
raid (md) for EFI partition:
with no option, 'search --file /EFI/redhat/shimx64.efi' sets root envvar
to 'md/boot_efi' which cannot be used for chainloading since there is no
effective EFI device behind.
Signed-off-by: Renaud Métrich <rmetrich@redhat.com>
diff --git a/grub-core/commands/search.c b/grub-core/commands/search.c
index ed090b3af..57d26ced8 100644
--- a/grub-core/commands/search.c
+++ b/grub-core/commands/search.c
@@ -47,7 +47,7 @@ struct search_ctx
{
const char *key;
const char *var;
- int no_floppy;
+ enum search_flags flags;
char **hints;
unsigned nhints;
int count;
@@ -62,9 +62,28 @@ iterate_device (const char *name, void *data)
int found = 0;
/* Skip floppy drives when requested. */
- if (ctx->no_floppy &&
+ if (ctx->flags & SEARCH_FLAGS_NO_FLOPPY &&
name[0] == 'f' && name[1] == 'd' && name[2] >= '0' && name[2] <=
'9')
- return 1;
+ return 0;
+
+ /* Limit to EFI disks when requested. */
+ if (ctx->flags & SEARCH_FLAGS_EFIDISK_ONLY)
+ {
+ grub_device_t dev;
+ dev = grub_device_open (name);
+ if (! dev)
+ {
+ grub_errno = GRUB_ERR_NONE;
+ return 0;
+ }
+ if (! dev->disk || dev->disk->dev->id != GRUB_DISK_DEVICE_EFIDISK_ID)
+ {
+ grub_device_close (dev);
+ grub_errno = GRUB_ERR_NONE;
+ return 0;
+ }
+ grub_device_close (dev);
+ }
#ifdef DO_SEARCH_FS_UUID
#define compare_fn grub_strcasecmp
@@ -261,13 +280,13 @@ try (struct search_ctx *ctx)
}
void
-FUNC_NAME (const char *key, const char *var, int no_floppy,
+FUNC_NAME (const char *key, const char *var, enum search_flags flags,
char **hints, unsigned nhints)
{
struct search_ctx ctx = {
.key = key,
.var = var,
- .no_floppy = no_floppy,
+ .flags = flags,
.hints = hints,
.nhints = nhints,
.count = 0,
diff --git a/grub-core/commands/search_wrap.c
b/grub-core/commands/search_wrap.c
index 47fc8eb99..464e6ebb1 100644
--- a/grub-core/commands/search_wrap.c
+++ b/grub-core/commands/search_wrap.c
@@ -40,6 +40,7 @@ static const struct grub_arg_option options[] =
N_("Set a variable to the first device found."), N_("VARNAME"),
ARG_TYPE_STRING},
{"no-floppy", 'n', 0, N_("Do not probe any floppy drive."), 0, 0},
+ {"efidisk-only", 0, 0, N_("Only probe EFI disks."), 0, 0},
{"hint", 'h', GRUB_ARG_OPTION_REPEATABLE,
N_("First try the device HINT. If HINT ends in comma, "
"also try subpartitions"), N_("HINT"), ARG_TYPE_STRING},
@@ -73,6 +74,7 @@ enum options
SEARCH_FS_UUID,
SEARCH_SET,
SEARCH_NO_FLOPPY,
+ SEARCH_EFIDISK_ONLY,
SEARCH_HINT,
SEARCH_HINT_IEEE1275,
SEARCH_HINT_BIOS,
@@ -89,6 +91,7 @@ grub_cmd_search (grub_extcmd_context_t ctxt, int argc,
char **args)
const char *id = 0;
int i = 0, j = 0, nhints = 0;
char **hints = NULL;
+ enum search_flags flags;
if (state[SEARCH_HINT].set)
for (i = 0; state[SEARCH_HINT].args[i]; i++)
@@ -180,15 +183,18 @@ grub_cmd_search (grub_extcmd_context_t ctxt, int
argc, char **args)
goto out;
}
+ if (state[SEARCH_NO_FLOPPY].set)
+ flags |= SEARCH_FLAGS_NO_FLOPPY;
+
+ if (state[SEARCH_EFIDISK_ONLY].set)
+ flags |= SEARCH_FLAGS_EFIDISK_ONLY;
+
if (state[SEARCH_LABEL].set)
- grub_search_label (id, var, state[SEARCH_NO_FLOPPY].set,
- hints, nhints);
+ grub_search_label (id, var, flags, hints, nhints);
else if (state[SEARCH_FS_UUID].set)
- grub_search_fs_uuid (id, var, state[SEARCH_NO_FLOPPY].set,
- hints, nhints);
+ grub_search_fs_uuid (id, var, flags, hints, nhints);
else if (state[SEARCH_FILE].set)
- grub_search_fs_file (id, var, state[SEARCH_NO_FLOPPY].set,
- hints, nhints);
+ grub_search_fs_file (id, var, flags, hints, nhints);
else
grub_error (GRUB_ERR_INVALID_COMMAND, "unspecified search type");
diff --git a/include/grub/search.h b/include/grub/search.h
index d80347df3..4190aeb2c 100644
--- a/include/grub/search.h
+++ b/include/grub/search.h
@@ -19,11 +19,20 @@
#ifndef GRUB_SEARCH_HEADER
#define GRUB_SEARCH_HEADER 1
-void grub_search_fs_file (const char *key, const char *var, int no_floppy,
+enum search_flags
+ {
+ SEARCH_FLAGS_NO_FLOPPY = 1,
+ SEARCH_FLAGS_EFIDISK_ONLY = 2
+ };
+
+void grub_search_fs_file (const char *key, const char *var,
+ enum search_flags flags,
char **hints, unsigned nhints);
-void grub_search_fs_uuid (const char *key, const char *var, int no_floppy,
+void grub_search_fs_uuid (const char *key, const char *var,
+ enum search_flags flags,
char **hints, unsigned nhints);
-void grub_search_label (const char *key, const char *var, int no_floppy,
+void grub_search_label (const char *key, const char *var,
+ enum search_flags flags,
char **hints, unsigned nhints);
#endif
--
2.34.1
Le 2/7/22 à 10:27, Renaud Métrich a écrit :
See inline, sorry for the format.
Le 2/4/22 à 23:28, Glenn Washburn a écrit :
On Tue, 1 Feb 2022 11:36:01 +0100
Renaud Métrich <rmetrich@redhat.com> wrote:
When using 'search' on EFI systems, we sometimes want to exclude
devices
that are not EFI disks (e.g. md, lvm).
This is typically used when wanting to chainload when having a software
raid (md) for EFI partition:
with no option, 'search --file /EFI/redhat/shimx64.efi' sets root
envvar
to 'md/boot_efi' which cannot be used for chainloading since there
is no
effective EFI device behind.
Example of "grub.cfg" file used to chainload when system boots over the
network:
In the future, please submit patches inline and not attached. Otherwise
it makes it more difficult to respond to its contents. I've added the
patch inline in my response and quoted.
~~~
menuentry 'Chainload Grub2 EFI from ESP' --id local {
unset root
search --file --no-floppy --efidisk-only --set=root
/EFI/BOOT/BOOTX64.EFI
if [ -f ($root)/EFI/BOOT/grubx64.efi ]; then
chainloader ($root)/EFI/BOOT/grubx64.efi
elif [ -f ($root)/EFI/redhat/shimx64.efi ]; then
chainloader ($root)/EFI/redhat/shimx64.efi
elif [ -f ($root)/EFI/redhat/grubx64.efi ]; then
chainloader ($root)/EFI/redhat/grubx64.efi
fi
}
~~~
This patch has been tested on QEMU/KVM systems and VMWare VMs (at
hardware level 6.7 and 7.0u2).
Related Red Hat BZ (public):
https://bugzilla.redhat.com/show_bug.cgi?id=2048904
From 46a8693953333e08fd2df4f722483b8db0f7da62 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Renaud=20M=C3=A9trich?= <rmetrich@redhat.com>
Date: Tue, 1 Feb 2022 07:17:24 +0100
Subject: [PATCH] search: new --efidisk-only option on EFI systems
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When using 'search' on EFI systems, we sometimes want to exclude
devices that are not EFI disks (e.g. md, lvm).
This is typically used when wanting to chainload when having a
software raid (md) for EFI partition:
with no option, 'search --file /EFI/redhat/shimx64.efi' sets root
envvar to 'md/boot_efi' which cannot be used for chainloading since
there is no effective EFI device behind.
Signed-off-by: Renaud Métrich <rmetrich@redhat.com>
diff --git a/grub-core/commands/search.c b/grub-core/commands/search.c
index ed090b3af..5e1e88643 100644
--- a/grub-core/commands/search.c
+++ b/grub-core/commands/search.c
@@ -48,6 +48,9 @@ struct search_ctx
const char *key;
const char *var;
int no_floppy;
+#ifdef GRUB_MACHINE_EFI
+ int efidisk_only;
+#endif
I think it would be cleaner to have a "flags" member here where right
now there would only be SEARCH_FLAGS_NO_FLOPPY and
SEARCH_FLAGS_EFIDISK_ONLY. This way if in the future someone wants to
add another filter to search they just add a flag instead of updating
all the function signatures. And for this patch it will remove the need
for most of the #ifdefs.
OK, will change this to a new "flags" and make "no_floppy" be a flag.
char **hints;
unsigned nhints;
int count;
@@ -64,7 +67,28 @@ iterate_device (const char *name, void *data)
/* Skip floppy drives when requested. */
if (ctx->no_floppy &&
name[0] == 'f' && name[1] == 'd' && name[2] >= '0' && name[2]
<= '9')
- return 1;
+ return 0;
So this function now returns success if --no-floppy was requested and
we've encountered a floppy? Am I missing something? If this is
desirable, perhaps it should be documented.
Glenn
Actually I think this is a bug in the --no-floppy code: by returning
1, the iterate stops, which is probably not what is expected at all.
I hence fixed this as part of my code but it's not related indeed.
+
+#ifdef GRUB_MACHINE_EFI
+ /* Limit to EFI disks when requested. */
+ if (ctx->efidisk_only)
+ {
+ grub_device_t dev;
+ dev = grub_device_open (name);
+ if (! dev)
+ {
+ grub_errno = GRUB_ERR_NONE;
+ return 0;
+ }
+ if (! dev->disk || dev->disk->dev->id !=
GRUB_DISK_DEVICE_EFIDISK_ID)
+ {
+ grub_device_close (dev);
+ grub_errno = GRUB_ERR_NONE;
+ return 0;
+ }
+ grub_device_close (dev);
+ }
+#endif
#ifdef DO_SEARCH_FS_UUID
#define compare_fn grub_strcasecmp
@@ -262,12 +286,18 @@ try (struct search_ctx *ctx)
void
FUNC_NAME (const char *key, const char *var, int no_floppy,
+#ifdef GRUB_MACHINE_EFI
+ int efidisk_only,
+#endif
char **hints, unsigned nhints)
{
struct search_ctx ctx = {
.key = key,
.var = var,
.no_floppy = no_floppy,
+#ifdef GRUB_MACHINE_EFI
+ .efidisk_only = efidisk_only,
+#endif
.hints = hints,
.nhints = nhints,
.count = 0,
@@ -303,8 +333,12 @@ grub_cmd_do_search (grub_command_t cmd
__attribute__ ((unused)), int argc, if (argc == 0)
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument
expected"));
- FUNC_NAME (args[0], argc == 1 ? 0 : args[1], 0, (args + 2),
- argc > 2 ? argc - 2 : 0);
+ FUNC_NAME (args[0], argc == 1 ? 0 : args[1], 0,
+#ifdef GRUB_MACHINE_EFI
+ /* efidisk_only */
+ 0,
+#endif
+ (args + 2), argc > 2 ? argc - 2 : 0);
return grub_errno;
}
diff --git a/grub-core/commands/search_wrap.c
b/grub-core/commands/search_wrap.c index 47fc8eb99..aed75b236 100644
--- a/grub-core/commands/search_wrap.c
+++ b/grub-core/commands/search_wrap.c
@@ -40,6 +40,9 @@ static const struct grub_arg_option options[] =
N_("Set a variable to the first device found."), N_("VARNAME"),
ARG_TYPE_STRING},
{"no-floppy", 'n', 0, N_("Do not probe any floppy
drive."), 0, 0}, +#ifdef GRUB_MACHINE_EFI
+ {"efidisk-only", 0, 0, N_("Only probe EFI disks."), 0, 0},
+#endif
{"hint", 'h', GRUB_ARG_OPTION_REPEATABLE,
N_("First try the device HINT. If HINT ends in comma, "
"also try subpartitions"), N_("HINT"), ARG_TYPE_STRING},
@@ -73,6 +76,9 @@ enum options
SEARCH_FS_UUID,
SEARCH_SET,
SEARCH_NO_FLOPPY,
+#ifdef GRUB_MACHINE_EFI
+ SEARCH_EFIDISK_ONLY,
+#endif
SEARCH_HINT,
SEARCH_HINT_IEEE1275,
SEARCH_HINT_BIOS,
@@ -182,12 +188,21 @@ grub_cmd_search (grub_extcmd_context_t ctxt,
int argc, char **args)
if (state[SEARCH_LABEL].set)
grub_search_label (id, var, state[SEARCH_NO_FLOPPY].set,
+#ifdef GRUB_MACHINE_EFI
+ state[SEARCH_EFIDISK_ONLY].set,
+#endif
hints, nhints);
else if (state[SEARCH_FS_UUID].set)
grub_search_fs_uuid (id, var, state[SEARCH_NO_FLOPPY].set,
+#ifdef GRUB_MACHINE_EFI
+ state[SEARCH_EFIDISK_ONLY].set,
+#endif
hints, nhints);
else if (state[SEARCH_FILE].set)
grub_search_fs_file (id, var, state[SEARCH_NO_FLOPPY].set,
+#ifdef GRUB_MACHINE_EFI
+ state[SEARCH_EFIDISK_ONLY].set,
+#endif
hints, nhints);
else
grub_error (GRUB_ERR_INVALID_COMMAND, "unspecified search type");
diff --git a/include/grub/search.h b/include/grub/search.h
index d80347df3..fc058add2 100644
--- a/include/grub/search.h
+++ b/include/grub/search.h
@@ -20,10 +20,19 @@
#define GRUB_SEARCH_HEADER 1
void grub_search_fs_file (const char *key, const char *var, int
no_floppy, +#ifdef GRUB_MACHINE_EFI
+ int efidisk_only,
+#endif
char **hints, unsigned nhints);
void grub_search_fs_uuid (const char *key, const char *var, int
no_floppy, +#ifdef GRUB_MACHINE_EFI
+ int efidisk_only,
+#endif
char **hints, unsigned nhints);
void grub_search_label (const char *key, const char *var, int
no_floppy, +#ifdef GRUB_MACHINE_EFI
+ int efidisk_only,
+#endif
char **hints, unsigned nhints);
#endif
--
2.34.1
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
OpenPGP_signature
Description: OpenPGP digital signature