Re: [PATCH] grub2: setup root in grub-setup and grub-install

[Daniel Kiper]

On Mon, May 27, 2019 at 10:07:15AM +0200, Michael Grzeschik wrote:
> With this feature it is now possible to set the root
> device in userspace preperation like grub-mkimage.

Why this patch is needed? I would like to see an explanation here.

> Signed-off-by: Michael Grzeschik <address@hidden>
> ---
>  include/grub/util/install.h |  4 ++--
>  util/grub-install.c         |  4 ++--
>  util/grub-setup.c           | 12 +++++++++++-
>  util/setup.c                | 11 +++++++----
>  4 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/include/grub/util/install.h b/include/grub/util/install.h
> index 2631b1074..ff02c365c 100644
> --- a/include/grub/util/install.h
> +++ b/include/grub/util/install.h
> @@ -191,13 +191,13 @@ grub_install_get_image_target (const char *arg);
>  void
>  grub_util_bios_setup (const char *dir,
>                     const char *boot_file, const char *core_file,
> -                   const char *dest, int force,
> +                   const char *root, const char *dest, int force,
>                     int fs_probe, int allow_floppy,
>                     int add_rs_codes);
>  void
>  grub_util_sparc_setup (const char *dir,
>                      const char *boot_file, const char *core_file,
> -                    const char *dest, int force,
> +                    const char *root, const char *dest, int force,
>                      int fs_probe, int allow_floppy,
>                      int add_rs_codes);
>
> diff --git a/util/grub-install.c b/util/grub-install.c
> index 8a55ad4b8..f90cf2cf7 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -1712,7 +1712,7 @@ main (int argc, char *argv[])
>       /*  Now perform the installation.  */
>       if (install_bootsector)
>         grub_util_bios_setup (platdir, "boot.img", "core.img",
> -                             install_drive, force,
> +                             NULL, install_drive, force,
>                               fs_probe, allow_floppy, add_rs_codes);
>       break;
>        }
> @@ -1738,7 +1738,7 @@ main (int argc, char *argv[])
>       /*  Now perform the installation.  */
>       if (install_bootsector)
>         grub_util_sparc_setup (platdir, "boot.img", "core.img",
> -                              install_drive, force,
> +                              NULL, install_device, force,
>                                fs_probe, allow_floppy,
>                                0 /* unused */ );
>       break;
> diff --git a/util/grub-setup.c b/util/grub-setup.c
> index 42b98ad3c..ae1f98f75 100644
> --- a/util/grub-setup.c
> +++ b/util/grub-setup.c
> @@ -87,6 +87,8 @@ static struct argp_option options[] = {
>     N_("install even if problems are detected"), 0},
>    {"skip-fs-probe",'s',0,      0,
>     N_("do not probe for filesystems in DEVICE"), 0},
> +  {"root-device", 'r', N_("DEVICE"), 0,
> +   N_("use DEVICE as the root device"), 0},
>    {"verbose",     'v', 0,      0, N_("print verbose messages."), 0},
>    {"allow-floppy", 'a', 0,      0,
>     /* TRANSLATORS: The potential breakage isn't limited to floppies but it's
> @@ -130,6 +132,7 @@ struct arguments
>    char *core_file;
>    char *dir;
>    char *dev_map;
> +  char *root_dev;
>    int  force;
>    int  fs_probe;
>    int allow_floppy;
> @@ -178,6 +181,13 @@ argp_parser (int key, char *arg, struct argp_state 
> *state)
>          arguments->dev_map = xstrdup (arg);
>          break;
>
> +      case 'r':
> +        if (arguments->root_dev)
> +          free (arguments->root_dev);
> +
> +        arguments->root_dev = xstrdup (arg);
> +        break;
> +
>        case 'f':
>          arguments->force = 1;
>          break;
> @@ -313,7 +323,7 @@ main (int argc, char *argv[])
>    GRUB_SETUP_FUNC (arguments.dir ? : DEFAULT_DIRECTORY,
>                  arguments.boot_file ? : DEFAULT_BOOT_FILE,
>                  arguments.core_file ? : DEFAULT_CORE_FILE,
> -                dest_dev, arguments.force,
> +                arguments.root_dev, dest_dev, arguments.force,
>                  arguments.fs_probe, arguments.allow_floppy,
>                  arguments.add_rs_codes);
>
> diff --git a/util/setup.c b/util/setup.c
> index 6f88f3cc4..9bef0a8a1 100644
> --- a/util/setup.c
> +++ b/util/setup.c
> @@ -252,13 +252,12 @@ identify_partmap (grub_disk_t disk __attribute__ 
> ((unused)),
>  void
>  SETUP (const char *dir,
>         const char *boot_file, const char *core_file,
> -       const char *dest, int force,
> +       const char *root, const char *dest, int force,
>         int fs_probe, int allow_floppy,
>         int add_rs_codes __attribute__ ((unused))) /* unused on sparc64 */
>  {
>    char *core_path;
>    char *boot_img, *core_img, *boot_path;
> -  char *root = 0;
>    size_t boot_size, core_size;
>    grub_uint16_t core_sectors;
>    grub_device_t root_dev = 0, dest_dev, core_dev;
> @@ -307,7 +306,10 @@ SETUP (const char *dir,
>
>    core_dev = dest_dev;
>
> -  {
> +  if (root)
> +    root_dev = grub_device_open(root);
> +
> +  if (!root_dev) {

Incorrect coding style...

>      char **root_devices = grub_guess_root_devices (dir);
>      char **cur;
>      int found = 0;
> @@ -320,6 +322,8 @@ SETUP (const char *dir,
>       char *drive;
>       grub_device_t try_dev;
>
> +     if (root_dev)
> +       break;

Hmmm... Why is this change needed?

>       drive = grub_util_get_grub_dev (*cur);
>       if (!drive)
>         continue;
> @@ -334,7 +338,6 @@ SETUP (const char *dir,
>         {
>           if (root_dev)
>             grub_device_close (root_dev);
> -         free (root);

Why?

Daniel