All of lore.kernel.org
 help / color / mirror / Atom feed
* Reproducible grub-install
@ 2019-10-21 14:30 Miguel Arruga Vivas
  2019-10-23  9:09 ` Daniel Kiper
  0 siblings, 1 reply; 5+ messages in thread
From: Miguel Arruga Vivas @ 2019-10-21 14:30 UTC (permalink / raw)
  To: guix-devel; +Cc: grub-devel

Hi, everybody!

After taking a deeper look into our (guix's) grub installation
procedure, I have the thought that it could be a neat idea to make the
boot directory an actual derivation instead something of the global
status.

From what I currently understand:

  - boot.img/core.img and load.cfg: The written images must be replaced
    on each installation.  This is one task performed by grub-install.

  - /boot/grub/*: The contents of these folders should be reproducible,
    such as the modules or the localization binaries, as currently
    grub.cfg is.  This is the other task performed by grub-install.

  - /boot/grub/grubenv: IIUC, this file must be writable by grub.  This
    should not be on the store, and not sharing the path may be the
    main problem right now to implement this.

AFAIK, the grubenv problem requires a modification of the grub code if
we try to use a different path for this kind-of-modifiable file, so this
would require modify grub to being able to lookup for that file
somewhere else.  This way the global state can be made explicit.

The image installation into the device is a separate issue from the
binaries installation, that could be separated into two separate
binaries, or two steps/flags for grub-install, one for binaries
installation into ${boot-directory}/grub and the other one for load.cfg
generation and core/boot.img installation.

To everyone: Are you aware of any other way to achieve this?  What do
you think?

To grub-devel: I'd be able to send patches for the latter if you think
it is a good idea without help, but I guess that the first kind of
modification would need some and deeper study of grub code.

To guix-devel: Even though the procedure I have in mind needs
changes in grub, there are alternative ways to achieve this with the
current tools, as copying the files and using the installation as an
"implicit" guix-challenge, but they are not as neat an clean as the
split between reproducible binaries installation and global state,
which includes the disk preparation for the load of the bootloader.

Happy hacking to all!
Miguel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Reproducible grub-install
  2019-10-21 14:30 Reproducible grub-install Miguel Arruga Vivas
@ 2019-10-23  9:09 ` Daniel Kiper
  2019-10-28 11:25   ` Granularity of grub-install (was Re: Reproducibility of grub-install) Miguel Arruga Vivas
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Kiper @ 2019-10-23  9:09 UTC (permalink / raw)
  To: Miguel Arruga Vivas; +Cc: guix-devel, grub-devel

On Mon, Oct 21, 2019 at 04:30:21PM +0200, Miguel Arruga Vivas wrote:
> Hi, everybody!
>
> After taking a deeper look into our (guix's) grub installation
> procedure, I have the thought that it could be a neat idea to make the
> boot directory an actual derivation instead something of the global
> status.

I do not understand what you mean by "make the boot directory an actual
derivation instead something of the global status". Could you elaborate
more about that?

> From what I currently understand:
>
>   - boot.img/core.img and load.cfg: The written images must be replaced
>     on each installation.  This is one task performed by grub-install.

Yep.

>   - /boot/grub/*: The contents of these folders should be reproducible,
>     such as the modules or the localization binaries, as currently
>     grub.cfg is.  This is the other task performed by grub-install.

I am not sure why grub.cfg have to be reproducible. OK, to some extent
it has but...

>
>   - /boot/grub/grubenv: IIUC, this file must be writable by grub.  This
>     should not be on the store, and not sharing the path may be the
>     main problem right now to implement this.

I do not understand this.

> AFAIK, the grubenv problem requires a modification of the grub code if
> we try to use a different path for this kind-of-modifiable file, so this
> would require modify grub to being able to lookup for that file
> somewhere else.  This way the global state can be made explicit.

What do you mean by "This way the global state can be made explicit."?

> The image installation into the device is a separate issue from the
> binaries installation, that could be separated into two separate
> binaries, or two steps/flags for grub-install, one for binaries
> installation into ${boot-directory}/grub and the other one for load.cfg
> generation and core/boot.img installation.

Why do you need to separate this thing into two steps?

> To everyone: Are you aware of any other way to achieve this?  What do
> you think?
>
> To grub-devel: I'd be able to send patches for the latter if you think

Patches are always welcome...

> it is a good idea without help, but I guess that the first kind of
> modification would need some and deeper study of grub code.
>

Yep...

Daniel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Granularity of grub-install (was Re: Reproducibility of grub-install)
  2019-10-23  9:09 ` Daniel Kiper
@ 2019-10-28 11:25   ` Miguel Arruga Vivas
  2019-10-28 11:27     ` [PATCH] install: Free allocated path for grub.efi Miguel Arruga Vivas
  0 siblings, 1 reply; 5+ messages in thread
From: Miguel Arruga Vivas @ 2019-10-28 11:25 UTC (permalink / raw)
  To: Daniel Kiper; +Cc: guix-devel, grub-devel

Hi,

First of all, thank you for your comments.  My answers are inline,
although the bad subject was a main issue of misunderstanding.  I
changed it, as I think it reflects better my current idea.  I'm sorry
for the confusion it certainly caused.

Wed, 23 Oct 2019 11:09:07 +0200
Daniel Kiper <dkiper@net-space.pl> wrote:
> On Mon, Oct 21, 2019 at 04:30:21PM +0200, Miguel Arruga Vivas wrote:
> > Hi, everybody!
> >
> > After taking a deeper look into our (guix's) grub installation
> > procedure, I have the thought that it could be a neat idea to make
> > the boot directory an actual derivation instead something of the
> > global status.  
> 
> I do not understand what you mean by "make the boot directory an
> actual derivation instead something of the global status". Could you
> elaborate more about that?

Currently, the /boot directory is partially a system-wide directory,
even though most of its contents are direct copies of the ones from
grub's installation prefix: /usr/... in most distributions including
BSD world, /{nix,gnu}/store/<hash>-grub-2.04 in Nix/Guix.  Most of its
contents don't depend at all on the actual machine and that can be
shared between machine-dependent boot-sector/efi-variables
"activations".  These contents include all grub modules and message
object files.  On the other hand, the task of the platform-specific
installation cannot be "shared", as it must be performed directly "on
the metal".  And there is a third task between these two: the generation
of the raw image, that depends on the hardware configuration through
the provided configuration file *and* the binary installation.

> (...)
> I am not sure why grub.cfg have to be reproducible. OK, to some extent
> it has but...

Guix doesn't generate grub.cfg through grub-mkconfig, as each kernel is
also associated with the configuration for the init system too--GNU
Shepherd in Guix--, and one kernel may boot up different systems, so it
is generated with Guile scripts.  My understanding is that grub-mkconfig
is a portable tool, not only intended for this case, whose output is
system dependant.  I mixed up concepts here, sorry.

> >
> >   - /boot/grub/grubenv: IIUC, this file must be writable by grub.
> > This should not be on the store, and not sharing the path may be the
> >     main problem right now to implement this.  
> 
> I do not understand this.
>
> > AFAIK, the grubenv problem requires a modification of the grub code
> > if we try to use a different path for this kind-of-modifiable file,
> > so this would require modify grub to being able to lookup for that
> > file somewhere else.  This way the global state can be made
> > explicit.  
> 
> What do you mean by "This way the global state can be made explicit."?

Sorry again.  We do not use load_env nor save_env in our
configuration.  I misunderstood the creation in grub-install.c as a
hidden requirement for it, but I'd just had to read the manual.  That
global state already is explicit enough.  I think now that its creation
should be optional, even though it's created by default as
grub-mkconfig makes use of it.  Should I send a patch for it?

> > The image installation into the device is a separate issue from the
> > binaries installation, that could be separated into two separate
> > binaries, or two steps/flags for grub-install, one for binaries
> > installation into ${boot-directory}/grub and the other one for
> > load.cfg generation and core/boot.img installation.  
> 
> Why do you need to separate this thing into two steps?

I'd like being able to have different grub /boot-like folders, maybe
through unions in the file system (as hard links) of the shared
contents and the physical system dependencies, and activate them
selectively without writing into them any more.  This is intended for
Guix's store[1], but other use cases are possible, in order to roll-back
to a previous generation of the system by only writing
core.img/grub.efi/etc. into platform-dependant locations.

As I said before, I see them now as three steps.  The middle one is
already there with grub-mkimage.  The bios-like installation can be
performed with grub-setup, but EFI systems need grub-install to copy
the image and activate it, the reason behind grub-setup deprecation if I
understand the code correctly.  And the copy of the correct files to
the boot-directory needs no modification at all of grub-install.

> > To grub-devel: I'd be able to send patches for the latter if you
> > think  
> 
> Patches are always welcome...

I'll send just to grub-devel following this mail a patch for a minor
problem I've found reading the code. 

The code for an --only-platform-installation flag (the name I'm using
right now) needs more work but I'll send it as soon as I have
something I've tested on x86 BIOS and x86_64 EFI.  From my
current understanding it would replace completely grub-setup, wouldn't
it?

Happy hacking!
Miguel

[1] https://guix.gnu.org/manual/en/html_node/Features.html


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] install: Free allocated path for grub.efi.
  2019-10-28 11:25   ` Granularity of grub-install (was Re: Reproducibility of grub-install) Miguel Arruga Vivas
@ 2019-10-28 11:27     ` Miguel Arruga Vivas
  2019-10-28 21:01       ` [PATCH 2/2] install: Add only-platform-installation option Miguel Arruga Vivas
  0 siblings, 1 reply; 5+ messages in thread
From: Miguel Arruga Vivas @ 2019-10-28 11:27 UTC (permalink / raw)
  To: grub-devel; +Cc: Daniel Kiper

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

Hi,

This patch is a minor issue, but in terms of correctness I think this
free is missing.

Happy hacking,
Miguel

[-- Attachment #2: 0001-install-Free-allocated-path-for-grub.efi.patch --]
[-- Type: text/x-patch, Size: 816 bytes --]

From 1720e89de777fd3a30a6824797d97855b7bb8b68 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miguel=20=C3=81ngel=20Arruga=20Vivas?=
 <rosen644835@gmail.com>
Date: Mon, 28 Oct 2019 10:08:25 +0100
Subject: [PATCH 1/2] install: Free allocated path for grub.efi.

* util/grub-install.c (main): Free local variable allocated with
grub_util_path_concat.
---
 util/grub-install.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/util/grub-install.c b/util/grub-install.c
index 8970b73aa..8f84bf8cd 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -1664,6 +1664,7 @@ main (int argc, char *argv[])
 				       /* memdisk */ NULL,
 				      have_load_cfg ? load_cfg : NULL,
 				       /* image target */ mkimage_target, 0);
+	free (dst);
       }
       break;
     case GRUB_INSTALL_PLATFORM_ARM_EFI:
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] install: Add only-platform-installation option.
  2019-10-28 11:27     ` [PATCH] install: Free allocated path for grub.efi Miguel Arruga Vivas
@ 2019-10-28 21:01       ` Miguel Arruga Vivas
  0 siblings, 0 replies; 5+ messages in thread
From: Miguel Arruga Vivas @ 2019-10-28 21:01 UTC (permalink / raw)
  To: grub-devel; +Cc: Daniel Kiper

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

Hello again,

I've tested this patch on BIOS x86 (on a vm) and x86_64 EFI.  Surely it
does extra work, plenty of checks and string allocations are not needed
at all, because it skips only calls that write files into grubdir or
platdir (not macppcdir nor efidir).

What do you think?  Refactoring the main method may help clarify the
flow (checks for the actual hardware/partition configuration -> write
files into grub's directory for boot, including early-boot related
files -> platform dependant installation), but I'd like to hear some
comments on the minimal change before moving code around.

Happy hacking!
Miguel

[-- Attachment #2: 0002-install-Add-only-platform-installation-option.patch --]
[-- Type: text/x-patch, Size: 8735 bytes --]

From 1c6dd7b456d9efc6fdd30cc5c170817cddb361ce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miguel=20=C3=81ngel=20Arruga=20Vivas?=
 <rosen644835@gmail.com>
Date: Mon, 28 Oct 2019 09:56:06 +0100
Subject: [PATCH 2/2] install: Add only-platform-installation option.

* util/grub-install.c (only_platform_installation): New variable.
(unnamed enum): Add OPTION_ONLY_PLATFORM_INSTALLATION.
(argp_parser)<OPTION_NO_NVRAM, OPTION_NO_BOOTSECTOR>: Add check.
<OPTION_ONLY_PLATFORM_INSTALLATION>: New case.
(options): Add only-platform-installation option.
(main): Do not write into grubdir and platdir when
only-platform-installation option is seleted.
---
 util/grub-install.c | 105 ++++++++++++++++++++++++++++----------------
 1 file changed, 66 insertions(+), 39 deletions(-)

diff --git a/util/grub-install.c b/util/grub-install.c
index 8f84bf8cd..2b446717e 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -79,6 +79,7 @@ static char *label_color;
 static char *label_bgcolor;
 static char *product_version;
 static int add_rs_codes = 1;
+static int only_platform_installation = 0;
 
 enum
   {
@@ -109,7 +110,8 @@ enum
     OPTION_LABEL_FONT,
     OPTION_LABEL_COLOR,
     OPTION_LABEL_BGCOLOR,
-    OPTION_PRODUCT_VERSION
+    OPTION_PRODUCT_VERSION,
+    OPTION_ONLY_PLATFORM_INSTALLATION
   };
 
 static int fs_probe = 1;
@@ -197,6 +199,9 @@ argp_parser (int key, char *arg, struct argp_state *state)
       return 0;
 
     case OPTION_NO_NVRAM:
+      if (only_platform_installation)
+	grub_util_error ("%s", _("Option --only-platform-installation is "
+				 "incompatible with --no-bootsector/nvram."));
       update_nvram = 0;
       return 0;
 
@@ -217,6 +222,9 @@ argp_parser (int key, char *arg, struct argp_state *state)
       return 0;
 
     case OPTION_NO_BOOTSECTOR:
+      if (only_platform_installation)
+	grub_util_error ("%s", _("Option --only-platform-installation is "
+				 "incompatible with --no-bootsector/nvram."));
       install_bootsector = 0;
       return 0;
 
@@ -233,6 +241,13 @@ argp_parser (int key, char *arg, struct argp_state *state)
       bootloader_id = xstrdup (arg);
       return 0;
 
+    case OPTION_ONLY_PLATFORM_INSTALLATION:
+      if (!install_bootsector && !update_nvram)
+	grub_util_error ("%s", _("Option --only-platform-installation is "
+				 "incompatible with --no-bootsector/nvram."));
+      only_platform_installation = 1;
+      return 0;
+
     case ARGP_KEY_ARG:
       if (install_device)
 	grub_util_error ("%s", _("More than one install device?"));
@@ -275,6 +290,8 @@ static struct argp_option options[] = {
   {"disk-module", OPTION_DISK_MODULE, N_("MODULE"), 0,
    N_("disk module to use (biosdisk or native). "
       "This option is only available on BIOS target."), 2},
+  {"only-platform-installation", OPTION_ONLY_PLATFORM_INSTALLATION, 0, 0,
+   N_("only perform the platform-dependant installation."), 0},
   {"no-nvram", OPTION_NO_NVRAM, 0, 0,
    N_("don't update the `boot-device'/`Boot*' NVRAM variables. "
       "This option is only available on EFI and IEEE1275 targets."), 2},
@@ -533,7 +550,8 @@ probe_cryptodisk_uuid (grub_disk_t disk)
       free (list);
       list = tmp;
     }
-  if (disk->dev->id == GRUB_DISK_DEVICE_CRYPTODISK_ID)
+  if (disk->dev->id == GRUB_DISK_DEVICE_CRYPTODISK_ID
+      && !only_platform_installation)
     {
       const char *uuid = grub_util_cryptodisk_get_uuid (disk);
       if (!load_cfg_f)
@@ -1247,11 +1265,12 @@ main (int argc, char *argv[])
 	}
     }
 
-  grub_install_copy_files (grub_install_source_directory,
-			   grubdir, platform);
+  if (!only_platform_installation)
+    grub_install_copy_files (grub_install_source_directory,
+			     grubdir, platform);
 
   char *envfile = grub_util_path_concat (2, grubdir, "grubenv");
-  if (!grub_util_is_regular (envfile))
+  if (!grub_util_is_regular (envfile) && !only_platform_installation)
     grub_util_create_envblk_file (envfile);
 
   size_t ndev = 0;
@@ -1343,9 +1362,10 @@ main (int argc, char *argv[])
   load_cfg = grub_util_path_concat (2, platdir,
 				  "load.cfg");
 
-  grub_util_unlink (load_cfg);
+  if (!only_platform_installation)
+    grub_util_unlink (load_cfg);
 
-  if (debug_image && debug_image[0])
+  if (debug_image && debug_image[0] && !only_platform_installation)
     {
       load_cfg_f = grub_util_fopen (load_cfg, "wb");
       have_load_cfg = 1;
@@ -1375,7 +1395,7 @@ main (int argc, char *argv[])
 	}
     }
 
-  if (!have_abstractions)
+  if (!have_abstractions && !only_platform_installation)
     {
       if ((disk_module && grub_strcmp (disk_module, "biosdisk") != 0)
 	  || grub_drives[1]
@@ -1418,8 +1438,9 @@ main (int argc, char *argv[])
 		grub_util_error (_("Can't create file: %s"), strerror (errno));
 	      fclose (flf);
 	      relfl = grub_make_system_path_relative_to_its_root (fl);
-	      fprintf (load_cfg_f, "search.file %s root ",
-		       relfl);
+	      if (!only_platform_installation)
+		fprintf (load_cfg_f, "search.file %s root ",
+			 relfl);
 	      grub_install_push_module ("search_fs_file");
 	    }
 	  for (curdev = grub_devices, curdrive = grub_drives; *curdev; curdev++,
@@ -1534,7 +1555,7 @@ main (int argc, char *argv[])
 	  prefix_drive = xasprintf ("(%s)", p);
 	}
     }
-  else
+  else if (!only_platform_installation)
     {
       if (config.is_cryptodisk_enabled)
 	{
@@ -1628,44 +1649,48 @@ main (int argc, char *argv[])
 				       core_name);
   char *prefix = xasprintf ("%s%s", prefix_drive ? : "",
 			    relative_grubdir);
-  grub_install_make_image_wrap (/* source dir  */ grub_install_source_directory,
-				/*prefix */ prefix,
-				/* output */ imgfile,
-				/* memdisk */ NULL,
-				have_load_cfg ? load_cfg : NULL,
-				/* image target */ mkimage_target, 0);
+  if (!only_platform_installation)
+    grub_install_make_image_wrap (/* source dir  */ grub_install_source_directory,
+				  /*prefix */ prefix,
+				  /* output */ imgfile,
+				  /* memdisk */ NULL,
+				  have_load_cfg ? load_cfg : NULL,
+				  /* image target */ mkimage_target, 0);
   /* Backward-compatibility kludges.  */
   switch (platform)
     {
     case GRUB_INSTALL_PLATFORM_MIPSEL_LOONGSON:
-      {
-	char *dst = grub_util_path_concat (2, bootdir, "grub.elf");
-	grub_install_copy_file (imgfile, dst, 1);
-	free (dst);
-      }
+      if (!only_platform_installation)
+	{
+	  char *dst = grub_util_path_concat (2, bootdir, "grub.elf");
+	  grub_install_copy_file (imgfile, dst, 1);
+	  free (dst);
+	}
       break;
 
     case GRUB_INSTALL_PLATFORM_I386_IEEE1275:
     case GRUB_INSTALL_PLATFORM_POWERPC_IEEE1275:
-      {
-	char *dst = grub_util_path_concat (2, grubdir, "grub");
-	grub_install_copy_file (imgfile, dst, 1);
-	free (dst);
-      }
+      if (!only_platform_installation)
+	{
+	  char *dst = grub_util_path_concat (2, grubdir, "grub");
+	  grub_install_copy_file (imgfile, dst, 1);
+	  free (dst);
+	}
       break;
 
     case GRUB_INSTALL_PLATFORM_I386_EFI:
     case GRUB_INSTALL_PLATFORM_X86_64_EFI:
-      {
-	char *dst = grub_util_path_concat (2, platdir, "grub.efi");
-	grub_install_make_image_wrap (/* source dir  */ grub_install_source_directory,
-				      /* prefix */ "",
-				       /* output */ dst,
-				       /* memdisk */ NULL,
-				      have_load_cfg ? load_cfg : NULL,
-				       /* image target */ mkimage_target, 0);
-	free (dst);
-      }
+      if (!only_platform_installation)
+	{
+	  char *dst = grub_util_path_concat (2, platdir, "grub.efi");
+	  grub_install_make_image_wrap (/* source dir  */ grub_install_source_directory,
+					/* prefix */ "",
+					/* output */ dst,
+					/* memdisk */ NULL,
+					have_load_cfg ? load_cfg : NULL,
+					/* image target */ mkimage_target, 0);
+	  free (dst);
+	}
       break;
     case GRUB_INSTALL_PLATFORM_ARM_EFI:
     case GRUB_INSTALL_PLATFORM_ARM64_EFI:
@@ -1703,7 +1728,8 @@ main (int argc, char *argv[])
 						  "boot.img");
 	char *boot_img = grub_util_path_concat (2, platdir,
 					      "boot.img");
-	grub_install_copy_file (boot_img_src, boot_img, 1);
+	if (!only_platform_installation)
+	  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
@@ -1732,7 +1758,8 @@ main (int argc, char *argv[])
 						  "boot.img");
 	char *boot_img = grub_util_path_concat (2, platdir,
 					      "boot.img");
-	grub_install_copy_file (boot_img_src, boot_img, 1);
+        if (!only_platform_installation)
+	  grub_install_copy_file (boot_img_src, boot_img, 1);
 
 	grub_util_info ("%sgrub-sparc64-setup %s %s %s %s --directory='%s' --device-map='%s' '%s'",
 			install_bootsector ? "" : "NOT RUNNING: ",
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-10-28 21:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-21 14:30 Reproducible grub-install Miguel Arruga Vivas
2019-10-23  9:09 ` Daniel Kiper
2019-10-28 11:25   ` Granularity of grub-install (was Re: Reproducibility of grub-install) Miguel Arruga Vivas
2019-10-28 11:27     ` [PATCH] install: Free allocated path for grub.efi Miguel Arruga Vivas
2019-10-28 21:01       ` [PATCH 2/2] install: Add only-platform-installation option Miguel Arruga Vivas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.