[bug#62223] [PATCH] image: Prefer gpt partition table for efi disk images

[bug#62223] [PATCH] image: Prefer gpt partition table for efi disk images

[Sergey Trofimov]

Hi guix, I got curious, what the easiest way to migrate existing guix os to a
new machine could be, and I've came to a conclusion that `guix system image`
fits this scenario perfectly. So I've tried to run `guix system image
--image-type=efi-raw --persistent --save-provenance system.scm`, but the
resulting image contained MBR style partition table. Although MBR support is a
must for an UEFI implementor, a better choice would be to use GPT style table.

* gnu/system/image.scm (efi-disk-image): Use gpt partition-table-type.
(efi32-disk-image): Use gpt partition-table-type.
* gnu/tests/image.scm: Assert partition table type of efi-disk-image.
---
 gnu/system/image.scm |  2 ++
 gnu/tests/image.scm  | 23 ++++++++++++++++++-----
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/gnu/system/image.scm b/gnu/system/image.scm
index afef79185f..5356ecd616 100644
--- a/gnu/system/image.scm
+++ b/gnu/system/image.scm
@@ -147,11 +147,13 @@ (define root-partition
 (define efi-disk-image
   (image-without-os
    (format 'disk-image)
+   (partition-table-type 'gpt)
    (partitions (list esp-partition root-partition))))
 
 (define efi32-disk-image
   (image-without-os
    (format 'disk-image)
+   (partition-table-type 'gpt)
    (partitions (list esp32-partition root-partition))))
 
 (define iso9660-image
diff --git a/gnu/tests/image.scm b/gnu/tests/image.scm
index 99d34b7670..be6852cae0 100644
--- a/gnu/tests/image.scm
+++ b/gnu/tests/image.scm
@@ -20,7 +20,7 @@ (define-module (gnu tests image)
   #:use-module (gnu)
   #:use-module (gnu image)
   #:use-module (gnu tests)
-  #:autoload   (gnu system image) (system-image root-offset)
+  #:autoload   (gnu system image) (system-image root-offset image-with-os efi-disk-image)
   #:use-module (gnu system uuid)
   #:use-module (gnu system vm)
   #:use-module (gnu packages guile)
@@ -153,6 +153,10 @@ (define i5
       (flags '(boot))
       (initializer dummy-initializer))))))
 
+;; A efi disk image with default partitions
+(define i6
+  (image-with-os efi-disk-image %simple-efi-os))
+
 (define (run-images-test)
   (define test
     (with-imported-modules '((srfi srfi-64)
@@ -202,10 +206,10 @@ (define d2-device
               (disk-get-primary-partition-count (disk-new d2-device)))
 
             (test-equal "test"
-                (let* ((disk (disk-new d2-device))
-                       (partitions (disk-partitions disk))
-                       (boot-partition (find normal-partition? partitions)))
-                  (partition-get-name boot-partition)))
+              (let* ((disk (disk-new d2-device))
+                     (partitions (disk-partitions disk))
+                     (boot-partition (find normal-partition? partitions)))
+                (partition-get-name boot-partition)))
 
             ;; Image i3.
             (define i3-image
@@ -259,6 +263,15 @@ (define (sector->byte sector)
                    (filter data-partition?
                            (disk-partitions (disk-new d5-device)))))
 
+            ;; Image i6.
+            (define i6-image
+              #$(system-image i6))
+            (define d6-device
+              (get-device i6-image))
+
+            (test-equal "gpt"
+              (disk-type-name (disk-probe d6-device)))
+
             (test-end)))))
 
   (gexp->derivation "images-test" test))
-- 
2.39.2

[bug#62223] [PATCH] image: Prefer gpt partition table for efi disk images

[Josselin Poiret via Guix-patches via]

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

Hi Sergey,

Sergey Trofimov <sarg@sarg.org.ru> writes:

> Hi guix, I got curious, what the easiest way to migrate existing guix os to a
> new machine could be, and I've came to a conclusion that `guix system image`
> fits this scenario perfectly. So I've tried to run `guix system image
> --image-type=efi-raw --persistent --save-provenance system.scm`, but the
> resulting image contained MBR style partition table. Although MBR support is a
> must for an UEFI implementor, a better choice would be to use GPT style table.

Thanks for the patch!  First off, if you want to add a comment to your
mail but not to the commit message, you should add it below the first --
(under the commit message), before the diff, that way it doesn't end up
in the commit message.

With this applied, I cannot build an example qcow2 image using `guix
system image -t qcow2 gnu/system/examples/bare-bones.tmpl`.  It fails
when installing GRUB onto the image.  Sure enough, the
install-grub-disk-image procedure seems to only support MBR.  Could you
also have a look at this?

Best,
-- 
Josselin Poiret

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

[bug#62223] Tested locally

[Sergey Trofimov]

I have created an image and verified that it boots on qemu with 
proper bios.
Then I've baked an image of my real system configuration and used 
it to reprovision my laptop.

vm.scm
--8<---------------cut here---------------start------------->8---
...
  (bootloader
   (bootloader-configuration
    (bootloader grub-efi-bootloader)
    (targets '("/boot"))))
  (file-systems
   (cons*
    (file-system
      (mount-point "/")
      (device (file-system-label "Guix_image"))
      (type "ext4"))
    (file-system
      (mount-point "/boot")
      (device (file-system-label "GNU-ESP"))
      (type "vfat"))
    %base-file-systems))
...
--8<---------------cut here---------------end--------------->8---

testing it with:
--8<---------------cut here---------------start------------->8---
guix system image --image-type=efi-raw vm.scm

qemu-system-x86_64 \
    -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin \
    -drive file=$HOME/vm.img,if=virtio
--8<---------------cut here---------------end--------------->8---

[bug#62223] [PATCH] image: Prefer gpt partition table for efi disk images

[Sergey Trofimov]

Josselin Poiret <dev@jpoiret.xyz> writes:

> [[PGP Signed Part:Undecided]]
> Hi Sergey,
>
> Thanks for the patch!  First off, if you want to add a comment 
> to your
> mail but not to the commit message, you should add it below the 
> first --
> (under the commit message), before the diff, that way it doesn't 
> end up
> in the commit message.
>

Thanks, noted.

>
> With this applied, I cannot build an example qcow2 image using 
> `guix
> system image -t qcow2 gnu/system/examples/bare-bones.tmpl`.  It 
> fails
> when installing GRUB onto the image.  Sure enough, the
> install-grub-disk-image procedure seems to only support MBR. 
> Could you
> also have a look at this?

Yeah, I'm glad you've caught that. qcow2 should be mbr style by 
default. I'll send a v2 for that.

[bug#62223] [PATCH v2] image: Prefer gpt partition table for efi images

[Sergey Trofimov]

* gnu/system/image.scm (efi-disk-image): Use gpt partition-table-type.
(efi32-disk-image): Use gpt partition-table-type.
(qcow2-image-type): Use mbr partition-table-type explicitly.
* gnu/tests/image.scm: Assert partition table type of efi-disk-image.
---
 gnu/system/image.scm |  3 +++
 gnu/tests/image.scm  | 23 ++++++++++++++++++-----
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/gnu/system/image.scm b/gnu/system/image.scm
index afef79185f..699feaf05d 100644
--- a/gnu/system/image.scm
+++ b/gnu/system/image.scm
@@ -147,11 +147,13 @@ (define root-partition
 (define efi-disk-image
   (image-without-os
    (format 'disk-image)
+   (partition-table-type 'gpt)
    (partitions (list esp-partition root-partition))))
 
 (define efi32-disk-image
   (image-without-os
    (format 'disk-image)
+   (partition-table-type 'gpt)
    (partitions (list esp32-partition root-partition))))
 
 (define iso9660-image
@@ -214,6 +216,7 @@ (define qcow2-image-type
    (constructor (cut image-with-os
                  (image
                   (inherit efi-disk-image)
+                  (partition-table-type 'mbr)
                   (name 'image.qcow2)
                   (format 'compressed-qcow2))
                  <>))))
diff --git a/gnu/tests/image.scm b/gnu/tests/image.scm
index 99d34b7670..be6852cae0 100644
--- a/gnu/tests/image.scm
+++ b/gnu/tests/image.scm
@@ -20,7 +20,7 @@ (define-module (gnu tests image)
   #:use-module (gnu)
   #:use-module (gnu image)
   #:use-module (gnu tests)
-  #:autoload   (gnu system image) (system-image root-offset)
+  #:autoload   (gnu system image) (system-image root-offset image-with-os efi-disk-image)
   #:use-module (gnu system uuid)
   #:use-module (gnu system vm)
   #:use-module (gnu packages guile)
@@ -153,6 +153,10 @@ (define i5
       (flags '(boot))
       (initializer dummy-initializer))))))
 
+;; A efi disk image with default partitions
+(define i6
+  (image-with-os efi-disk-image %simple-efi-os))
+
 (define (run-images-test)
   (define test
     (with-imported-modules '((srfi srfi-64)
@@ -202,10 +206,10 @@ (define d2-device
               (disk-get-primary-partition-count (disk-new d2-device)))
 
             (test-equal "test"
-                (let* ((disk (disk-new d2-device))
-                       (partitions (disk-partitions disk))
-                       (boot-partition (find normal-partition? partitions)))
-                  (partition-get-name boot-partition)))
+              (let* ((disk (disk-new d2-device))
+                     (partitions (disk-partitions disk))
+                     (boot-partition (find normal-partition? partitions)))
+                (partition-get-name boot-partition)))
 
             ;; Image i3.
             (define i3-image
@@ -259,6 +263,15 @@ (define (sector->byte sector)
                    (filter data-partition?
                            (disk-partitions (disk-new d5-device)))))
 
+            ;; Image i6.
+            (define i6-image
+              #$(system-image i6))
+            (define d6-device
+              (get-device i6-image))
+
+            (test-equal "gpt"
+              (disk-type-name (disk-probe d6-device)))
+
             (test-end)))))
 
   (gexp->derivation "images-test" test))
-- 
2.39.2

bug#62223: [PATCH v2] image: Prefer gpt partition table for efi images

[Josselin Poiret via Guix-patches via]

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

Hi Sergey, 

Sergey Trofimov <sarg@sarg.org.ru> writes:

> * gnu/system/image.scm (efi-disk-image): Use gpt partition-table-type.
> (efi32-disk-image): Use gpt partition-table-type.
> (qcow2-image-type): Use mbr partition-table-type explicitly.
> * gnu/tests/image.scm: Assert partition table type of efi-disk-image.

Pushed as 209204e23b39af09e0ea92540b6fa00a60e6a0ae, thanks!

Best,
-- 
Josselin Poiret

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