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

Previous Next

Package: guix-patches;

Reported by: Sergey Trofimov <sarg <at> sarg.org.ru>

Date: Thu, 16 Mar 2023 17:39:01 UTC

Severity: normal

Tags: patch

Done: Josselin Poiret <dev <at> jpoiret.xyz>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 62223 in the body.
You can then email your comments to 62223 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#62223; Package guix-patches. (Thu, 16 Mar 2023 17:39:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Sergey Trofimov <sarg <at> sarg.org.ru>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Thu, 16 Mar 2023 17:39:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Sergey Trofimov <sarg <at> sarg.org.ru>
To: guix-patches <at> gnu.org
Cc: Sergey Trofimov <sarg <at> sarg.org.ru>
Subject: [PATCH] image: Prefer gpt partition table for efi disk images
Date: Thu, 16 Mar 2023 18:31:38 +0100
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





Information forwarded to guix-patches <at> gnu.org:
bug#62223; Package guix-patches. (Tue, 21 Mar 2023 21:42:01 GMT) Full text and rfc822 format available.

Message #8 received at 62223 <at> debbugs.gnu.org (full text, mbox):

From: Josselin Poiret <dev <at> jpoiret.xyz>
To: Sergey Trofimov <sarg <at> sarg.org.ru>, 62223 <at> debbugs.gnu.org
Cc: Sergey Trofimov <sarg <at> sarg.org.ru>
Subject: Re: [bug#62223] [PATCH] image: Prefer gpt partition table for efi
 disk images
Date: Tue, 21 Mar 2023 22:41:49 +0100
[Message part 1 (text/plain, inline)]
Hi Sergey,

Sergey Trofimov <sarg <at> 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
[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#62223; Package guix-patches. (Wed, 22 Mar 2023 08:23:01 GMT) Full text and rfc822 format available.

Message #11 received at 62223 <at> debbugs.gnu.org (full text, mbox):

From: Sergey Trofimov <sarg <at> sarg.org.ru>
To: 62223 <at> debbugs.gnu.org
Subject: Tested locally
Date: Wed, 22 Mar 2023 09:14:13 +0100
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---




Information forwarded to guix-patches <at> gnu.org:
bug#62223; Package guix-patches. (Wed, 22 Mar 2023 12:51:01 GMT) Full text and rfc822 format available.

Message #14 received at 62223 <at> debbugs.gnu.org (full text, mbox):

From: Sergey Trofimov <sarg <at> sarg.org.ru>
To: Josselin Poiret <dev <at> jpoiret.xyz>
Cc: 62223 <at> debbugs.gnu.org
Subject: Re: [bug#62223] [PATCH] image: Prefer gpt partition table for efi
 disk images
Date: Wed, 22 Mar 2023 13:48:47 +0100
Josselin Poiret <dev <at> 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.




Information forwarded to guix-patches <at> gnu.org:
bug#62223; Package guix-patches. (Wed, 22 Mar 2023 12:54:02 GMT) Full text and rfc822 format available.

Message #17 received at 62223 <at> debbugs.gnu.org (full text, mbox):

From: Sergey Trofimov <sarg <at> sarg.org.ru>
To: 62223 <at> debbugs.gnu.org
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Sergey Trofimov <sarg <at> sarg.org.ru>
Subject: [PATCH v2] image: Prefer gpt partition table for efi images
Date: Wed, 22 Mar 2023 13:53:28 +0100
* 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





Reply sent to Josselin Poiret <dev <at> jpoiret.xyz>:
You have taken responsibility. (Fri, 07 Jul 2023 19:51:02 GMT) Full text and rfc822 format available.

Notification sent to Sergey Trofimov <sarg <at> sarg.org.ru>:
bug acknowledged by developer. (Fri, 07 Jul 2023 19:51:02 GMT) Full text and rfc822 format available.

Message #22 received at 62223-done <at> debbugs.gnu.org (full text, mbox):

From: Josselin Poiret <dev <at> jpoiret.xyz>
To: Sergey Trofimov <sarg <at> sarg.org.ru>, 62223-done <at> debbugs.gnu.org
Cc: Sergey Trofimov <sarg <at> sarg.org.ru>
Subject: Re: [PATCH v2] image: Prefer gpt partition table for efi images
Date: Fri, 07 Jul 2023 21:50:34 +0200
[Message part 1 (text/plain, inline)]
Hi Sergey, 

Sergey Trofimov <sarg <at> 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
[signature.asc (application/pgp-signature, inline)]

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sat, 05 Aug 2023 11:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 265 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.