unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
* bug#44353: guix system disk-image -t raw fails with grub-efi-bootloader
@ 2020-10-31 16:47 Jesse Gibbons
  2020-11-07  7:09 ` Maxim Cournoyer
  0 siblings, 1 reply; 28+ messages in thread
From: Jesse Gibbons @ 2020-10-31 16:47 UTC (permalink / raw)
  To: 44353

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

To replicate:
cd to a git checkout of the source code, gnu/system/examples
The hash for my current checkout is 
d7e033b9a153a9e60f52ff64f4eb355c1c3d0a6e which is also the hash for my 
current version of guix.
guix system disk-image -t raw lightweight-desktop.tmpl

lightweight-desktop.tmpl defines an operating-system that uses 
grub-efi-bootloader, and is embedded in the info pages as an example 
operating system. Since it is lightweight, it doesn't take as long to 
build as desktop.tmpl.

Expected results: guix builds the raw image just fine, and that image 
can be copied to an SD card and run on a computer with an efi 
bootloader, just like the counterpart with grub-bootloader. There is 
nothing in the documentation suggesting that this should not work by design.

Actual results: guix fails, see attached log.

-Jesse

[-- Attachment #2: 1zv1w0vbd4yvnmnnyn06ajkllx21gc-partition.img.drv.bz2 --]
[-- Type: application/octet-stream, Size: 1627 bytes --]

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

* bug#44353: guix system disk-image -t raw fails with grub-efi-bootloader
  2020-10-31 16:47 bug#44353: guix system disk-image -t raw fails with grub-efi-bootloader Jesse Gibbons
@ 2020-11-07  7:09 ` Maxim Cournoyer
  2020-11-07  9:08   ` Mathieu Othacehe
  0 siblings, 1 reply; 28+ messages in thread
From: Maxim Cournoyer @ 2020-11-07  7:09 UTC (permalink / raw)
  To: Jesse Gibbons; +Cc: 44353, Mathieu Othacehe

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

Hello!

Jesse Gibbons <jgibbons2357@gmail.com> writes:

> To replicate:
> cd to a git checkout of the source code, gnu/system/examples
> The hash for my current checkout is
> d7e033b9a153a9e60f52ff64f4eb355c1c3d0a6e which is also the hash for my
> current version of guix.
> guix system disk-image -t raw lightweight-desktop.tmpl
>
> lightweight-desktop.tmpl defines an operating-system that uses
> grub-efi-bootloader, and is embedded in the info pages as an example
> operating system. Since it is lightweight, it doesn't take as long to
> build as desktop.tmpl.
>
> Expected results: guix builds the raw image just fine, and that image
> can be copied to an SD card and run on a computer with an efi
> bootloader, just like the counterpart with grub-bootloader. There is
> nothing in the documentation suggesting that this should not work by
> design.
>
> Actual results: guix fails, see attached log.
>
> -Jesse

Thank you for the report!

I'm CC'ing Mathieu as he knows this part of the code much better than I
do, having overhauled the generation of disk images recently!

Below are two patches I made while investigating this.  Mathieu, does
that look like a proper fix?  It allows images to be generated, but I'm
unsure whether they'd be bootable (I couldn't successfully boot either
EFI or MBR GRUB-based disk images in QEMU).

Anyway, attached are the patches.

Thank you!

Maxim


[-- Attachment #2: 0001-build-image-Ensure-required-arguments-are-present.patch --]
[-- Type: text/x-patch, Size: 2406 bytes --]

From 557a938633ed55c41aed8b41a0f93b6151f00943 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Date: Fri, 6 Nov 2020 21:37:33 -0500
Subject: [PATCH 1/2] build: image: Ensure required arguments are present.

* gnu/build/image.scm (initialize-root-partition): Validate arguments.
[system-directory, references-graph]: Change from keyword to positional
arguments.
[doc]: Add space between sentences.
* gnu/system/image.scm (system-iso9660-image): Adjust.
---
 gnu/build/image.scm | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/gnu/build/image.scm b/gnu/build/image.scm
index 640a784204..053972166b 100644
--- a/gnu/build/image.scm
+++ b/gnu/build/image.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2017 Marius Bakke <mbakke@fastmail.com>
 ;;; Copyright © 2020 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2020 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -179,7 +180,7 @@ to call-with-database."
                                     make-device-nodes
                                     (wal-mode? #t)
                                     #:allow-other-keys)
-  "Initialize the given ROOT directory. Use BOOTCFG and BOOTCFG-LOCATION to
+  "Initialize the given ROOT directory.  Use BOOTCFG and BOOTCFG-LOCATION to
 install the bootloader configuration.
 
 If REGISTER-CLOSURES? is true, register REFERENCES-GRAPHS in the store.  If
@@ -187,6 +188,20 @@ DEDUPLICATE? is true, then also deduplicate files common to CLOSURES and the
 rest of the store when registering the closures.  SYSTEM-DIRECTORY is the name
 of the directory of the 'system' derivation.  Pass WAL-MODE? to
 register-closure."
+
+  (define (validate-arguments)
+    (when bootcfg
+      (unless bootcfg-location
+        (error "Missing 'bootcfg-location' argument")))
+    (when bootloader-installer
+      (unless bootloader-package
+        (error "Missing 'bootloader-package' argument")))
+    (unless references-graphs
+      (error "Missing 'references-graphs' argument"))
+    (unless system-directory
+      (error "Missing 'system-directory' argument")))
+
+  (validate-arguments)
   (populate-root-file-system system-directory root)
   (populate-store references-graphs root)
 
-- 
2.28.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-image-Only-attempt-bootloader-installation-when-supp.patch --]
[-- Type: text/x-patch, Size: 2106 bytes --]

From c642a6984d4961256f50ca5133d1eec5ca1af4e9 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Date: Sat, 7 Nov 2020 00:20:49 -0500
Subject: [PATCH 2/2] image: Only attempt bootloader installation when
 supported.

Fixes <https://issues.guix.gnu.org/44353>.

The initialize-root-partition in (guix build image) attempts to install a
bootloader when BOOTLOADER-INSTALLER is defined.  Unfortunately, it relies on
a special #f value for the second argument (device), which only the
INSTALL-GRUB procedure from (gnu bootloader grub) supports.

* gnu/system/image.scm (system-disk-image): Only pass the bootloader installer
argument when the bootloader name is 'grub.  Add comment.
---
 gnu/system/image.scm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/gnu/system/image.scm b/gnu/system/image.scm
index 4075a26552..d4932466e5 100644
--- a/gnu/system/image.scm
+++ b/gnu/system/image.scm
@@ -345,8 +345,16 @@ used in the image."
                               #:grub-efi #+grub-efi
                               #:bootloader-package
                               #+(bootloader-package bootloader)
+                              ;; Special case: most bootloaders can be copied
+                              ;; directly at some fixed location on the image
+                              ;; disk, but when installed to the master boot
+                              ;; record (MBR), GRUB requires support files
+                              ;; present under /boot/grub, which is handled by
+                              ;; its 'installer' procedure.
                               #:bootloader-installer
-                              #+(bootloader-installer bootloader)
+                              #+(if (eq? 'grub (bootloader-name bootloader))
+                                    (bootloader-installer bootloader)
+                                    #f)
                               #:bootcfg #$bootcfg
                               #:bootcfg-location
                               #$(bootloader-configuration-file bootloader))
-- 
2.28.0


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

* bug#44353: guix system disk-image -t raw fails with grub-efi-bootloader
  2020-11-07  7:09 ` Maxim Cournoyer
@ 2020-11-07  9:08   ` Mathieu Othacehe
  2020-11-07 11:26     ` Bengt Richter
  2020-11-07 20:32     ` Maxim Cournoyer
  0 siblings, 2 replies; 28+ messages in thread
From: Mathieu Othacehe @ 2020-11-07  9:08 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 44353, Jesse Gibbons

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


Hello Maxim,

Thanks for your patch! It's hard to provide a reliable abstraction on
top of all the exotic bootloader installation methods existing.

Currently, each bootloader implements two methods:

* bootloader-installer
* bootloader-disk-image-installer

The first one is dedicated to the installation of the bootloader on a
mounted directory, while the second one is meant to work on a disk
device such as "/dev/sda" or directly on a disk-image.

When producing a disk-image with the "raw" type, we are always creating
and populating an ESP partition (see raw-image-type), regardless of the
selected bootloader. In fact, "raw" should be renamed to "hybrid-efi".

The produced image can work on machines with legacy mbr boot or with EFI
boot. Hence, calling "install-grub-efi" like it's done while building
the lightweight-desktop operating-system is useless and fails. The
attached patch fixes it.

You can test it with:

--8<---------------cut here---------------start------------->8---
qemu-system-x86_64 -m 1024 -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin -hda disk.img
--8<---------------cut here---------------end--------------->8---

> +                              ;; Special case: most bootloaders can be copied
> +                              ;; directly at some fixed location on the image
> +                              ;; disk, but when installed to the master boot
> +                              ;; record (MBR), GRUB requires support files
> +                              ;; present under /boot/grub, which is handled by
> +                              ;; its 'installer' procedure.
>                                #:bootloader-installer
> -                              #+(bootloader-installer bootloader)
> +                              #+(if (eq? 'grub (bootloader-name bootloader))
> +                                    (bootloader-installer bootloader)
> +                                    #f)

That would prevent other bootloaders relying on this procedure to work,
such as extlinux.

Thanks,

Mathieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-bootloader-grub-Fix-EFI-installation.patch --]
[-- Type: text/x-diff, Size: 3073 bytes --]

From 2145fc40d3eb949885ce94883b09c7291c607be6 Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <othacehe@gnu.org>
Date: Sat, 7 Nov 2020 10:03:53 +0100
Subject: [PATCH] bootloader: grub: Fix EFI installation.

When producing a disk-image, "install-grub-efi" will be called with EFI-DIR
set to false.  The EFI bootloader is already installed by
"initialize-efi-partition". In that case, do not proceed to bootloader
installation.

Fixes: <https://issues.guix.gnu.org/44353>.

* gnu/bootloader/grub.scm (install-grub-efi): Do not install the bootloader if
EFI-DIR is false.
---
 gnu/bootloader/grub.scm | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index 0899fab61f..3404456df9 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -574,20 +574,24 @@ fi~%"))))
 (define install-grub-efi
   #~(lambda (bootloader efi-dir mount-point)
       ;; Install GRUB onto the EFI partition mounted at EFI-DIR, for the
-      ;; system whose root is mounted at MOUNT-POINT.
-      (let ((grub-install (string-append bootloader "/sbin/grub-install"))
-            (install-dir (string-append mount-point "/boot"))
-            ;; When installing Guix, it's common to mount EFI-DIR below
-            ;; MOUNT-POINT rather than /boot/efi on the live image.
-            (target-esp (if (file-exists? (string-append mount-point efi-dir))
-                            (string-append mount-point efi-dir)
-                            efi-dir)))
-        ;; Tell 'grub-install' that there might be a LUKS-encrypted /boot or
-        ;; root partition.
-        (setenv "GRUB_ENABLE_CRYPTODISK" "y")
-        (invoke/quiet grub-install "--boot-directory" install-dir
-                      "--bootloader-id=Guix"
-                      "--efi-directory" target-esp))))
+      ;; system whose root is mounted at MOUNT-POINT.  When producing a
+      ;; disk-image, EFI-DIR is false and the EFI bootloader is already
+      ;; installed using "initialize-efi-partition".
+      (when efi-dir
+        (let ((grub-install (string-append bootloader "/sbin/grub-install"))
+              (install-dir (string-append mount-point "/boot"))
+              ;; When installing Guix, it's common to mount EFI-DIR below
+              ;; MOUNT-POINT rather than /boot/efi on the live image.
+              (target-esp (if (file-exists?
+                               (string-append mount-point efi-dir))
+                              (string-append mount-point efi-dir)
+                              efi-dir)))
+          ;; Tell 'grub-install' that there might be a LUKS-encrypted /boot or
+          ;; root partition.
+          (setenv "GRUB_ENABLE_CRYPTODISK" "y")
+          (invoke/quiet grub-install "--boot-directory" install-dir
+                        "--bootloader-id=Guix"
+                        "--efi-directory" target-esp)))))
 
 (define (install-grub-efi-netboot subdir)
   "Define a grub-efi-netboot bootloader installer for installation in SUBDIR,
-- 
2.29.2


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

* bug#44353: guix system disk-image -t raw fails with grub-efi-bootloader
  2020-11-07  9:08   ` Mathieu Othacehe
@ 2020-11-07 11:26     ` Bengt Richter
  2020-11-07 20:32     ` Maxim Cournoyer
  1 sibling, 0 replies; 28+ messages in thread
From: Bengt Richter @ 2020-11-07 11:26 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 44353, Jesse Gibbons, Maxim Cournoyer

Hi Mathieu,

On +2020-11-07 10:08:36 +0100, Mathieu Othacehe wrote:
> 
> Hello Maxim,
> 
> Thanks for your patch! It's hard to provide a reliable abstraction on
> top of all the exotic bootloader installation methods existing.
> 
> Currently, each bootloader implements two methods:
> 
> * bootloader-installer
> * bootloader-disk-image-installer
> 
> The first one is dedicated to the installation of the bootloader on a
> mounted directory, while the second one is meant to work on a disk
> device such as "/dev/sda" or directly on a disk-image.
> 
> When producing a disk-image with the "raw" type, we are always creating
> and populating an ESP partition (see raw-image-type), regardless of the
> selected bootloader. In fact, "raw" should be renamed to "hybrid-efi".
> 
> The produced image can work on machines with legacy mbr boot or with EFI
> boot. Hence, calling "install-grub-efi" like it's done while building
> the lightweight-desktop operating-system is useless and fails. The
> attached patch fixes it.
> 
> You can test it with:
> 
> --8<---------------cut here---------------start------------->8---
> qemu-system-x86_64 -m 1024 -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin -hda disk.img
> --8<---------------cut here---------------end--------------->8---
> 
> > +                              ;; Special case: most bootloaders can be copied
> > +                              ;; directly at some fixed location on the image
> > +                              ;; disk, but when installed to the master boot
> > +                              ;; record (MBR), GRUB requires support files
> > +                              ;; present under /boot/grub, which is handled by
> > +                              ;; its 'installer' procedure.
> >                                #:bootloader-installer
> > -                              #+(bootloader-installer bootloader)
> > +                              #+(if (eq? 'grub (bootloader-name bootloader))
> > +                                    (bootloader-installer bootloader)
> > +                                    #f)
> 
> That would prevent other bootloaders relying on this procedure to work,
> such as extlinux.
> 
> Thanks,
> 
> Mathieu

Given that writing to "raw" disks is something the dd command is often used for,
how much trouble would it be to provide an option for bootloader-disk-image-installer
to output a shell script with the necessary dd commands, instead of doing the raw writing itself?

I am imagining a tarball with separate binary block-sequence file images for the GPT or MBR
header blocks, the embedded boot loader or UEFI partition image, and root partition
etc..

I think the block-sequence images can be sliced out of the backing file of a loopback mount that
fdisk and mkfs.* can format as desired, unless I'm missing something.

I would like the script to use lsblk -o model,serial to identify the raw disk to write to,
so there is no shooting the wrong foot ;)

This is sketchy on the details, but ISTM a tarball like this would make it easy to prepare
a USB-accessible disk using any laptop that was in a state where it was trusted to do
sudo dd ... right.

If the laptop didn't have all the tools, perhaps a minimal static busybox could be included
in the tarball to guarantee existence of the dd and lsblk tools etc.

There might be some file content that needs to be written with file i/o after dd has written
the content-initialized monolith file system images. This could be interactive choices of alternate
hostname, passwords, or whatever.

Remember, this script is not burning a bootable installer (though it could), it is burning
the bootable system an installer would install.

The point of this is that it happens as the script with the dd commands executes on an arbitrary
laptop with a raw USB disk attached, not as initialization dialog on first boot of the system
whose image is being written to the USB disk.

Obviously all files should be verifiable one way or another.

Hopefully it would also make it easier to share/generate system images for raspberries or RISC-V ARM, etc.

I guess you could call this a shell-script derivation, meant to talk to bash/dd instead of the guix daemon.

Has anyone done this kind of factoring already?

TIA for comment :)
-- 
Regards,
Bengt Richter




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

* bug#44353: guix system disk-image -t raw fails with grub-efi-bootloader
  2020-11-07  9:08   ` Mathieu Othacehe
  2020-11-07 11:26     ` Bengt Richter
@ 2020-11-07 20:32     ` Maxim Cournoyer
  2020-11-08 11:04       ` Mathieu Othacehe
  1 sibling, 1 reply; 28+ messages in thread
From: Maxim Cournoyer @ 2020-11-07 20:32 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 44353, Jesse Gibbons

Hello Mathieu!

Mathieu Othacehe <othacehe@gnu.org> writes:

> Hello Maxim,
>
> Thanks for your patch! It's hard to provide a reliable abstraction on
> top of all the exotic bootloader installation methods existing.
>
> Currently, each bootloader implements two methods:
>
> * bootloader-installer
> * bootloader-disk-image-installer
>
> The first one is dedicated to the installation of the bootloader on a
> mounted directory, while the second one is meant to work on a disk
> device such as "/dev/sda" or directly on a disk-image.
>
> When producing a disk-image with the "raw" type, we are always creating
> and populating an ESP partition (see raw-image-type), regardless of the
> selected bootloader. In fact, "raw" should be renamed to "hybrid-efi".
>
> The produced image can work on machines with legacy mbr boot or with EFI
> boot. Hence, calling "install-grub-efi" like it's done while building
> the lightweight-desktop operating-system is useless and fails. The
> attached patch fixes it.

Thank you for the clarifications!

> You can test it with:
>
> qemu-system-x86_64 -m 1024 -bios $(guix build
> ovmf)/share/firmware/ovmf_x64.bin -hda disk.img

Thank you!  I hadn't realized that the default SeaBIOS BIOS used by QEMU
wasn't EFI-capable!  The image now boots, but fails bringing up its file
systems for some reason.  Perhaps I'm supposed to edit the file system
definitions of the template before use?

Here's how I tested it:

$ time ./pre-inst-env guix system disk-image
gnu/system/examples/lightweight-desktop.tmpl --verbosity=2 --no-offload

That took 38 minutes on my system and produced /gnu/store/...-disk-image.

$ cp /gnu/store/...-disk-image /tmp/lightweight-desktop.img

$ chmod +rw /tmp/lightweight-desktop.img

Then I tried running it with the suggested command:

$ qemu-system-x86_64 -m 1024 -bios $(guix build
ovmf)/share/firmware/ovmf_x64.bin -hda /tmp/lightweight-desktop.img

>> +                              ;; Special case: most bootloaders can be copied
>> +                              ;; directly at some fixed location on the image
>> +                              ;; disk, but when installed to the master boot
>> +                              ;; record (MBR), GRUB requires support files
>> +                              ;; present under /boot/grub, which is handled by
>> +                              ;; its 'installer' procedure.
>>                                #:bootloader-installer
>> -                              #+(bootloader-installer bootloader)
>> +                              #+(if (eq? 'grub (bootloader-name bootloader))
>> +                                    (bootloader-installer bootloader)
>> +                                    #f)
>
> That would prevent other bootloaders relying on this procedure to work,
> such as extlinux.

I pondered about such solution, but when I verified the bootloaders
under gnu/bootloaders, it appeared that *only* the install-grub
procedure supported being passed #f as its mount point; the others would
fail because of the unexpected #f boolean value.

For example, the install-extlinux procedure from (gnu bootloaders
extlinux):

--8<---------------cut here---------------start------------->8---
(define (install-extlinux mbr)
  #~(lambda (bootloader device mount-point)
      (let ((extlinux (string-append bootloader "/sbin/extlinux"))
            (install-dir (string-append mount-point "/boot/extlinux"))
            (syslinux-dir (string-append bootloader "/share/syslinux")))
        (for-each (lambda (file)
                    (install-file file install-dir))
                  (find-files syslinux-dir "\\.c32$"))
        (invoke/quiet extlinux "--install" install-dir)
        (write-file-on-device (string-append syslinux-dir "/" #$mbr)
                              440 device 0))))
--8<---------------cut here---------------end--------------->8---

Would break if device was set to #f, as is done in (guix build image)'s
initialize-efi-partition:

--8<---------------cut here---------------start------------->8---
  (when bootloader-installer
    (display "installing bootloader...\n")
    (bootloader-installer bootloader-package #f root))
--8<---------------cut here---------------end--------------->8---

Is my analysis correct?  If so, the patch I sent yesterday would fix the
problem more thoroughly.

Thank you,

Maxim




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

* bug#44353: guix system disk-image -t raw fails with grub-efi-bootloader
  2020-11-07 20:32     ` Maxim Cournoyer
@ 2020-11-08 11:04       ` Mathieu Othacehe
  2020-11-12  3:57         ` bug#44353: [PATCH version-1.2.0 v2 1/3] image: Remove conflicting user-provided EFI file system Maxim Cournoyer
  2020-11-12  7:09         ` bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image Maxim Cournoyer
  0 siblings, 2 replies; 28+ messages in thread
From: Mathieu Othacehe @ 2020-11-08 11:04 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 44353, Jesse Gibbons


Hey,

> $ time ./pre-inst-env guix system disk-image
> gnu/system/examples/lightweight-desktop.tmpl --verbosity=2 --no-offload
>
> That took 38 minutes on my system and produced /gnu/store/...-disk-image.

Strange, only 8 minutes for me. Does it include the time necessary to
fetch all substitutes? Are you using a SSD drive?

> $ qemu-system-x86_64 -m 1024 -bios $(guix build
> ovmf)/share/firmware/ovmf_x64.bin -hda /tmp/lightweight-desktop.img

It tries to mount the EFI file-system with UUID "1234-ABCD" and
fails. You can remove this one for the lightweight-desktop configuration
to obtain a bootable system.

> Would break if device was set to #f, as is done in (guix build image)'s
> initialize-efi-partition:
>
>   (when bootloader-installer
>     (display "installing bootloader...\n")
>     (bootloader-installer bootloader-package #f root))
>
> Is my analysis correct?  If so, the patch I sent yesterday would fix the
> problem more thoroughly.

Yes it is probably broken too. However, I would prefer not to introduce
bootloader specific stuff in (gnu system image).

I think the bootloaders should try to do their best with the DEVICE and
MOUNT-POINT arguments passed to bootloader-installer procedure. It
means, trying to install themselves using only MOUNT-POINT argument or
bailing out.

WDYT?

Thanks,

Mathieu




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

* bug#44353: [PATCH version-1.2.0 v2 1/3] image: Remove conflicting user-provided EFI file system.
  2020-11-08 11:04       ` Mathieu Othacehe
@ 2020-11-12  3:57         ` Maxim Cournoyer
  2020-11-12  3:57           ` bug#44353: [PATCH version-1.2.0 v2 2/3] bootloader: grub: Skip install-grub-efi when producing a disk image Maxim Cournoyer
                             ` (2 more replies)
  2020-11-12  7:09         ` bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image Maxim Cournoyer
  1 sibling, 3 replies; 28+ messages in thread
From: Maxim Cournoyer @ 2020-11-12  3:57 UTC (permalink / raw)
  To: 44353; +Cc: Mathieu Othacehe, Jesse Gibbons, Maxim Cournoyer

When the bootloader used is grub-efi-bootloader, an ESP partition and file
system is already added by the image generator.  If a conflicting
user-provided EFI file system is provided, it will (likely) cause the boot to
fail, as happens for the lightweight-desktop.tmpl and desktop.tmpl templates
under gnu/system/examples.

* gnu/system/image.scm (operating-system-for-image): Also remove file systems
whose mount point is "/boot/efi".
---
 gnu/system/image.scm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gnu/system/image.scm b/gnu/system/image.scm
index 81152f0fc4..4972d9067b 100644
--- a/gnu/system/image.scm
+++ b/gnu/system/image.scm
@@ -576,7 +576,9 @@ it can be used for bootloading."
          (file-systems-to-keep
           (srfi-1:remove
            (lambda (fs)
-             (string=? (file-system-mount-point fs) "/"))
+             (let ((mount-point (file-system-mount-point fs)))
+               (or (string=? mount-point "/")
+                   (string=? mount-point "/boot/efi"))))
            (operating-system-file-systems base-os)))
          (format (image-format image))
          (os
-- 
2.28.0





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

* bug#44353: [PATCH version-1.2.0 v2 2/3] bootloader: grub: Skip install-grub-efi when producing a disk image.
  2020-11-12  3:57         ` bug#44353: [PATCH version-1.2.0 v2 1/3] image: Remove conflicting user-provided EFI file system Maxim Cournoyer
@ 2020-11-12  3:57           ` Maxim Cournoyer
  2020-11-12  8:42             ` Mathieu Othacehe
  2020-11-12  3:57           ` bug#44353: [PATCH version-1.2.0 v2 3/3] doc: Detail which bootloader get used with disk-image or vm-image Maxim Cournoyer
  2020-11-12  8:39           ` bug#44353: [PATCH version-1.2.0 v2 1/3] image: Remove conflicting user-provided EFI file system Mathieu Othacehe
  2 siblings, 1 reply; 28+ messages in thread
From: Maxim Cournoyer @ 2020-11-12  3:57 UTC (permalink / raw)
  To: 44353; +Cc: Mathieu Othacehe, Jesse Gibbons, Maxim Cournoyer

Fixes <http://issues.guix.gnu.org/44353>.

Every bootloader should try their best to install themselves using only the
MOUNT-POINT and otherwise do nothing.  This requirement comes from the
necessity to call INSTALL-GRUB when installing the (non-EFI) GRUB bootloader,
which needs to populate the root file system with extra modules that cannot be
fit in the core.img file, limited in size to 491520 bytes (by the i386-pc
format required for legacy BIOS compatibility).

As introducing bootloader knowledge at the level of the image code is
undesirable, every bootloader should be adapted to support this fall-back for
their installation procedure (TODO).

* gnu/bootloader/grub.scm (install-grub-efi)[efi-dir]: Skip when the EFI-DIR
argument is set to #f.
---
 gnu/bootloader/grub.scm | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index 0899fab61f..af7b7561ff 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -573,21 +573,24 @@ fi~%"))))
 
 (define install-grub-efi
   #~(lambda (bootloader efi-dir mount-point)
-      ;; Install GRUB onto the EFI partition mounted at EFI-DIR, for the
-      ;; system whose root is mounted at MOUNT-POINT.
-      (let ((grub-install (string-append bootloader "/sbin/grub-install"))
-            (install-dir (string-append mount-point "/boot"))
-            ;; When installing Guix, it's common to mount EFI-DIR below
-            ;; MOUNT-POINT rather than /boot/efi on the live image.
-            (target-esp (if (file-exists? (string-append mount-point efi-dir))
-                            (string-append mount-point efi-dir)
-                            efi-dir)))
-        ;; Tell 'grub-install' that there might be a LUKS-encrypted /boot or
-        ;; root partition.
-        (setenv "GRUB_ENABLE_CRYPTODISK" "y")
-        (invoke/quiet grub-install "--boot-directory" install-dir
-                      "--bootloader-id=Guix"
-                      "--efi-directory" target-esp))))
+      ;; There is nothing useful to do when called in the context of a disk
+      ;; image generation.
+      (when efi-dir
+        ;; Install GRUB onto the EFI partition mounted at EFI-DIR, for the
+        ;; system whose root is mounted at MOUNT-POINT.
+        (let ((grub-install (string-append bootloader "/sbin/grub-install"))
+              (install-dir (string-append mount-point "/boot"))
+              ;; When installing Guix, it's common to mount EFI-DIR below
+              ;; MOUNT-POINT rather than /boot/efi on the live image.
+              (target-esp (if (file-exists? (string-append mount-point efi-dir))
+                              (string-append mount-point efi-dir)
+                              efi-dir)))
+          ;; Tell 'grub-install' that there might be a LUKS-encrypted /boot or
+          ;; root partition.
+          (setenv "GRUB_ENABLE_CRYPTODISK" "y")
+          (invoke/quiet grub-install "--boot-directory" install-dir
+                        "--bootloader-id=Guix"
+                        "--efi-directory" target-esp)))))
 
 (define (install-grub-efi-netboot subdir)
   "Define a grub-efi-netboot bootloader installer for installation in SUBDIR,
-- 
2.28.0





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

* bug#44353: [PATCH version-1.2.0 v2 3/3] doc: Detail which bootloader get used with disk-image or vm-image.
  2020-11-12  3:57         ` bug#44353: [PATCH version-1.2.0 v2 1/3] image: Remove conflicting user-provided EFI file system Maxim Cournoyer
  2020-11-12  3:57           ` bug#44353: [PATCH version-1.2.0 v2 2/3] bootloader: grub: Skip install-grub-efi when producing a disk image Maxim Cournoyer
@ 2020-11-12  3:57           ` Maxim Cournoyer
  2020-11-12  8:45             ` Mathieu Othacehe
  2020-11-12  8:39           ` bug#44353: [PATCH version-1.2.0 v2 1/3] image: Remove conflicting user-provided EFI file system Mathieu Othacehe
  2 siblings, 1 reply; 28+ messages in thread
From: Maxim Cournoyer @ 2020-11-12  3:57 UTC (permalink / raw)
  To: 44353; +Cc: Mathieu Othacehe, Jesse Gibbons, Maxim Cournoyer

* doc/guix.texi (Invoking guix system): Extend doc.
---
 doc/guix.texi | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index b7f1bc1f00..e15ee4092c 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -30909,11 +30909,25 @@ a value.  Docker images are built to contain exactly what they need, so
 the @option{--image-size} option is ignored in the case of
 @code{docker-image}.
 
+@cindex disk-image, creating disk images
 The @code{disk-image} command can produce various image types.  The
 image type can be selected using the @command{--image-type} option.  It
-defaults to @code{raw}. When its value is @code{iso9660}, the
+defaults to @code{raw}.  When its value is @code{iso9660}, the
 @option{--label} option can be used to specify a volume ID with
-@code{disk-image}.
+@code{disk-image}.  When using @code{disk-image}, the bootloader
+installed on the generated image is taken from the provided
+@code{operating-system} definition.  The following example demonstrates
+how to generate an image that uses the @code{grub-efi-bootloader}
+bootloader and boot it with QEMU:
+
+@example
+image=$(guix system disk-image --image-type=qcow2 \
+             gnu/system/examples/lightweight-desktop.tmpl)
+cp $image /tmp/my-image.qcow2
+chmod +rw /tmp/my-image.qcow2
+qemu-system-x86_64 -enable-kvm -hda /tmp/my-image.qcow2 -m 1000
+                   -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin
+@end example
 
 When using the @code{raw} image type, a raw disk image is produced; it
 can be copied as is to a USB stick, for instance.  Assuming
@@ -30927,10 +30941,17 @@ the image to it using the following command:
 The @code{--list-image-types} command lists all the available image
 types.
 
+@cindex vm-image, creating virtual machine images
 When using @code{vm-image}, the returned image is in qcow2 format, which
-the QEMU emulator can efficiently use. @xref{Running Guix in a VM},
-for more information on how to run the image in a virtual machine.
-
+the QEMU emulator can efficiently use. @xref{Running Guix in a VM}, for
+more information on how to run the image in a virtual machine.  The
+@code{grub-bootloader} bootloader is always used independently of what
+is declared in the @code{operating-system} file passed as argument.
+This is to make it easier to work with QEMU, which uses the SeaBIOS BIOS
+by default, expecting a bootloader to be installed in the Master Boot
+Record (MBR).
+
+@cindex docker-image, creating docker images
 When using @code{docker-image}, a Docker image is produced.  Guix builds
 the image from scratch, not from a pre-existing Docker base image.  As a
 result, it contains @emph{exactly} what you define in the operating
-- 
2.28.0





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

* bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image.
  2020-11-08 11:04       ` Mathieu Othacehe
  2020-11-12  3:57         ` bug#44353: [PATCH version-1.2.0 v2 1/3] image: Remove conflicting user-provided EFI file system Maxim Cournoyer
@ 2020-11-12  7:09         ` Maxim Cournoyer
  2020-11-12  8:36           ` Mathieu Othacehe
  2020-11-15 21:45           ` Ludovic Courtès
  1 sibling, 2 replies; 28+ messages in thread
From: Maxim Cournoyer @ 2020-11-12  7:09 UTC (permalink / raw)
  To: 44353; +Cc: Mathieu Othacehe, Jesse Gibbons, Maxim Cournoyer

* guix/scripts/system.scm (%options)[volatile-root?]: New boolean option.
(%default-options): Set its default value to #f.
(show-help): Add help doc.
* guix/scripts/system.scm (perform-action): Propagate option...
(system-derivation-for-action): ...here.  Use it to set the volatile-root?
field of the image object passed to SYSTEM-IMAGE.
* doc/guix.texi (Invoking guix system): Document it.
---
 doc/guix.texi           | 18 ++++++++++++------
 guix/scripts/system.scm | 21 +++++++++++++++++----
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index e15ee4092c..30efb7fc97 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -30911,14 +30911,20 @@ the @option{--image-size} option is ignored in the case of
 
 @cindex disk-image, creating disk images
 The @code{disk-image} command can produce various image types.  The
-image type can be selected using the @command{--image-type} option.  It
+image type can be selected using the @option{--image-type} option.  It
 defaults to @code{raw}.  When its value is @code{iso9660}, the
 @option{--label} option can be used to specify a volume ID with
-@code{disk-image}.  When using @code{disk-image}, the bootloader
-installed on the generated image is taken from the provided
-@code{operating-system} definition.  The following example demonstrates
-how to generate an image that uses the @code{grub-efi-bootloader}
-bootloader and boot it with QEMU:
+@code{disk-image}.  By default, the root file system of a disk image is
+mounted volatile; the @option{--non-volatile} option can be used to make
+it persistent instead.  The @option{--non-volatile} is useful to make
+use of extra disk space that can be obtained by using a larger value for
+the @option{--image-size} option; otherwise, the amount of physical
+memory available determines the capacity of the volatile file system
+overlay used for the root file system.  When using @code{disk-image},
+the bootloader installed on the generated image is taken from the
+provided @code{operating-system} definition.  The following example
+demonstrates how to generate an image that uses the
+@code{grub-efi-bootloader} bootloader and boot it with QEMU:
 
 @example
 image=$(guix system disk-image --image-type=qcow2 \
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index ad998156c2..004f1c9739 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -674,7 +674,8 @@ checking this by themselves in their 'check' procedure."
 (define* (system-derivation-for-action os action
                                        #:key image-size image-type
                                        full-boot? container-shared-network?
-                                       mappings label)
+                                       mappings label
+                                       volatile-root?)
   "Return as a monadic value the derivation for OS according to ACTION."
   (mlet %store-monad ((target (current-target-system)))
     (case action
@@ -706,7 +707,8 @@ checking this by themselves in their 'check' procedure."
                          base-image))
             (target (or base-target target))
             (size image-size)
-            (operating-system os))))))
+            (operating-system os)
+            (volatile-root? volatile-root?))))))
       ((docker-image)
        (system-docker-image os
                             #:shared-network? container-shared-network?)))))
@@ -761,6 +763,7 @@ and TARGET arguments."
                          dry-run? derivations-only?
                          use-substitutes? bootloader-target target
                          image-size image-type
+                         volatile-root?
                          full-boot? label container-shared-network?
                          (mappings '())
                          (gc-root #f))
@@ -768,7 +771,8 @@ and TARGET arguments."
 bootloader; BOOTLOADER-TAGET is the target for the bootloader; TARGET is the
 target root directory; IMAGE-SIZE is the size of the image to be built, for
 the 'vm-image' and 'disk-image' actions.  IMAGE-TYPE is the type of image to
-be built.
+be built.  When VOLATILE-ROOT? is #t, the root file system is mounted
+volatile.
 
 FULL-BOOT? is used for the 'vm' action; it determines whether to
 boot directly to the kernel or to the bootloader.  CONTAINER-SHARED-NETWORK?
@@ -816,6 +820,7 @@ static checks."
                                                 #:label label
                                                 #:image-type image-type
                                                 #:image-size image-size
+                                                #:volatile-root? volatile-root?
                                                 #:full-boot? full-boot?
                                                 #:container-shared-network? container-shared-network?
                                                 #:mappings mappings))
@@ -974,6 +979,8 @@ Some ACTIONS support additional ARGS.\n"))
       --image-size=SIZE  for 'vm-image', produce an image of SIZE"))
   (display (G_ "
       --no-bootloader    for 'init', do not install a bootloader"))
+  (display (G_ "
+      --non-volatile     for 'disk-image', persist root file system changes"))
   (display (G_ "
       --label=LABEL      for 'disk-image', label disk image with LABEL"))
   (display (G_ "
@@ -1048,6 +1055,9 @@ Some ACTIONS support additional ARGS.\n"))
          (option '("no-bootloader" "no-grub") #f #f
                  (lambda (opt name arg result)
                    (alist-cons 'install-bootloader? #f result)))
+         (option '("non-volatile") #f #f
+                 (lambda (opt name arg result)
+                   (alist-cons 'volatile-root? #f result)))
          (option '("label") #t #f
                  (lambda (opt name arg result)
                    (alist-cons 'label arg result)))
@@ -1109,7 +1119,8 @@ Some ACTIONS support additional ARGS.\n"))
     (image-type . raw)
     (image-size . guess)
     (install-bootloader? . #t)
-    (label . #f)))
+    (label . #f)
+    (volatile-root? . #t)))
 
 (define (verbosity-level opts)
   "Return the verbosity level based on OPTS, the alist of parsed options."
@@ -1206,6 +1217,8 @@ resulting from command-line parsing."
                                #:image-type (lookup-image-type-by-name
                                              (assoc-ref opts 'image-type))
                                #:image-size (assoc-ref opts 'image-size)
+                               #:volatile-root?
+                               (assoc-ref opts 'volatile-root?)
                                #:full-boot? (assoc-ref opts 'full-boot?)
                                #:container-shared-network?
                                (assoc-ref opts 'container-shared-network?)
-- 
2.28.0





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

* bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image.
  2020-11-12  7:09         ` bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image Maxim Cournoyer
@ 2020-11-12  8:36           ` Mathieu Othacehe
  2020-11-12 14:59             ` Maxim Cournoyer
  2020-11-12 21:18             ` Ludovic Courtès
  2020-11-15 21:45           ` Ludovic Courtès
  1 sibling, 2 replies; 28+ messages in thread
From: Mathieu Othacehe @ 2020-11-12  8:36 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 44353, Jesse Gibbons


Hello Maxim,

> * guix/scripts/system.scm (%options)[volatile-root?]: New boolean option.
> (%default-options): Set its default value to #f.
> (show-help): Add help doc.
> * guix/scripts/system.scm (perform-action): Propagate option...
> (system-derivation-for-action): ...here.  Use it to set the volatile-root?
> field of the image object passed to SYSTEM-IMAGE.
> * doc/guix.texi (Invoking guix system): Document it.

This is a nice addition and it looks good to me.

Thanks,

Mathieu




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

* bug#44353: [PATCH version-1.2.0 v2 1/3] image: Remove conflicting user-provided EFI file system.
  2020-11-12  3:57         ` bug#44353: [PATCH version-1.2.0 v2 1/3] image: Remove conflicting user-provided EFI file system Maxim Cournoyer
  2020-11-12  3:57           ` bug#44353: [PATCH version-1.2.0 v2 2/3] bootloader: grub: Skip install-grub-efi when producing a disk image Maxim Cournoyer
  2020-11-12  3:57           ` bug#44353: [PATCH version-1.2.0 v2 3/3] doc: Detail which bootloader get used with disk-image or vm-image Maxim Cournoyer
@ 2020-11-12  8:39           ` Mathieu Othacehe
  2020-11-17 18:37             ` Maxim Cournoyer
  2 siblings, 1 reply; 28+ messages in thread
From: Mathieu Othacehe @ 2020-11-12  8:39 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 44353, Jesse Gibbons


Hey Maxim,

> When the bootloader used is grub-efi-bootloader, an ESP partition and file
> system is already added by the image generator.  If a conflicting
> user-provided EFI file system is provided, it will (likely) cause the boot to
> fail, as happens for the lightweight-desktop.tmpl and desktop.tmpl templates
> under gnu/system/examples.

Nice one. I would just clarify in the commit message that the ESP
partition is added only when the selected image type is "raw".

Otherwise, it looks fine!

Thanks,

Mathieu




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

* bug#44353: [PATCH version-1.2.0 v2 2/3] bootloader: grub: Skip install-grub-efi when producing a disk image.
  2020-11-12  3:57           ` bug#44353: [PATCH version-1.2.0 v2 2/3] bootloader: grub: Skip install-grub-efi when producing a disk image Maxim Cournoyer
@ 2020-11-12  8:42             ` Mathieu Othacehe
  2020-11-17 18:29               ` Maxim Cournoyer
  0 siblings, 1 reply; 28+ messages in thread
From: Mathieu Othacehe @ 2020-11-12  8:42 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 44353, Jesse Gibbons


Hello Maxim,

> As introducing bootloader knowledge at the level of the image code is
> undesirable, every bootloader should be adapted to support this fall-back for
> their installation procedure (TODO).

Or maybe find a way to remove the need to call "bootloader-installer"
when producing a disk-image as we discussed on IRC.

Looks fine!

Thanks,

Mathieu




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

* bug#44353: [PATCH version-1.2.0 v2 3/3] doc: Detail which bootloader get used with disk-image or vm-image.
  2020-11-12  3:57           ` bug#44353: [PATCH version-1.2.0 v2 3/3] doc: Detail which bootloader get used with disk-image or vm-image Maxim Cournoyer
@ 2020-11-12  8:45             ` Mathieu Othacehe
  2020-11-12 21:16               ` Ludovic Courtès
  0 siblings, 1 reply; 28+ messages in thread
From: Mathieu Othacehe @ 2020-11-12  8:45 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 44353, Jesse Gibbons


Hey,

> +@example
> +image=$(guix system disk-image --image-type=qcow2 \
> +             gnu/system/examples/lightweight-desktop.tmpl)
> +cp $image /tmp/my-image.qcow2
> +chmod +rw /tmp/my-image.qcow2
> +qemu-system-x86_64 -enable-kvm -hda /tmp/my-image.qcow2 -m 1000
> +                   -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin
> +@end example

This looks good! Note that I plan to remove the "vm-image" command and
rename "disk-image" command to "image" once the 1.2.0 is out.

Thanks,

Mathieu




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

* bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image.
  2020-11-12  8:36           ` Mathieu Othacehe
@ 2020-11-12 14:59             ` Maxim Cournoyer
  2020-11-12 17:06               ` Mathieu Othacehe
  2020-11-12 21:18             ` Ludovic Courtès
  1 sibling, 1 reply; 28+ messages in thread
From: Maxim Cournoyer @ 2020-11-12 14:59 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 44353, Jesse Gibbons

Hello Mathieu,

Mathieu Othacehe <othacehe@gnu.org> writes:

> Hello Maxim,
>
>> * guix/scripts/system.scm (%options)[volatile-root?]: New boolean option.
>> (%default-options): Set its default value to #f.
>> (show-help): Add help doc.
>> * guix/scripts/system.scm (perform-action): Propagate option...
>> (system-derivation-for-action): ...here.  Use it to set the volatile-root?
>> field of the image object passed to SYSTEM-IMAGE.
>> * doc/guix.texi (Invoking guix system): Document it.
>
> This is a nice addition and it looks good to me.
>
> Thanks,
>
> Mathieu

One thing I wasn't sure was if it'd be better to implicitly derive
volatile-root? #f when image-type was specified (not (eq? 'guess)).  The
rationale being that users specifying the image-size themselves probably
want to make use of extra space (assumption) but can't unless
volatile-root? is #t on the image.

If we choose the above it'd lead to less code and one less switch; on
the other hand it is not as explicit.

What do you think?

Maxim




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

* bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image.
  2020-11-12 14:59             ` Maxim Cournoyer
@ 2020-11-12 17:06               ` Mathieu Othacehe
  2020-11-17 14:44                 ` Maxim Cournoyer
  0 siblings, 1 reply; 28+ messages in thread
From: Mathieu Othacehe @ 2020-11-12 17:06 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 44353, Jesse Gibbons


Hello Maxim,

> One thing I wasn't sure was if it'd be better to implicitly derive
> volatile-root? #f when image-type was specified (not (eq? 'guess)).  The
> rationale being that users specifying the image-size themselves probably
> want to make use of extra space (assumption) but can't unless
> volatile-root? is #t on the image.
>
> If we choose the above it'd lead to less code and one less switch; on
> the other hand it is not as explicit.
>
> What do you think?

When developing the image API, I hesitated keeping the default of using
volatile root file systems by default.

In most cases, I think that the user may prefer to have non volatile
images. They already have to copy the image from the store and make it
writable so that QEMU can use it with "-hda" argument.

Switching the default to non volatile could be an option I think. Doing
what you are proposing: setting volatile to false when image-size is
passed could also be a first step.

Thanks,

Mathieu




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

* bug#44353: [PATCH version-1.2.0 v2 3/3] doc: Detail which bootloader get used with disk-image or vm-image.
  2020-11-12  8:45             ` Mathieu Othacehe
@ 2020-11-12 21:16               ` Ludovic Courtès
  0 siblings, 0 replies; 28+ messages in thread
From: Ludovic Courtès @ 2020-11-12 21:16 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 44353, Jesse Gibbons, Maxim Cournoyer

Hi,

Mathieu Othacehe <othacehe@gnu.org> skribis:

>> +@example
>> +image=$(guix system disk-image --image-type=qcow2 \
>> +             gnu/system/examples/lightweight-desktop.tmpl)
>> +cp $image /tmp/my-image.qcow2
>> +chmod +rw /tmp/my-image.qcow2
>> +qemu-system-x86_64 -enable-kvm -hda /tmp/my-image.qcow2 -m 1000
>> +                   -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin
>> +@end example
>
> This looks good! Note that I plan to remove the "vm-image" command and
> rename "disk-image" command to "image" once the 1.2.0 is out.

However note that the manual should not be change on ‘version-1.2.0’ at
this point, “the strings are frozen”, they say.

Ludo’.




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

* bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image.
  2020-11-12  8:36           ` Mathieu Othacehe
  2020-11-12 14:59             ` Maxim Cournoyer
@ 2020-11-12 21:18             ` Ludovic Courtès
  2020-11-16  2:48               ` Maxim Cournoyer
  1 sibling, 1 reply; 28+ messages in thread
From: Ludovic Courtès @ 2020-11-12 21:18 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 44353, Jesse Gibbons, Maxim Cournoyer

Mathieu Othacehe <othacehe@gnu.org> skribis:

>> * guix/scripts/system.scm (%options)[volatile-root?]: New boolean option.
>> (%default-options): Set its default value to #f.
>> (show-help): Add help doc.
>> * guix/scripts/system.scm (perform-action): Propagate option...
>> (system-derivation-for-action): ...here.  Use it to set the volatile-root?
>> field of the image object passed to SYSTEM-IMAGE.
>> * doc/guix.texi (Invoking guix system): Document it.
>
> This is a nice addition and it looks good to me.

Can we keep it for ‘master’ only, notably because of the “string
freeze”?

Ludo’.




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

* bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image.
  2020-11-12  7:09         ` bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image Maxim Cournoyer
  2020-11-12  8:36           ` Mathieu Othacehe
@ 2020-11-15 21:45           ` Ludovic Courtès
  2020-11-16  3:47             ` Maxim Cournoyer
  2020-11-16  9:23             ` Mathieu Othacehe
  1 sibling, 2 replies; 28+ messages in thread
From: Ludovic Courtès @ 2020-11-15 21:45 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 44353, Mathieu Othacehe, Jesse Gibbons

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> * guix/scripts/system.scm (%options)[volatile-root?]: New boolean option.
> (%default-options): Set its default value to #f.
> (show-help): Add help doc.
> * guix/scripts/system.scm (perform-action): Propagate option...
> (system-derivation-for-action): ...here.  Use it to set the volatile-root?
> field of the image object passed to SYSTEM-IMAGE.
> * doc/guix.texi (Invoking guix system): Document it.

Due notably to the “string freeze”, I think we shouldn’t apply it to
‘version-1.2.0’.

Some comments:

> +@code{disk-image}.  By default, the root file system of a disk image is
> +mounted volatile; the @option{--non-volatile} option can be used to make

That’s not generally the case, though in (gnu system image), only two
image types have it set to false.

Before the new image API though, ‘disk-image’ did not produce a volatile
root, IIRC.  I’m tempted to think that we should set (volatile-root?
#f) on image types where it makes sense, which is maybe all of them
except ISO.  (Then we need to make sure ‘guix system vm’ still gets a
volatile root.)

WDYT, Mathieu?

So apart from the sentence above, the patch LGTM for ‘master’!

Thanks,
Ludo’.




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

* bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image.
  2020-11-12 21:18             ` Ludovic Courtès
@ 2020-11-16  2:48               ` Maxim Cournoyer
  0 siblings, 0 replies; 28+ messages in thread
From: Maxim Cournoyer @ 2020-11-16  2:48 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 44353, Mathieu Othacehe, Jesse Gibbons

Hey,

Ludovic Courtès <ludo@gnu.org> writes:

> Mathieu Othacehe <othacehe@gnu.org> skribis:
>
>>> * guix/scripts/system.scm (%options)[volatile-root?]: New boolean option.
>>> (%default-options): Set its default value to #f.
>>> (show-help): Add help doc.
>>> * guix/scripts/system.scm (perform-action): Propagate option...
>>> (system-derivation-for-action): ...here.  Use it to set the volatile-root?
>>> field of the image object passed to SYSTEM-IMAGE.
>>> * doc/guix.texi (Invoking guix system): Document it.
>>
>> This is a nice addition and it looks good to me.
>
> Can we keep it for ‘master’ only, notably because of the “string
> freeze”?

Sure, let's do that!

Maxim




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

* bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image.
  2020-11-15 21:45           ` Ludovic Courtès
@ 2020-11-16  3:47             ` Maxim Cournoyer
  2020-11-16  9:23             ` Mathieu Othacehe
  1 sibling, 0 replies; 28+ messages in thread
From: Maxim Cournoyer @ 2020-11-16  3:47 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 44353, Mathieu Othacehe, Jesse Gibbons

Hello,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> * guix/scripts/system.scm (%options)[volatile-root?]: New boolean option.
>> (%default-options): Set its default value to #f.
>> (show-help): Add help doc.
>> * guix/scripts/system.scm (perform-action): Propagate option...
>> (system-derivation-for-action): ...here.  Use it to set the volatile-root?
>> field of the image object passed to SYSTEM-IMAGE.
>> * doc/guix.texi (Invoking guix system): Document it.
>
> Due notably to the “string freeze”, I think we shouldn’t apply it to
> ‘version-1.2.0’.
>
> Some comments:
>
>> +@code{disk-image}.  By default, the root file system of a disk image is
>> +mounted volatile; the @option{--non-volatile} option can be used to make
>
> That’s not generally the case, though in (gnu system image), only two
> image types have it set to false.

Note that the only two images with volatile-root? #f are ARM, and not by
intent but as a workaround:

   ;; FIXME: Deleting and creating "/var/run" and "/tmp" on the overlayfs
   ;; fails.

> Before the new image API though, ‘disk-image’ did not produce a volatile
> root, IIRC.  I’m tempted to think that we should set (volatile-root?
> #f) on image types where it makes sense, which is maybe all of them
> except ISO.  (Then we need to make sure ‘guix system vm’ still gets a
> volatile root.)
>
> WDYT, Mathieu?

Based on your comments and those of Mathieut, I've made volatile-root?
#f the default for 'guix system disk-image', with a '--volatile' option
to maintain the ability to have the rootfs mounted volatile, and
adjusted the doc accordingly.

> So apart from the sentence above, the patch LGTM for ‘master’!

Thanks for the review!

Maxim




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

* bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image.
  2020-11-15 21:45           ` Ludovic Courtès
  2020-11-16  3:47             ` Maxim Cournoyer
@ 2020-11-16  9:23             ` Mathieu Othacehe
  2020-11-16 13:09               ` Ludovic Courtès
  1 sibling, 1 reply; 28+ messages in thread
From: Mathieu Othacehe @ 2020-11-16  9:23 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 44353, Jesse Gibbons, Maxim Cournoyer


Hey Ludo,

> Before the new image API though, ‘disk-image’ did not produce a volatile
> root, IIRC.  I’m tempted to think that we should set (volatile-root?
> #f) on image types where it makes sense, which is maybe all of them
> except ISO.  (Then we need to make sure ‘guix system vm’ still gets a
> volatile root.)
>
> WDYT, Mathieu?

That's also what I proposed to Maxim. Note that "guix system vm" is
still using the "old" VM based image production method and doesn't share
the new image API volatile default.

Thanks,

Mathieu




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

* bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image.
  2020-11-16  9:23             ` Mathieu Othacehe
@ 2020-11-16 13:09               ` Ludovic Courtès
  2020-11-16 13:34                 ` Mathieu Othacehe
  0 siblings, 1 reply; 28+ messages in thread
From: Ludovic Courtès @ 2020-11-16 13:09 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 44353, Jesse Gibbons, Maxim Cournoyer

Mathieu Othacehe <othacehe@gnu.org> skribis:

>> Before the new image API though, ‘disk-image’ did not produce a volatile
>> root, IIRC.  I’m tempted to think that we should set (volatile-root?
>> #f) on image types where it makes sense, which is maybe all of them
>> except ISO.  (Then we need to make sure ‘guix system vm’ still gets a
>> volatile root.)
>>
>> WDYT, Mathieu?
>
> That's also what I proposed to Maxim. Note that "guix system vm" is
> still using the "old" VM based image production method and doesn't share
> the new image API volatile default.

Right.

Speaking of which, can we remove ‘system-disk-image-in-vm’?  It has zero
users now.

Ludo’.




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

* bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image.
  2020-11-16 13:09               ` Ludovic Courtès
@ 2020-11-16 13:34                 ` Mathieu Othacehe
  2020-11-17 20:21                   ` Maxim Cournoyer
  0 siblings, 1 reply; 28+ messages in thread
From: Mathieu Othacehe @ 2020-11-16 13:34 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 44353, Jesse Gibbons, Maxim Cournoyer


Hey,

> Speaking of which, can we remove ‘system-disk-image-in-vm’?  It has zero
> users now.

Sure, I also need to find some time to route "vm-image", "vm" and
possibly "docker-image" commands to new image API.

Thanks,

Mathieu




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

* bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image.
  2020-11-12 17:06               ` Mathieu Othacehe
@ 2020-11-17 14:44                 ` Maxim Cournoyer
  0 siblings, 0 replies; 28+ messages in thread
From: Maxim Cournoyer @ 2020-11-17 14:44 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 44353, Jesse Gibbons

Hello Mathieu,

Mathieu Othacehe <othacehe@gnu.org> writes:


[...]

> When developing the image API, I hesitated keeping the default of using
> volatile root file systems by default.
>
> In most cases, I think that the user may prefer to have non volatile
> images. They already have to copy the image from the store and make it
> writable so that QEMU can use it with "-hda" argument.
>
> Switching the default to non volatile could be an option I think. Doing
> what you are proposing: setting volatile to false when image-size is
> passed could also be a first step.

Hi Mathieu!  As I mentioned in a reply to Ludovic and based from your
feedback I'll make the disk-image produced to be non-volatile by
default, with the switch to make them volatile becoming "--volatile".

Thanks!

Maxim




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

* bug#44353: [PATCH version-1.2.0 v2 2/3] bootloader: grub: Skip install-grub-efi when producing a disk image.
  2020-11-12  8:42             ` Mathieu Othacehe
@ 2020-11-17 18:29               ` Maxim Cournoyer
  0 siblings, 0 replies; 28+ messages in thread
From: Maxim Cournoyer @ 2020-11-17 18:29 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 44353, Jesse Gibbons

Hello,

Mathieu Othacehe <othacehe@gnu.org> writes:

> Hello Maxim,
>
>> As introducing bootloader knowledge at the level of the image code is
>> undesirable, every bootloader should be adapted to support this fall-back for
>> their installation procedure (TODO).
>
> Or maybe find a way to remove the need to call "bootloader-installer"
> when producing a disk-image as we discussed on IRC.

I looked into this option, using GPT + a 1 MiB boot partition, hoping to
produce a GRUB to put there with grub mkstandalone, but that is too big
to fit for the i386-pc format requirements, sadly.

Maxim




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

* bug#44353: [PATCH version-1.2.0 v2 1/3] image: Remove conflicting user-provided EFI file system.
  2020-11-12  8:39           ` bug#44353: [PATCH version-1.2.0 v2 1/3] image: Remove conflicting user-provided EFI file system Mathieu Othacehe
@ 2020-11-17 18:37             ` Maxim Cournoyer
  0 siblings, 0 replies; 28+ messages in thread
From: Maxim Cournoyer @ 2020-11-17 18:37 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 44353, Jesse Gibbons

Hello Mathieu!

Mathieu Othacehe <othacehe@gnu.org> writes:

> Hey Maxim,
>
>> When the bootloader used is grub-efi-bootloader, an ESP partition and file
>> system is already added by the image generator.  If a conflicting
>> user-provided EFI file system is provided, it will (likely) cause the boot to
>> fail, as happens for the lightweight-desktop.tmpl and desktop.tmpl templates
>> under gnu/system/examples.
>
> Nice one. I would just clarify in the commit message that the ESP
> partition is added only when the selected image type is "raw".

Done, thank you!

Thank you!

Maxim




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

* bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image.
  2020-11-16 13:34                 ` Mathieu Othacehe
@ 2020-11-17 20:21                   ` Maxim Cournoyer
  0 siblings, 0 replies; 28+ messages in thread
From: Maxim Cournoyer @ 2020-11-17 20:21 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 44353-done, Jesse Gibbons

Hello,

Mathieu Othacehe <othacehe@gnu.org> writes:

> Hey,
>
>> Speaking of which, can we remove ‘system-disk-image-in-vm’?  It has zero
>> users now.

Done!

This series has now been pushed to master.  Closing.

Thank you,

Maxim




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

end of thread, other threads:[~2020-11-17 20:22 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-31 16:47 bug#44353: guix system disk-image -t raw fails with grub-efi-bootloader Jesse Gibbons
2020-11-07  7:09 ` Maxim Cournoyer
2020-11-07  9:08   ` Mathieu Othacehe
2020-11-07 11:26     ` Bengt Richter
2020-11-07 20:32     ` Maxim Cournoyer
2020-11-08 11:04       ` Mathieu Othacehe
2020-11-12  3:57         ` bug#44353: [PATCH version-1.2.0 v2 1/3] image: Remove conflicting user-provided EFI file system Maxim Cournoyer
2020-11-12  3:57           ` bug#44353: [PATCH version-1.2.0 v2 2/3] bootloader: grub: Skip install-grub-efi when producing a disk image Maxim Cournoyer
2020-11-12  8:42             ` Mathieu Othacehe
2020-11-17 18:29               ` Maxim Cournoyer
2020-11-12  3:57           ` bug#44353: [PATCH version-1.2.0 v2 3/3] doc: Detail which bootloader get used with disk-image or vm-image Maxim Cournoyer
2020-11-12  8:45             ` Mathieu Othacehe
2020-11-12 21:16               ` Ludovic Courtès
2020-11-12  8:39           ` bug#44353: [PATCH version-1.2.0 v2 1/3] image: Remove conflicting user-provided EFI file system Mathieu Othacehe
2020-11-17 18:37             ` Maxim Cournoyer
2020-11-12  7:09         ` bug#44353: [PATCH version-1.2.0 v2] guix: system: Add a new '--non-volatile' option for disk-image Maxim Cournoyer
2020-11-12  8:36           ` Mathieu Othacehe
2020-11-12 14:59             ` Maxim Cournoyer
2020-11-12 17:06               ` Mathieu Othacehe
2020-11-17 14:44                 ` Maxim Cournoyer
2020-11-12 21:18             ` Ludovic Courtès
2020-11-16  2:48               ` Maxim Cournoyer
2020-11-15 21:45           ` Ludovic Courtès
2020-11-16  3:47             ` Maxim Cournoyer
2020-11-16  9:23             ` Mathieu Othacehe
2020-11-16 13:09               ` Ludovic Courtès
2020-11-16 13:34                 ` Mathieu Othacehe
2020-11-17 20:21                   ` Maxim Cournoyer

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).