grub-devel
[Top][All Lists]
Advanced

[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: Tue, 8 Feb 2022 08:40:39 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.0

So Glenn, I'm new to Grub developer, in the past I was relying on Javier Martinez Canillas ...

Just sent v3.

Le 2/7/22 à 22:32, Glenn Washburn a écrit :
On Mon, 7 Feb 2022 12:12:14 +0100
Renaud Métrich <rmetrich@redhat.com> wrote:

Please find inline the new patch integrating Glenn's comments (new
"flags" option instead of "no-floppy" / "efidisk-only").
Thanks for making this inline, but its still not in a great format for
maintainers. Please use "git format-patch" and "git send-email" for the
next iteration. Its pretty easy to setup.

Also, please use the -v option to format-patch to increment the version
of the patch we're on (the next should be v3). Also create a new thread
for each new patch series sent to this list.

I suspect tht Daniel will also request that it be documented in the
commit message that no-floppy handling was changed. Perhaps a line like
"Refactor handling of --no-floppy option as well." Daniel might have
other ideas though.

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;
Ok, so this fixes a bug. At a minimum, that needs to be documented in
the commit message. I think there should be a separate patch with this
bug fix, since its unrelated. Daniel may accept it all as one patch.
I'll let him chime in.

+
+  /* 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;
+    }
This indent formatting looks off. I think some tabs are getting
converted to 4 spaces.

+      if (! dev->disk || dev->disk->dev->id != GRUB_DISK_DEVICE_EFIDISK_ID)
+    {
+      grub_device_close (dev);
+      grub_errno = GRUB_ERR_NONE;
+      return 0;
+    }
Ditto.

+      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
This is weird here. It looks like the #endif has a space before it
on the line (but it doesn't in master). Perhaps an artifact of how you
did the inline? Using git to format and send the patch should fix these
issues.

Glenn

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]