Re: [RFC] Dedicated LVM volume as alternative to embedding

["Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 9126 bytes --]

This solution wouldn't play well with LVM layers. A better solution is
being jointly developped with lvm guys.
On 03.06.2013 02:58, Piotras wrote:
> Hi,
> 
> Attached patch allows for using alternative disk area when usual embedding
> is not possible. It helps for case where LVM is used on boot disk and user
> can create new volume for exclusive use by GRUB.
> 
> I currently require for the volume to use contiguous sectors on single disk
> (boot disk). Name of the volume is passed with additional parameter for
> grub-setup:
>> grub-bios-setup --dedicated-loader-volume=lvm/myVG-myLoaderLV --skip-fs-probe ... /dev/sda
> Parameter "--skip-fs-probe" is required for current version of the patch.
> 
> The implementation is inspired by existing function grub_util_ldm_embed in
> grub-core/disk/ldm.c (notice that this function is not working in current
> form).
> 
> It should be possible to generalize my patch to cover both LVM and LDM with
> common function, but I didn't test it. It may also be extend for using with
> traditional partition tables (where use can create new partition for
> exclusing use by GRUB).
> 
> I'd like to check if this feature can be added to official GRUB and what
> changes would be required for the attached patch to be merged.
> 
> 
> Best regards,
> 
> Piotr Krysiuk
> 
> 
> ---
>  grub-core/disk/lvm.c        |   75 ++++++++++++++++++++++++++++++++++++++++++-
>  include/grub/emu/hostdisk.h |    7 ++++
>  util/grub-setup.c           |   24 +++++++++++---
>  3 files changed, 100 insertions(+), 6 deletions(-)
> 
> diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
> index 508e94a..8b7ff44 100644
> --- a/grub-core/disk/lvm.c
> +++ b/grub-core/disk/lvm.c
> @@ -746,7 +746,80 @@ grub_lvm_detect (grub_disk_t disk,
>    return NULL;
>  }
> 
> -
> +#ifdef GRUB_UTIL
> +
> +grub_err_t
> +grub_util_lvm_embed (struct grub_disk *disk,
> +                     const char *loader_lv_name,
> +                     unsigned int *nsectors,
> +                     unsigned int max_nsectors,
> +                     grub_embed_type_t embed_type,
> +                     grub_disk_addr_t **sectors)
> +{
> +  grub_disk_t loader_disk;
> +  struct grub_diskfilter_lv *loader_lv;
> +  unsigned i;
> +
> +  if (embed_type != GRUB_EMBED_PCBIOS)
> +    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
> +                       "LVM curently supports only PC-BIOS embedding");
> +
> +  loader_disk = grub_disk_open (loader_lv_name);
> +  if (! loader_disk)
> +    grub_util_error ("%s", grub_errmsg);
> +
> +  if (loader_disk->dev->id != GRUB_DISK_DEVICE_DISKFILTER_ID)
> +    goto unable_to_embed;
> +
> +  loader_lv = loader_disk->data;
> +
> +  if (loader_lv->size > (1U << 15))
> +    return grub_error (GRUB_ERR_OUT_OF_RANGE,
> +                       N_("your dedicated loader volume is larger
> then 16MBytes;"
> +                          " grub won't use it to prevent
> unintentional corruption"
> +                          " of user data"));
> +
> +  if (!loader_lv->visible || !loader_lv->fullname)
> +    goto unable_to_embed;
> +
> +  if (loader_lv->segment_count != 1)
> +    goto unable_to_embed;
> +
> +  if (loader_lv->segments->type != GRUB_DISKFILTER_STRIPED
> +      || loader_lv->segments->node_count != 1
> +      || loader_lv->segments->start_extent != 0)
> +    goto unable_to_embed;
> +
> +  if (disk->partition
> +      || grub_strcmp (loader_lv->segments->nodes->pv->disk->name, disk->name))
> +    goto unable_to_embed;
> +
> +  if (loader_lv->size < *nsectors)
> +    return grub_error (GRUB_ERR_OUT_OF_RANGE,
> +                       N_("your dedicated loader volume is too small;"
> +                          " grub can't use it"));
> +  *nsectors = loader_lv->size;
> +  if (*nsectors > max_nsectors)
> +    *nsectors = max_nsectors;
> +  *sectors = grub_malloc (*nsectors * sizeof (**sectors));
> +  if (!*sectors)
> +    return grub_errno;
> +  for (i = 0; i < *nsectors; i++)
> +    (*sectors)[i] = (loader_lv->segments->nodes->start
> +                     + loader_lv->segments->nodes->pv->start_sector
> +                     + i);
> +
> +  grub_disk_close (loader_disk);
> +  return GRUB_ERR_NONE;
> +
> + unable_to_embed:
> +
> +  return grub_error (GRUB_ERR_FILE_NOT_FOUND,
> +                     N_("your dedicated loader volume is invalid;"
> +                        " grub can't use it"));
> +}
> +
> +#endif
> 
>  static struct grub_diskfilter grub_lvm_dev = {
>    .name = "lvm",
> diff --git a/include/grub/emu/hostdisk.h b/include/grub/emu/hostdisk.h
> index 058973b..af59b9e 100644
> --- a/include/grub/emu/hostdisk.h
> +++ b/include/grub/emu/hostdisk.h
> @@ -51,6 +51,13 @@ grub_util_ldm_embed (struct grub_disk *disk,
> unsigned int *nsectors,
>       unsigned int max_nsectors,
>       grub_embed_type_t embed_type,
>       grub_disk_addr_t **sectors);
> +grub_err_t
> +grub_util_lvm_embed (struct grub_disk *disk,
> +                     const char *loader_lv_name,
> +                     unsigned int *nsectors,
> +                     unsigned int max_nsectors,
> +                     grub_embed_type_t embed_type,
> +                     grub_disk_addr_t **sectors);
>  #endif
>  grub_disk_addr_t
>  grub_hostdisk_find_partition_start (const char *dev);
> diff --git a/util/grub-setup.c b/util/grub-setup.c
> index 27a815f..b95d05b 100644
> --- a/util/grub-setup.c
> +++ b/util/grub-setup.c
> @@ -85,6 +85,7 @@
> 
>  #define DEFAULT_BOOT_FILE "boot.img"
>  #define DEFAULT_CORE_FILE "core.img"
> +#define OPT_LOADER_VOLUME       -3
> 
>  #ifdef GRUB_SETUP_SPARC64
>  #define grub_target_to_host16(x) grub_be_to_cpu16(x)
> @@ -243,7 +244,7 @@ identify_partmap (grub_disk_t disk __attribute__ ((unused)),
>  static void
>  setup (const char *dir,
>         const char *boot_file, const char *core_file,
> -       const char *dest, int force,
> +       const char *dest, const char *loader_volume, int force,
>         int fs_probe, int allow_floppy)
>  {
>    char *boot_path, *core_path, *core_path_dev, *core_path_dev_full;
> @@ -459,12 +460,12 @@ setup (const char *dir,
> 
>      free (tmp_img);
> 
> -    if (! ctx.dest_partmap && ! fs && !is_ldm)
> +    if (! ctx.dest_partmap && ! fs && !is_ldm && !loader_volume)
>        {
>   grub_util_warn ("%s", _("Attempting to install GRUB to a
> partitionless disk or to a partition.  This is a BAD idea."));
>   goto unable_to_embed;
>        }
> -    if (ctx.multiple_partmaps || (ctx.dest_partmap && fs) || (is_ldm && fs))
> +    if (ctx.multiple_partmaps || (ctx.dest_partmap && fs) || (is_ldm
> && fs) || (loader_volume && fs))
>        {
>   grub_util_warn ("%s", _("Attempting to install GRUB to a disk with
> multiple partition labels.  This is not supported yet."));
>   goto unable_to_embed;
> @@ -492,7 +493,10 @@ setup (const char *dir,
>        maxsec = ((0x78000 - GRUB_KERNEL_I386_PC_LINK_ADDR)
>   >> GRUB_DISK_SECTOR_BITS);
> 
> -    if (is_ldm)
> +    if (loader_volume)
> +      err = grub_util_lvm_embed (dest_dev->disk, loader_volume, &nsec, maxsec,
> + GRUB_EMBED_PCBIOS, &sectors);
> +    else if (is_ldm)
>        err = grub_util_ldm_embed (dest_dev->disk, &nsec, maxsec,
>   GRUB_EMBED_PCBIOS, &sectors);
>      else if (ctx.dest_partmap)
> @@ -983,6 +987,8 @@ static struct argp_option options[] = {
>     N_("use GRUB files in the directory DIR [default=%s]"), 0},
>    {"device-map",  'm', N_("FILE"), 0,
>     N_("use FILE as the device map [default=%s]"), 0},
> +  {"dedicated-loader-volume", OPT_LOADER_VOLUME, N_("VOLUME"), 0,
> +   N_("allocate VOLUME for exclusive use by GRUB. Existing data on
> VOLUME will be overwritten!"), 0},
>    {"force",       'f', 0,      0,
>     N_("install even if problems are detected"), 0},
>    {"skip-fs-probe",'s',0,      0,
> @@ -1024,6 +1030,7 @@ struct arguments
>    char *core_file;
>    char *dir;
>    char *dev_map;
> +  char *loader_volume;
>    int  force;
>    int  fs_probe;
>    int allow_floppy;
> @@ -1071,6 +1078,13 @@ argp_parser (int key, char *arg, struct
> argp_state *state)
>          arguments->dev_map = xstrdup (arg);
>          break;
> 
> +      case OPT_LOADER_VOLUME:
> +        if (arguments->loader_volume)
> +          free (arguments->loader_volume);
> +
> +        arguments->loader_volume = xstrdup (arg);
> +        break;
> +
>        case 'f':
>          arguments->force = 1;
>          break;
> @@ -1203,7 +1217,7 @@ main (int argc, char *argv[])
>    setup (arguments.dir ? : DEFAULT_DIRECTORY,
>   arguments.boot_file ? : DEFAULT_BOOT_FILE,
>   arguments.core_file ? : DEFAULT_CORE_FILE,
> - dest_dev, arguments.force,
> + dest_dev, arguments.loader_volume, arguments.force,
>   arguments.fs_probe, arguments.allow_floppy);
> 
>    /* Free resources.  */
> --
> 1.7.9.5
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 291 bytes --]