* 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.