[RFC] Dedicated LVM volume as alternative to embedding

[RFC] Dedicated LVM volume as alternative to embedding

[Piotras]

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

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

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- 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 --]

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

[Andrey Borzenkov]

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

В Mon, 03 Jun 2013 03:26:23 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> This solution wouldn't play well with LVM layers. A better solution is
> being jointly developped with lvm guys.

Do you have pointers to this discussion?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 03.06.2013 06:42, Andrey Borzenkov wrote:
> В Mon, 03 Jun 2013 03:26:23 +0200
> Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:
> 
>> This solution wouldn't play well with LVM layers. A better solution is
>> being jointly developped with lvm guys.
> 
> Do you have pointers to this discussion?
> 
> 
meatspace://berlin/linuxtag
> 
> _______________________________________________
> 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 --]

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

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 6/3/2013 6:09 AM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 03.06.2013 06:42, Andrey Borzenkov wrote:
>> В Mon, 03 Jun 2013 03:26:23 +0200 Vladimir 'φ-coder/phcoder'
>> Serbinenko <phcoder@gmail.com> пишет:
>> 
>>> This solution wouldn't play well with LVM layers. A better
>>> solution is being jointly developped with lvm guys.
>> 
>> Do you have pointers to this discussion?
>> 
>> 
> meatspace://berlin/linuxtag

What kind of URL is that?



-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJRwK5KAAoJEJrBOlT6nu75dQwH/jqqMkiK0wtdDE3dCkqt9z5/
fOmuARt4u0XeQ4T7IXUJ19DDbi6ZNSs/7XiVQZFixhkfhmUrlhjRiVlsffATtIU0
lHT+rJYlLRexaIisfVATaH925l+chA6k1lXkAmXbiev2OfGQdPh8DOL4Ztoa9jOJ
zZd90eWJVB6ZSFZizVQ3ETKjTzxFp6VRCO0NmyyugnFL8BQ9/hTyX6XkoMAgDZN2
UV84d3Tn+2KVxT2yAtYin8cD0Ac7JSzv2O01KhVLaV1inNqoeFxeqtj1P4oQ5GHf
XzDxYMUOa4YxzAC9PUeeH5Mrg2j7haFahR47hnynURj3nnEQKGQuZUku17PPxUE=
=7oIa
-----END PGP SIGNATURE-----