Re: [RFC] grub-install: allow none or multiple install devices on PC BIOS

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 08.05.2015 20:53, Andrei Borzenkov wrote:
> There are two main applications.
> 
> 1. Omit install device to create generic image intended for chainloading
> from other master loader. Such image can be put on any device (or file
> system) and will still be able to find its $root. Currently even with
> --no-bootsector grub-install optimizes image by skipping UUID search if
> possible.
> 
> 2. Redundant installation on multi-device filesystem, RAID or similar.
> This allows both optimizing image w.r.t. to using --prefix vs. load.cfg
> as well as creating image just once.
> 
> Patch allows transparently use none or multiple installation devices,
> similar to
> 
> grub_devices="/dev/sda /dev/sda1 /dev/sdb"
> grub-install $grub_devices
> 
> where grub_devices can be empty and still do the right thing.
> 
> This is work in progress, although it is functionally complete and just
> needs some cleanups.
> 
> Comments?
I like the idea and it was on my "nice to have" list for some time. Do
you want to clean it up first or should I review this version?
> 
> ---
>  grub-core/kern/disk.c |   5 +-
>  include/grub/disk.h   |   2 +
>  util/grub-install.c   | 217 
> +++++++++++++++++++++++++++++++-------------------
>  3 files changed, 143 insertions(+), 81 deletions(-)
> 
> diff --git a/grub-core/kern/disk.c b/grub-core/kern/disk.c
> index 789f8c0..56f16b4 100644
> --- a/grub-core/kern/disk.c
> +++ b/grub-core/kern/disk.c
> @@ -168,7 +168,10 @@ grub_disk_dev_unregister (grub_disk_dev_t dev)
>  
>  /* Return the location of the first ',', if any, which is not
>     escaped by a '\'.  */
> -static const char *
> +#if !defined (GRUB_UTIL)
> +static
> +#endif
> +const char *
>  find_part_sep (const char *name)
>  {
>    const char *p = name;
> diff --git a/include/grub/disk.h b/include/grub/disk.h
> index b385af8..b2081eb 100644
> --- a/include/grub/disk.h
> +++ b/include/grub/disk.h
> @@ -256,6 +256,8 @@ void grub_ldm_fini (void);
>  void grub_mdraid09_fini (void);
>  void grub_mdraid1x_fini (void);
>  void grub_diskfilter_fini (void);
> +extern const char *find_part_sep (const char *);
> +
>  #endif
>  
>  #endif /* ! GRUB_DISK_HEADER */
> diff --git a/util/grub-install.c b/util/grub-install.c
> index 7b394c9..bb6532c 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -57,7 +57,10 @@ static char *target;
>  static int removable = 0;
>  static int recheck = 0;
>  static int update_nvram = 1;
> -static char *install_device = NULL;
> +static char **install_devices = NULL;
> +static char **install_drives = NULL;
> +static int n_install_devices;
> +static int n_allocated_devices;
>  static char *debug_image = NULL;
>  static char *rootdir = NULL;
>  static char *bootdir = NULL;
> @@ -234,9 +237,12 @@ argp_parser (int key, char *arg, struct argp_state 
> *state)
>        return 0;
>  
>      case ARGP_KEY_ARG:
> -      if (install_device)
> -     grub_util_error ("%s", _("More than one install device?"));
> -      install_device = xstrdup (arg);
> +      if (n_install_devices >= n_allocated_devices)
> +     {
> +       n_allocated_devices += 16;
> +       install_devices = xrealloc (install_devices, n_allocated_devices);
> +     }
> +      install_devices[n_install_devices++] = xstrdup (arg);
>        return 0;
>  
>      default:
> @@ -534,25 +540,55 @@ probe_cryptodisk_uuid (grub_disk_t disk)
>  }
>  
>  static int
> -is_same_disk (const char *a, const char *b)
> +same_disks (char **root_devs)
>  {
> -  while (1)
> +  int i;
> +
> +  for (i = 0; i < n_install_devices; i++)
>      {
> -      if ((*a == ',' || *a == '\0') && (*b == ',' || *b == '\0'))
> -     return 1;
> -      if (*a != *b)
> -     return 0;
> -      if (*a == '\\')
> +      char **d;
> +      const char *p1 = find_part_sep (install_drives[i]);
> +      size_t len1 = p1 ? p1 - install_drives[i] : strlen (install_drives[i]);
> +
> +      for (d = root_devs; *d; d++)
>       {
> -       if (a[1] != b[1])
> -         return 0;
> -       a += 2;
> -       b += 2;
> -       continue;
> +       const char *p2 = find_part_sep (*d);
> +       size_t len2 = p2 ? p2 - *d : strlen (*d);
> +
> +       if (len1 == len2 &&
> +           strncmp (install_drives[i], *d, len1) == 0)
> +         break;
>       }
> -      a++;
> -      b++;
> +      if (!*d)
> +     return 0;
> +    }
> +
> +  return 1;
> +}
> +
> +static int
> +same_partitions (char **root_devs)
> +{
> +  const char *first_part;
> +  char **p;
> +
> +  if (!root_devs[1])
> +    return 1;
> +
> +  first_part = find_part_sep (root_devs[0]);
> +  for (p = root_devs + 1; *p; p++)
> +    {
> +      const char *part = find_part_sep (*p);
> +
> +      if ((first_part == NULL) ^ (part == NULL))
> +     return 0;
> +      if (!first_part && !part)
> +     continue;
> +      if (strcmp (first_part, part))
> +     return 0;
>      }
> +
> +  return 1;
>  }
>  
>  static char *
> @@ -835,6 +871,8 @@ main (int argc, char *argv[])
>    int efidir_is_mac = 0;
>    int is_prep = 0;
>    const char *pkgdatadir;
> +  size_t i;
> +
>  
>    grub_util_host_init (&argc, &argv);
>    product_version = xstrdup (PACKAGE_VERSION);
> @@ -926,12 +964,26 @@ main (int argc, char *argv[])
>    switch (platform)
>      {
>      case GRUB_INSTALL_PLATFORM_I386_PC:
> +      break;
> +
> +    default:
> +      if (n_install_devices > 1)
> +     grub_util_error ("%s", _("More than one install device?"));
> +      break;
> +    }
> +
> +  switch (platform)
> +    {
> +    case GRUB_INSTALL_PLATFORM_I386_PC:
> +      if (!install_devices)
> +     install_bootsector = 0;
> +      break;
>      case GRUB_INSTALL_PLATFORM_SPARC64_IEEE1275:
> -      if (!install_device)
> +      if (!install_devices)
>       grub_util_error ("%s", _("install device isn't specified"));
>        break;
>      case GRUB_INSTALL_PLATFORM_POWERPC_IEEE1275:
> -      if (install_device)
> +      if (install_devices)
>       is_prep = 1;
>        break;
>      case GRUB_INSTALL_PLATFORM_MIPS_ARC:
> @@ -952,8 +1004,11 @@ main (int argc, char *argv[])
>      case GRUB_INSTALL_PLATFORM_MIPS_QEMU_MIPS:
>      case GRUB_INSTALL_PLATFORM_I386_XEN:
>      case GRUB_INSTALL_PLATFORM_X86_64_XEN:
> -      free (install_device);
> -      install_device = NULL;
> +      for (i = 0; i < n_install_devices; i++)
> +     free (install_devices[i]);
> +      free (install_devices);
> +      install_devices = NULL;
> +      n_install_devices = n_allocated_devices = 0;
>        break;
>  
>        /* pacify warning.  */
> @@ -996,11 +1051,6 @@ main (int argc, char *argv[])
>        is_efi = 1;
>        break;
>      default:
> -      is_efi = 0;
> -      break;
> -
> -      /* pacify warning.  */
> -    case GRUB_INSTALL_PLATFORM_MAX:
>        break;
>      }
>  
> @@ -1009,8 +1059,6 @@ main (int argc, char *argv[])
>    if (is_efi)
>      {
>        grub_fs_t fs;
> -      free (install_device);
> -      install_device = NULL;
>        if (!efidir)
>       {
>         char *d = grub_util_path_concat (2, bootdir, "efi");
> @@ -1045,7 +1093,9 @@ main (int argc, char *argv[])
>        if (!efidir_device_names || !efidir_device_names[0])
>       grub_util_error (_("cannot find a device for %s (is /dev mounted?)"),
>                            efidir);
> -      install_device = efidir_device_names[0];
> +      /* FIXME free install_devices */
> +      /* We will use just the first device */
> +      install_devices = efidir_device_names;
>  
>        for (curdev = efidir_device_names; *curdev; curdev++)
>         grub_util_pull_device (*curdev);
> @@ -1207,7 +1257,9 @@ main (int argc, char *argv[])
>         if (grub_strcmp (fs->name, "hfs") == 0
>             || grub_strcmp (fs->name, "hfsplus") == 0)
>           {
> -           install_device = macppcdir_device_names[0];
> +           /* FIXME free install_devices */
> +           /* We just use the first one */
> +           install_devices = macppcdir_device_names;
>             is_prep = 0;
>           }
>       }
> @@ -1318,35 +1370,37 @@ main (int argc, char *argv[])
>             debug_image);
>      }
>    char *prefix_drive = NULL;
> -  char *install_drive = NULL;
>  
> -  if (install_device)
> +  if (install_devices)
>      {
> -      if (install_device[0] == '('
> -       && install_device[grub_strlen (install_device) - 1] == ')')
> -        {
> -       size_t len = grub_strlen (install_device) - 2;
> -       install_drive = xmalloc (len + 1);
> -       memcpy (install_drive, install_device + 1, len);
> -       install_drive[len] = '\0';
> -        }
> -      else
> -     {
> -       grub_util_pull_device (install_device);
> -       install_drive = grub_util_get_grub_dev (install_device);
> -       if (!install_drive)
> -         grub_util_error (_("cannot find a GRUB drive for %s.  Check your 
> device.map"),
> -                          install_device);
> -     }
> +      install_drives = xmalloc (n_install_devices * sizeof 
> (*install_drives));
> +
> +      for (i = 0; i < n_install_devices; i++)
> +     if (install_devices[i][0] == '('
> +         && install_devices[i][grub_strlen (install_devices[i]) - 1] == ')')
> +       {
> +         size_t len = grub_strlen (install_devices[i]) - 2;
> +         install_drives[i] = xmalloc (len + 1);
> +         memcpy (install_drives[i], install_devices[i] + 1, len);
> +         install_drives[i][len] = '\0';
> +       }
> +     else
> +       {
> +         grub_util_pull_device (install_devices[i]);
> +         install_drives[i] = grub_util_get_grub_dev (install_devices[i]);
> +         if (!install_drives[i])
> +           grub_util_error (_("cannot find a GRUB drive for %s.  Check your 
> device.map"),
> +                            install_devices[i]);
> +       }
>      }
>  
>    if (!have_abstractions)
>      {
>        if ((disk_module && grub_strcmp (disk_module, "biosdisk") != 0)
> -       || grub_drives[1]
> -       || (!install_drive
> +       || (grub_drives[1] && !same_partitions (grub_drives))
> +       || (!install_drives
>             && platform != GRUB_INSTALL_PLATFORM_POWERPC_IEEE1275)
> -       || (install_drive && !is_same_disk (grub_drives[0], install_drive))
> +       || (install_drives && !same_disks (grub_drives))
>         || !have_bootdev (platform))
>       {
>         char *uuid = NULL;
> @@ -1656,24 +1710,27 @@ main (int argc, char *argv[])
>                                             "boot.img");
>       grub_install_copy_file (boot_img_src, boot_img, 1);
>  
> -     grub_util_info ("%sgrub-bios-setup %s %s %s %s %s --directory='%s' 
> --device-map='%s' '%s'",
> -                     /* TRANSLATORS: This is a prefix in the log to indicate 
> that usually
> -                        a command would be executed but due to an option was 
> skipped.  */
> -                     install_bootsector ? "" : _("NOT RUNNING: "),
> -                     allow_floppy ? "--allow-floppy " : "",
> -                     verbosity ? "--verbose " : "",
> -                     force ? "--force " : "",
> -                     !fs_probe ? "--skip-fs-probe" : "",
> -                     !add_rs_codes ? "--no-rs-codes" : "",
> -                     platdir,
> -                     device_map,
> -                     install_device);
> -                     
> -     /*  Now perform the installation.  */
> -     if (install_bootsector)
> -       grub_util_bios_setup (platdir, "boot.img", "core.img",
> -                             install_drive, force,
> -                             fs_probe, allow_floppy, add_rs_codes);
> +     for (i = 0; i < n_install_devices; i++)
> +       {
> +         grub_util_info ("%sgrub-bios-setup %s %s %s %s %s --directory='%s' 
> --device-map='%s' '%s'",
> +                         /* TRANSLATORS: This is a prefix in the log to 
> indicate that usually
> +                            a command would be executed but due to an option 
> was skipped.  */
> +                         install_bootsector ? "" : _("NOT RUNNING: "),
> +                         allow_floppy ? "--allow-floppy " : "",
> +                         verbosity ? "--verbose " : "",
> +                         force ? "--force " : "",
> +                         !fs_probe ? "--skip-fs-probe" : "",
> +                         !add_rs_codes ? "--no-rs-codes" : "",
> +                         platdir,
> +                         device_map,
> +                         install_devices[i]);
> +
> +         /*  Now perform the installation.  */
> +         if (install_bootsector)
> +           grub_util_bios_setup (platdir, "boot.img", "core.img",
> +                                 install_drives[i], force,
> +                                 fs_probe, allow_floppy, add_rs_codes);
> +       }
>       break;
>        }
>      case GRUB_INSTALL_PLATFORM_SPARC64_IEEE1275:
> @@ -1693,12 +1750,12 @@ main (int argc, char *argv[])
>                       !fs_probe ? "--skip-fs-probe" : "",
>                       platdir,
>                       device_map,
> -                     install_drive);
> +                     install_drives[0]);
>                       
>       /*  Now perform the installation.  */
>       if (install_bootsector)
>         grub_util_sparc_setup (platdir, "boot.img", "core.img",
> -                              install_device, force,
> +                              install_devices[0], force,
>                                fs_probe, allow_floppy,
>                                0 /* unused */ );
>       break;
> @@ -1734,7 +1791,7 @@ main (int argc, char *argv[])
>  
>         fill_core_services (core_services);
>  
> -       ins_dev = grub_device_open (install_drive);
> +       ins_dev = grub_device_open (install_drives[0]);
>  
>         bless (ins_dev, core_services, 0);
>  
> @@ -1745,7 +1802,7 @@ main (int argc, char *argv[])
>  
>             partno = ins_dev->disk->partition
>               ? ins_dev->disk->partition->number + 1 : 0;
> -           dev = grub_util_get_os_disk (install_device);
> +           dev = grub_util_get_os_disk (install_devices[0]);
>             grub_install_register_ieee1275 (0, dev, partno,
>                                             "\\\\BootX");
>           }
> @@ -1757,10 +1814,10 @@ main (int argc, char *argv[])
>         break;
>       }
>        /* If a install device is defined, copy the core.elf to PReP 
> partition.  */
> -      if (is_prep && install_device && install_device[0])
> +      if (is_prep)
>       {
>         grub_device_t ins_dev;
> -       ins_dev = grub_device_open (install_drive);
> +       ins_dev = grub_device_open (install_drives[0]);
>         if (!ins_dev || !is_prep_partition (ins_dev))
>           {
>             grub_util_error ("%s", _("the chosen partition is not a PReP 
> partition"));
> @@ -1772,13 +1829,13 @@ main (int argc, char *argv[])
>           }
>         else
>           {
> -           char *s = xasprintf ("dd if=/dev/zero of=%s", install_device);
> +           char *s = xasprintf ("dd if=/dev/zero of=%s", install_devices[0]);
>             grub_util_error (_("the PReP partition is not empty. If you are 
> sure you want to use it, run dd to clear it: `%s'"),
>                              s);
>           }
>         grub_device_close (ins_dev);
>         if (update_nvram)
> -         grub_install_register_ieee1275 (1, grub_util_get_os_disk 
> (install_device),
> +         grub_install_register_ieee1275 (1, grub_util_get_os_disk 
> (install_devices[0]),
>                                           0, NULL);
>         break;
>        }
> @@ -1798,7 +1855,7 @@ main (int argc, char *argv[])
>       }
>        break;
>      case GRUB_INSTALL_PLATFORM_MIPS_ARC:
> -      grub_install_sgi_setup (install_device, imgfile, "grub");
> +      grub_install_sgi_setup (install_devices[0], imgfile, "grub");
>        break;
>  
>      case GRUB_INSTALL_PLATFORM_I386_EFI:
> @@ -1834,7 +1891,7 @@ main (int argc, char *argv[])
>  
>         fill_core_services(core_services);
>  
> -       ins_dev = grub_device_open (install_drive);
> +       ins_dev = grub_device_open (install_drives[0]);
>  
>         bless (ins_dev, boot_efi, 1);
>         if (!removable && update_nvram)
>

signature.asc
Description: OpenPGP digital signature