Re: [PATCH] grub2: add support to search for partuuid

2019-05-28 Thread Nick Vinson


On 5/27/19 1:07 AM, Michael Grzeschik wrote:
> With this feature the grub-shell is able to search for partuuid strings.
> This reduces the complexity to find the correct boot partition on
> systems with unspecified device connectivity.
> 
> Signed-off-by: Michael Grzeschik 
> ---
>  grub-core/Makefile.core.def  |  5 +
>  grub-core/commands/search.c  | 25 -
>  grub-core/commands/search_partuuid.c |  5 +
>  grub-core/commands/search_wrap.c |  8 +++-
>  grub-core/partmap/msdos.c|  9 +
>  include/grub/disk.h  |  2 ++
>  6 files changed, 52 insertions(+), 2 deletions(-)
>  create mode 100644 grub-core/commands/search_partuuid.c
> 
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 474a63e68..208feb934 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1070,6 +1070,11 @@ module = {
>common = commands/search_file.c;
>  };
>  
> +module = {
> +  name = search_partuuid;
> +  common = commands/search_partuuid.c;
> +};
> +
>  module = {
>name = search_fs_uuid;
>common = commands/search_uuid.c;
> diff --git a/grub-core/commands/search.c b/grub-core/commands/search.c
> index ed090b3af..3ac93943b 100644
> --- a/grub-core/commands/search.c
> +++ b/grub-core/commands/search.c
> @@ -72,7 +72,26 @@ iterate_device (const char *name, void *data)
>  #define compare_fn grub_strcmp
>  #endif
>  
> -#ifdef DO_SEARCH_FILE
> +#ifdef (DO_SEARCH_PARTUUID)

Typo here.  DO_SEARCH_PARTUUID should not be wrapped with parentheses.

> +{
> +  grub_disk_t disk;
> +  char *part_uuid;
> +  char *uuid;
> +
> +  disk = grub_disk_open (name);
> +  if (disk && disk->partition)
> + {
> +  part_uuid = grub_xasprintf ("%08x-%02x", disk->nt_disk_sig,
> +  disk->partition->index + 1);
> +  if (compare_fn (part_uuid, ctx->key) == 0)
> + found = 1;
> +  if (part_uuid)
> +grub_free (part_uuid);
> +}
> +  if (disk)
> +grub_disk_close (disk);
> +}

Did you intend to omit GPT support for this command?  Asking because it
would be a bit weird in my opinion to add support for PARTUUIDs but omit
the most commonly used partition scheme that actually supports them.

Thanks,
Nicholas Vinson

> +#elif defined (DO_SEARCH_FILE)
>  {
>char *buf;
>grub_file_t file;
> @@ -313,6 +332,8 @@ static grub_command_t cmd;
>  
>  #ifdef DO_SEARCH_FILE
>  GRUB_MOD_INIT(search_fs_file)
> +#elif defined (DO_SEARCH_PARTUUID)
> +GRUB_MOD_INIT(search_partuuid)
>  #elif defined (DO_SEARCH_FS_UUID)
>  GRUB_MOD_INIT(search_fs_uuid)
>  #else
> @@ -327,6 +348,8 @@ GRUB_MOD_INIT(search_label)
>  
>  #ifdef DO_SEARCH_FILE
>  GRUB_MOD_FINI(search_fs_file)
> +#elif defined (DO_SEARCH_PARTUUID)
> +GRUB_MOD_FINI(search_partuuid)
>  #elif defined (DO_SEARCH_FS_UUID)
>  GRUB_MOD_FINI(search_fs_uuid)
>  #else
> diff --git a/grub-core/commands/search_partuuid.c 
> b/grub-core/commands/search_partuuid.c
> new file mode 100644
> index 0..e4aa20b5f
> --- /dev/null
> +++ b/grub-core/commands/search_partuuid.c
> @@ -0,0 +1,5 @@
> +#define DO_SEARCH_PARTUUID 1
> +#define FUNC_NAME grub_search_partuuid
> +#define COMMAND_NAME "search.partuuid"
> +#define HELP_MESSAGE N_("Search devices by PARTUUID. If VARIABLE is 
> specified, the first device found is set to a variable.")
> +#include "search.c"
> diff --git a/grub-core/commands/search_wrap.c 
> b/grub-core/commands/search_wrap.c
> index d7fd26b94..ba36dbac2 100644
> --- a/grub-core/commands/search_wrap.c
> +++ b/grub-core/commands/search_wrap.c
> @@ -36,6 +36,8 @@ static const struct grub_arg_option options[] =
>   0, 0},
>  {"fs-uuid",  'u', 0, N_("Search devices by a filesystem 
> UUID."),
>   0, 0},
> +{"partuuid", 'p', 0, N_("Search devices by a PARTUUID."),
> + 0, 0},
>  {"set",  's', GRUB_ARG_OPTION_OPTIONAL,
>   N_("Set a variable to the first device found."), N_("VARNAME"),
>   ARG_TYPE_STRING},
> @@ -71,6 +73,7 @@ enum options
>  SEARCH_FILE,
>  SEARCH_LABEL,
>  SEARCH_FS_UUID,
> +SEARCH_PARTUUID,
>  SEARCH_SET,
>  SEARCH_NO_FLOPPY,
>  SEARCH_HINT,
> @@ -186,6 +189,9 @@ grub_cmd_search (grub_extcmd_context_t ctxt, int argc, 
> char **args)
>else if (state[SEARCH_FS_UUID].set)
>  grub_search_fs_uuid (id, var, state[SEARCH_NO_FLOPPY].set,
>hints, nhints);
> +  else if (state[SEARCH_PARTUUID].set)
> +grub_search_partuuid (id, var, state[SEARCH_NO_FLOPPY].set,
> +  hints, nhints);
>else if (state[SEARCH_FILE].set)
>  grub_search_fs_file (id, var, state[SEARCH_NO_FLOPPY].set, 
>hints, nhints);
> @@ -207,7 +213,7 @@ GRUB_MOD_INIT(search)
> N_("[-f|-l|-u|-s|-n] [--hint HINT [--hint HINT] ...]"
>" 

Re: [PATCH] grub2: add support to search for partuuid

2019-05-28 Thread Daniel Kiper
CC-ing Vladimir.

On Mon, May 27, 2019 at 10:07:39AM +0200, Michael Grzeschik wrote:
> With this feature the grub-shell is able to search for partuuid strings.
> This reduces the complexity to find the correct boot partition on
> systems with unspecified device connectivity.

What is an usage case for this feature?

> Signed-off-by: Michael Grzeschik 
> ---
>  grub-core/Makefile.core.def  |  5 +
>  grub-core/commands/search.c  | 25 -
>  grub-core/commands/search_partuuid.c |  5 +
>  grub-core/commands/search_wrap.c |  8 +++-
>  grub-core/partmap/msdos.c|  9 +
>  include/grub/disk.h  |  2 ++
>  6 files changed, 52 insertions(+), 2 deletions(-)
>  create mode 100644 grub-core/commands/search_partuuid.c
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 474a63e68..208feb934 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1070,6 +1070,11 @@ module = {
>common = commands/search_file.c;
>  };
>
> +module = {
> +  name = search_partuuid;
> +  common = commands/search_partuuid.c;
> +};
> +
>  module = {
>name = search_fs_uuid;
>common = commands/search_uuid.c;
> diff --git a/grub-core/commands/search.c b/grub-core/commands/search.c
> index ed090b3af..3ac93943b 100644
> --- a/grub-core/commands/search.c
> +++ b/grub-core/commands/search.c
> @@ -72,7 +72,26 @@ iterate_device (const char *name, void *data)
>  #define compare_fn grub_strcmp
>  #endif

Should not you "#define compare_fn grub_strcasecmp" a few lines above?

Otherwise LGTM.

Daniel

> -#ifdef DO_SEARCH_FILE
> +#ifdef (DO_SEARCH_PARTUUID)
> +{
> +  grub_disk_t disk;
> +  char *part_uuid;
> +  char *uuid;
> +
> +  disk = grub_disk_open (name);
> +  if (disk && disk->partition)
> + {
> +  part_uuid = grub_xasprintf ("%08x-%02x", disk->nt_disk_sig,
> +  disk->partition->index + 1);
> +  if (compare_fn (part_uuid, ctx->key) == 0)
> + found = 1;
> +  if (part_uuid)
> +grub_free (part_uuid);
> +}
> +  if (disk)
> +grub_disk_close (disk);
> +}
> +#elif defined (DO_SEARCH_FILE)
>  {
>char *buf;
>grub_file_t file;
> @@ -313,6 +332,8 @@ static grub_command_t cmd;
>
>  #ifdef DO_SEARCH_FILE
>  GRUB_MOD_INIT(search_fs_file)
> +#elif defined (DO_SEARCH_PARTUUID)
> +GRUB_MOD_INIT(search_partuuid)
>  #elif defined (DO_SEARCH_FS_UUID)
>  GRUB_MOD_INIT(search_fs_uuid)
>  #else
> @@ -327,6 +348,8 @@ GRUB_MOD_INIT(search_label)
>
>  #ifdef DO_SEARCH_FILE
>  GRUB_MOD_FINI(search_fs_file)
> +#elif defined (DO_SEARCH_PARTUUID)
> +GRUB_MOD_FINI(search_partuuid)
>  #elif defined (DO_SEARCH_FS_UUID)
>  GRUB_MOD_FINI(search_fs_uuid)
>  #else
> diff --git a/grub-core/commands/search_partuuid.c 
> b/grub-core/commands/search_partuuid.c
> new file mode 100644
> index 0..e4aa20b5f
> --- /dev/null
> +++ b/grub-core/commands/search_partuuid.c
> @@ -0,0 +1,5 @@
> +#define DO_SEARCH_PARTUUID 1
> +#define FUNC_NAME grub_search_partuuid
> +#define COMMAND_NAME "search.partuuid"
> +#define HELP_MESSAGE N_("Search devices by PARTUUID. If VARIABLE is 
> specified, the first device found is set to a variable.")
> +#include "search.c"
> diff --git a/grub-core/commands/search_wrap.c 
> b/grub-core/commands/search_wrap.c
> index d7fd26b94..ba36dbac2 100644
> --- a/grub-core/commands/search_wrap.c
> +++ b/grub-core/commands/search_wrap.c
> @@ -36,6 +36,8 @@ static const struct grub_arg_option options[] =
>   0, 0},
>  {"fs-uuid",  'u', 0, N_("Search devices by a filesystem 
> UUID."),
>   0, 0},
> +{"partuuid", 'p', 0, N_("Search devices by a PARTUUID."),
> + 0, 0},
>  {"set",  's', GRUB_ARG_OPTION_OPTIONAL,
>   N_("Set a variable to the first device found."), N_("VARNAME"),
>   ARG_TYPE_STRING},
> @@ -71,6 +73,7 @@ enum options
>  SEARCH_FILE,
>  SEARCH_LABEL,
>  SEARCH_FS_UUID,
> +SEARCH_PARTUUID,
>  SEARCH_SET,
>  SEARCH_NO_FLOPPY,
>  SEARCH_HINT,
> @@ -186,6 +189,9 @@ grub_cmd_search (grub_extcmd_context_t ctxt, int argc, 
> char **args)
>else if (state[SEARCH_FS_UUID].set)
>  grub_search_fs_uuid (id, var, state[SEARCH_NO_FLOPPY].set,
>hints, nhints);
> +  else if (state[SEARCH_PARTUUID].set)
> +grub_search_partuuid (id, var, state[SEARCH_NO_FLOPPY].set,
> +  hints, nhints);
>else if (state[SEARCH_FILE].set)
>  grub_search_fs_file (id, var, state[SEARCH_NO_FLOPPY].set,
>hints, nhints);
> @@ -207,7 +213,7 @@ GRUB_MOD_INIT(search)
> N_("[-f|-l|-u|-s|-n] [--hint HINT [--hint HINT] ...]"
>" NAME"),
> N_("Search devices by file, filesystem label"
> -  " or filesystem UUID."
> + 

[PATCH] grub2: add support to search for partuuid

2019-05-27 Thread Michael Grzeschik
With this feature the grub-shell is able to search for partuuid strings.
This reduces the complexity to find the correct boot partition on
systems with unspecified device connectivity.

Signed-off-by: Michael Grzeschik 
---
 grub-core/Makefile.core.def  |  5 +
 grub-core/commands/search.c  | 25 -
 grub-core/commands/search_partuuid.c |  5 +
 grub-core/commands/search_wrap.c |  8 +++-
 grub-core/partmap/msdos.c|  9 +
 include/grub/disk.h  |  2 ++
 6 files changed, 52 insertions(+), 2 deletions(-)
 create mode 100644 grub-core/commands/search_partuuid.c

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 474a63e68..208feb934 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -1070,6 +1070,11 @@ module = {
   common = commands/search_file.c;
 };
 
+module = {
+  name = search_partuuid;
+  common = commands/search_partuuid.c;
+};
+
 module = {
   name = search_fs_uuid;
   common = commands/search_uuid.c;
diff --git a/grub-core/commands/search.c b/grub-core/commands/search.c
index ed090b3af..3ac93943b 100644
--- a/grub-core/commands/search.c
+++ b/grub-core/commands/search.c
@@ -72,7 +72,26 @@ iterate_device (const char *name, void *data)
 #define compare_fn grub_strcmp
 #endif
 
-#ifdef DO_SEARCH_FILE
+#ifdef (DO_SEARCH_PARTUUID)
+{
+  grub_disk_t disk;
+  char *part_uuid;
+  char *uuid;
+
+  disk = grub_disk_open (name);
+  if (disk && disk->partition)
+   {
+  part_uuid = grub_xasprintf ("%08x-%02x", disk->nt_disk_sig,
+  disk->partition->index + 1);
+  if (compare_fn (part_uuid, ctx->key) == 0)
+   found = 1;
+  if (part_uuid)
+grub_free (part_uuid);
+}
+  if (disk)
+grub_disk_close (disk);
+}
+#elif defined (DO_SEARCH_FILE)
 {
   char *buf;
   grub_file_t file;
@@ -313,6 +332,8 @@ static grub_command_t cmd;
 
 #ifdef DO_SEARCH_FILE
 GRUB_MOD_INIT(search_fs_file)
+#elif defined (DO_SEARCH_PARTUUID)
+GRUB_MOD_INIT(search_partuuid)
 #elif defined (DO_SEARCH_FS_UUID)
 GRUB_MOD_INIT(search_fs_uuid)
 #else
@@ -327,6 +348,8 @@ GRUB_MOD_INIT(search_label)
 
 #ifdef DO_SEARCH_FILE
 GRUB_MOD_FINI(search_fs_file)
+#elif defined (DO_SEARCH_PARTUUID)
+GRUB_MOD_FINI(search_partuuid)
 #elif defined (DO_SEARCH_FS_UUID)
 GRUB_MOD_FINI(search_fs_uuid)
 #else
diff --git a/grub-core/commands/search_partuuid.c 
b/grub-core/commands/search_partuuid.c
new file mode 100644
index 0..e4aa20b5f
--- /dev/null
+++ b/grub-core/commands/search_partuuid.c
@@ -0,0 +1,5 @@
+#define DO_SEARCH_PARTUUID 1
+#define FUNC_NAME grub_search_partuuid
+#define COMMAND_NAME "search.partuuid"
+#define HELP_MESSAGE N_("Search devices by PARTUUID. If VARIABLE is specified, 
the first device found is set to a variable.")
+#include "search.c"
diff --git a/grub-core/commands/search_wrap.c b/grub-core/commands/search_wrap.c
index d7fd26b94..ba36dbac2 100644
--- a/grub-core/commands/search_wrap.c
+++ b/grub-core/commands/search_wrap.c
@@ -36,6 +36,8 @@ static const struct grub_arg_option options[] =
  0, 0},
 {"fs-uuid",'u', 0, N_("Search devices by a filesystem 
UUID."),
  0, 0},
+{"partuuid",   'p', 0, N_("Search devices by a PARTUUID."),
+ 0, 0},
 {"set",'s', GRUB_ARG_OPTION_OPTIONAL,
  N_("Set a variable to the first device found."), N_("VARNAME"),
  ARG_TYPE_STRING},
@@ -71,6 +73,7 @@ enum options
 SEARCH_FILE,
 SEARCH_LABEL,
 SEARCH_FS_UUID,
+SEARCH_PARTUUID,
 SEARCH_SET,
 SEARCH_NO_FLOPPY,
 SEARCH_HINT,
@@ -186,6 +189,9 @@ grub_cmd_search (grub_extcmd_context_t ctxt, int argc, char 
**args)
   else if (state[SEARCH_FS_UUID].set)
 grub_search_fs_uuid (id, var, state[SEARCH_NO_FLOPPY].set,
 hints, nhints);
+  else if (state[SEARCH_PARTUUID].set)
+grub_search_partuuid (id, var, state[SEARCH_NO_FLOPPY].set,
+hints, nhints);
   else if (state[SEARCH_FILE].set)
 grub_search_fs_file (id, var, state[SEARCH_NO_FLOPPY].set, 
 hints, nhints);
@@ -207,7 +213,7 @@ GRUB_MOD_INIT(search)
  N_("[-f|-l|-u|-s|-n] [--hint HINT [--hint HINT] ...]"
 " NAME"),
  N_("Search devices by file, filesystem label"
-" or filesystem UUID."
+" PARTUUID or filesystem UUID."
 " If --set is specified, the first device found is"
 " set to a variable. If no variable name is"
 " specified, `root' is used."),
diff --git a/grub-core/partmap/msdos.c b/grub-core/partmap/msdos.c
index 7b8e45076..8a4af5931 100644
--- a/grub-core/partmap/msdos.c
+++ b/grub-core/partmap/msdos.c
@@ -19,6 +19,7 @@
 
 #include