unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
* bug#27735: Unbootable images with GuixSD on... "GuixSD"
@ 2017-07-17 14:40 Tobias Geerinckx-Rice
  2017-07-17 14:51 ` bug#27735: [PATCH 1/2] build, vm: Use a slightly less generic label Tobias Geerinckx-Rice
  2017-07-17 17:17 ` bug#27735: Unbootable images with GuixSD on... "GuixSD" Danny Milosavljevic
  0 siblings, 2 replies; 20+ messages in thread
From: Tobias Geerinckx-Rice @ 2017-07-17 14:40 UTC (permalink / raw)
  To: 27735


[-- Attachment #1.1: Type: text/plain, Size: 2287 bytes --]

Guix,

[I lost most hours of sleep to this. I might ramble more than usual.]

The default label for images was recently changed[1] to "GuixSD".
While I think it's a fine label, that's also a problem: I've been using
it for years for my root partitions. And when one broke last night, I
couldn't use the GuixSD installer to rescue it.

The installer's now expects exactly one "GuixSD" partition when booting
— at least on UEFI. If the GRUB finds two, the GRUB will randomly
choose. In my case, the GRUB chose a frozen system.

(With a black screen that made debugging hell, but that's probably an
unrelated effect of the roughness of our UEFI support.)

The real problem here is that we're using a label as a UUID.
From gnu/build/vm.scm:

        ;; Create a tiny configuration file telling the embedded grub
        ;; where to load the real thing.
        (call-with-output-file grub-config
          (lambda (port)
            (format port
                    "insmod part_msdos~@
                    search --set=root --label GuixSD~@
                    configfile /boot/grub/grub.cfg~%")))

I'm not the first to think so, as noted in gnu/system/vm.scm:

  (define root-label
    ;; Volume name of the root file system.  Since we don't know which
device
    ;; will hold it, we use the volume name to find it (using the UUID would
    ;; be even better, but somewhat less convenient.)
    (normalize-label "GuixSD"))

I like that understatement. I'm not sure how to go about creating a
reproducible almost-UUID based on the store hash and passing it to all
the right places in a reasonably non-horrible manner either, random
hacker. And it would mean even more work and testing after all the
heroic effort on the new installer image + UEFI support by Danny,
Marius, and others.

Until it does happen, I suggest we change the name to "GuixSD-image"[2].
Still fragile, but not the PR fail that ‘don't call your GuixSD file
system GuixSD or it will break GuixSD’ would be.

Zzz,

T G-R

[1]:
https://git.savannah.gnu.org/cgit/guix.git/commit/?id=651de2bdb5fd451c50933dcf8d647d470d826261
[2]: Or whatever. I remember someone (Danny?) calling "-image" an
implementation detail. I think it's a description of the end result.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 504 bytes --]

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

* bug#27735: [PATCH 1/2] build, vm: Use a slightly less generic label.
  2017-07-17 14:40 bug#27735: Unbootable images with GuixSD on... "GuixSD" Tobias Geerinckx-Rice
@ 2017-07-17 14:51 ` Tobias Geerinckx-Rice
  2017-07-17 17:20   ` Danny Milosavljevic
  2017-07-17 17:17 ` bug#27735: Unbootable images with GuixSD on... "GuixSD" Danny Milosavljevic
  1 sibling, 1 reply; 20+ messages in thread
From: Tobias Geerinckx-Rice @ 2017-07-17 14:51 UTC (permalink / raw)
  To: 27735

* gnu/build/vm.scm (initialize-hard-disk): Use "GuixSD-image" as label.
* gnu/system/install.scm (installation-os): Likewise.
* gnu/system/vm.scm (system-disk-image): Likewise.
---

Or GuixSD-bikeshed or whatever.

 gnu/build/vm.scm       | 7 +++++--
 gnu/system/install.scm | 2 +-
 gnu/system/vm.scm      | 2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index d8c53ef37..e71b1b92e 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -354,7 +354,7 @@ SYSTEM-DIRECTORY is the name of the directory of the 'system' derivation."
       (error "failed to create GRUB EFI image"))))
 
 (define* (make-iso9660-image grub config-file os-drv target
-                             #:key (volume-id "GuixSD") (volume-uuid #f))
+                             #:key (volume-id "GuixSD-image") (volume-uuid #f))
   "Given a GRUB package, creates an iso image as TARGET, using CONFIG-FILE as
 Grub configuration and OS-DRV as the stuff in it."
   (let ((grub-mkrescue (string-append grub "/bin/grub-mkrescue")))
@@ -440,11 +440,14 @@ passing it a directory name where it is mounted."
 
         ;; Create a tiny configuration file telling the embedded grub
         ;; where to load the real thing.
+        ;; XXX This is quite fragile, and can leave the system in an unusable
+        ;; state when there's more than one volume with this label present.
+        ;; Reproducible (not-)UUIDs could reduce the risk but not eliminate it.
         (call-with-output-file grub-config
           (lambda (port)
             (format port
                     "insmod part_msdos~@
-                    search --set=root --label GuixSD~@
+                    search --set=root --label GuixSD-image~@
                     configfile /boot/grub/grub.cfg~%")))
 
         (display "creating EFI firmware image...")
diff --git a/gnu/system/install.scm b/gnu/system/install.scm
index f9aa7f673..866440eb4 100644
--- a/gnu/system/install.scm
+++ b/gnu/system/install.scm
@@ -306,7 +306,7 @@ Use Alt-F2 for documentation.
      ;; the appropriate one.
      (cons* (file-system
               (mount-point "/")
-              (device "GuixSD")
+              (device "GuixSD-image")
               (title 'label)
               (type "ext4"))
 
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index dd9be2c6f..6e06781d5 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -345,7 +345,7 @@ to USB sticks meant to be read-only."
     ;; Volume name of the root file system.  Since we don't know which device
     ;; will hold it, we use the volume name to find it (using the UUID would
     ;; be even better, but somewhat less convenient.)
-    (normalize-label "GuixSD"))
+    (normalize-label "GuixSD-image"))
 
   (define file-systems-to-keep
     (remove (lambda (fs)
-- 
2.13.1

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

* bug#27735: Unbootable images with GuixSD on... "GuixSD"
  2017-07-17 14:40 bug#27735: Unbootable images with GuixSD on... "GuixSD" Tobias Geerinckx-Rice
  2017-07-17 14:51 ` bug#27735: [PATCH 1/2] build, vm: Use a slightly less generic label Tobias Geerinckx-Rice
@ 2017-07-17 17:17 ` Danny Milosavljevic
  2017-07-17 18:12   ` Tobias Geerinckx-Rice
  2017-07-18 11:49   ` Ludovic Courtès
  1 sibling, 2 replies; 20+ messages in thread
From: Danny Milosavljevic @ 2017-07-17 17:17 UTC (permalink / raw)
  To: Tobias Geerinckx-Rice; +Cc: 27735

Hi T G-R,

On Mon, 17 Jul 2017 16:40:56 +0200
Tobias Geerinckx-Rice <me@tobias.gr> wrote:

> The installer's now expects exactly one "GuixSD" partition when booting
> — at least on UEFI. If the GRUB finds two, the GRUB will randomly
> choose. In my case, the GRUB chose a frozen system.

Sorry!

> The real problem here is that we're using a label as a UUID.

I agree.  Unfortunately Guix UUIDs are difficult to use consistently or I would have changed it over to begin with.

>   (define root-label
>     ;; Volume name of the root file system.  Since we don't know which
> device
>     ;; will hold it, we use the volume name to find it (using the UUID would
>     ;; be even better, but somewhat less convenient.)
>     (normalize-label "GuixSD"))
> 
> I like that understatement. I'm not sure how to go about creating a
> reproducible almost-UUID based on the store hash and passing it to all
> the right places in a reasonably non-horrible manner either, random
> hacker. And it would mean even more work and testing after all the
> heroic effort on the new installer image + UEFI support by Danny,
> Marius, and others.

Yeah, having the UUID actually be derived from the store hash would be good.

I think for now having a random UUID be generated would be fine too.

> Until it does happen, I suggest we change the name to "GuixSD-image"[2].
> Still fragile, but not the PR fail that ‘don't call your GuixSD file
> system GuixSD or it will break GuixSD’ would be.

I think for the time being that's a good workaround.  But I think we should get the UUIDs for booting working.

> [1]: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=651de2bdb5fd451c50933dcf8d647d470d826261

The actual reason I had to change it is because of the dashes (which are not valid in ECMA-6 IRV).

> [2]: Or whatever. 

"GuixSD_installer" ?  Well anyway, just pick one, doesn't matter much which, except that the characters have to be out of [A-Za-z0-9_] (will be uppercased for ISO-9660) because of ECMA-119 Appendix A (ECMA-6 IRV).

>I remember someone (Danny?) calling "-image" an
> implementation detail. I think it's a description of the end result.

Yeah, that was me.  I don't understand how an actual operating system on a drive is an image.  Maybe I'm old-fashioned, dunno, but I think an image is something that is made up by light rays on a screen, not the real object.  In the case of computing an image is a backup file of a drive, not what is on the drive to begin with.

Also, even if it were an image, the image shouldn't say "<foo> image" in the image itself.  A mirror which doesn't add anything to your image when you look into it, either :)

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

* bug#27735: [PATCH 1/2] build, vm: Use a slightly less generic label.
  2017-07-17 14:51 ` bug#27735: [PATCH 1/2] build, vm: Use a slightly less generic label Tobias Geerinckx-Rice
@ 2017-07-17 17:20   ` Danny Milosavljevic
  2017-07-17 17:58     ` Tobias Geerinckx-Rice
  2017-07-18 10:09     ` Ludovic Courtès
  0 siblings, 2 replies; 20+ messages in thread
From: Danny Milosavljevic @ 2017-07-17 17:20 UTC (permalink / raw)
  To: Tobias Geerinckx-Rice; +Cc: 27735

Dash is invalid.  Otherwise OK!

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

* bug#27735: [PATCH 1/2] build, vm: Use a slightly less generic label.
  2017-07-17 17:20   ` Danny Milosavljevic
@ 2017-07-17 17:58     ` Tobias Geerinckx-Rice
  2017-07-18 10:09     ` Ludovic Courtès
  1 sibling, 0 replies; 20+ messages in thread
From: Tobias Geerinckx-Rice @ 2017-07-17 17:58 UTC (permalink / raw)
  To: dannym, 27735


[-- Attachment #1.1: Type: text/plain, Size: 234 bytes --]

Danny,

On 17/07/17 19:20, Danny Milosavljevic wrote:
> Dash is invalid.  Otherwise OK!

Good to know (I consider you an expert on file system label esoterica,
mainly so I don't have to be). Thanks!

Kind regards,

T G-R


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 504 bytes --]

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

* bug#27735: Unbootable images with GuixSD on... "GuixSD"
  2017-07-17 17:17 ` bug#27735: Unbootable images with GuixSD on... "GuixSD" Danny Milosavljevic
@ 2017-07-17 18:12   ` Tobias Geerinckx-Rice
  2017-07-17 18:37     ` Tobias Geerinckx-Rice
  2017-07-18 11:49   ` Ludovic Courtès
  1 sibling, 1 reply; 20+ messages in thread
From: Tobias Geerinckx-Rice @ 2017-07-17 18:12 UTC (permalink / raw)
  To: dannym; +Cc: 27735


[-- Attachment #1.1: Type: text/plain, Size: 1041 bytes --]

On 17/07/17 19:17, Danny Milosavljevic wrote:> Hi T G-R,
> Yeah, that was me.  I don't understand how an actual operating system
> on a drive is an image.  Maybe I'm old-fashioned, dunno, but I think
> an image is something that is made up by light rays on a screen, not
> the real object.  In the case of computing an image is a backup file
> of a drive, not what is on the drive to begin with.
> 
> Also, even if it were an image, the image shouldn't say "<foo> image"
> in the image itself.  A mirror which doesn't add anything to your
> image when you look into it, either :)

I agree completely. I've become so used to dd'ing ISOs to USB drives
that I've come to think *only* in terms of fake discs, but you're right.
I chose ‘image’ reluctantly, because my first ideas (‘installer’, ‘vm’)
were even less relevant: this label's used for all disc images. Oh well.

We could choose the worst and ugliest label possible: that'd compel
someone to fix this!

(No. "GuixSD_image" it is.)

Thanks,

T G-R


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 504 bytes --]

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

* bug#27735: Unbootable images with GuixSD on... "GuixSD"
  2017-07-17 18:12   ` Tobias Geerinckx-Rice
@ 2017-07-17 18:37     ` Tobias Geerinckx-Rice
  0 siblings, 0 replies; 20+ messages in thread
From: Tobias Geerinckx-Rice @ 2017-07-17 18:37 UTC (permalink / raw)
  To: 27735-done


[-- Attachment #1.1: Type: text/plain, Size: 88 bytes --]

Done with commit 0862b95433cacf91e44248097caa09119fc532a6.

Kind regards,

T G-R


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 504 bytes --]

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

* bug#27735: [PATCH 1/2] build, vm: Use a slightly less generic label.
  2017-07-17 17:20   ` Danny Milosavljevic
  2017-07-17 17:58     ` Tobias Geerinckx-Rice
@ 2017-07-18 10:09     ` Ludovic Courtès
  2017-07-18 12:30       ` Tobias Geerinckx-Rice
  1 sibling, 1 reply; 20+ messages in thread
From: Ludovic Courtès @ 2017-07-18 10:09 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 27735

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> Dash is invalid.  Otherwise OK!

Can we do ‘string-map’ to replace dash with underscore, just like we did
‘normalize-label’?

Ludo’.

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

* bug#27735: Unbootable images with GuixSD on... "GuixSD"
  2017-07-17 17:17 ` bug#27735: Unbootable images with GuixSD on... "GuixSD" Danny Milosavljevic
  2017-07-17 18:12   ` Tobias Geerinckx-Rice
@ 2017-07-18 11:49   ` Ludovic Courtès
  2017-07-18 15:09     ` Tobias Geerinckx-Rice
  2017-07-19 19:11     ` Danny Milosavljevic
  1 sibling, 2 replies; 20+ messages in thread
From: Ludovic Courtès @ 2017-07-18 11:49 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 27735

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

Hello,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

>> The real problem here is that we're using a label as a UUID.
>
> I agree.  Unfortunately Guix UUIDs are difficult to use consistently or I would have changed it over to begin with.

What about generating a UUID in a deterministic yet somewhat unique
fashion along these lines (untested):


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 3494 bytes --]

diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index ec3fb031a..6b53eacbf 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -56,9 +56,12 @@
   #:use-module (gnu system file-systems)
   #:use-module (gnu system)
   #:use-module (gnu services)
+  #:use-module ((gnu build file-systems)
+                #:select (string->iso9660-uuid))
 
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
+  #:use-module (rnrs bytevectors)
   #:use-module (ice-9 match)
 
   #:export (expression->derivation-in-linux-vm
@@ -344,12 +347,29 @@ to USB sticks meant to be read-only."
     (if (string=? "iso9660" file-system-type)
         string-upcase
         identity))
+
   (define root-label
-    ;; Volume name of the root file system.  Since we don't know which device
-    ;; will hold it, we use the volume name to find it (using the UUID would
-    ;; be even better, but somewhat less convenient.)
+    ;; Volume name of the root file system.
     (normalize-label "GuixSD_image"))
 
+  (define root-uuid
+    ;; UUID of the root file system, computed in a deterministic fashion.
+    (if (string=? "iso9660" file-system-type)
+        (let ((pad (compose (cut string-pad <> 2 #\0)
+                            number->string)))
+          (string->iso9660-uuid
+           (string-append "1970-01-01-"
+                          (pad (hash name 24))
+                          (pad (hash file-system-type 60))
+                          (pad (hash (operating-system-host-name os) 60)))))
+        (uint-list->bytevector
+         (list (hash (string-append file-system-type name)
+                     (expt 2 64))
+               (hash (operating-system-host-name os)
+                     (expt 2 64)))
+         (endianness little)
+         8)))
+
   (define file-systems-to-keep
     (remove (lambda (fs)
               (string=? (file-system-mount-point fs) "/"))
@@ -367,8 +387,8 @@ to USB sticks meant to be read-only."
               ;; Force our own root file system.
               (file-systems (cons (file-system
                                     (mount-point "/")
-                                    (device root-label)
-                                    (title 'label)
+                                    (device root-uuid)
+                                    (title 'uuid)
                                     (type file-system-type))
                                   file-systems-to-keep)))))
 
@@ -376,8 +396,7 @@ to USB sticks meant to be read-only."
                          (bootcfg  (operating-system-bootcfg os)))
       (if (string=? "iso9660" file-system-type)
           (iso9660-image #:name name
-                         #:file-system-label root-label
-                         #:file-system-uuid #f
+                         #:file-system-uuid root-uuid
                          #:os-drv os-drv
                          #:bootcfg-drv bootcfg
                          #:bootloader (bootloader-configuration-bootloader
@@ -395,7 +414,7 @@ to USB sticks meant to be read-only."
                                                        file-system-type)
                                              "ext4"
                                              file-system-type)
-                      #:file-system-label root-label
+                      #:file-system-label root-label ;FIXME: use ROOT-UUID
                       #:copy-inputs? #t
                       #:register-closures? #t
                       #:inputs `(("system" ,os-drv)

[-- Attachment #3: Type: text/plain, Size: 145 bytes --]


We cannot use the store file name’s hash, unfortunately, because the
UUID has to be given on the “host side.”

Thoughts?

Ludo’.

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

* bug#27735: [PATCH 1/2] build, vm: Use a slightly less generic label.
  2017-07-18 10:09     ` Ludovic Courtès
@ 2017-07-18 12:30       ` Tobias Geerinckx-Rice
  2017-07-18 13:48         ` Danny Milosavljevic
  0 siblings, 1 reply; 20+ messages in thread
From: Tobias Geerinckx-Rice @ 2017-07-18 12:30 UTC (permalink / raw)
  To: ludo, dannym; +Cc: 27735


[-- Attachment #1.1: Type: text/plain, Size: 235 bytes --]

Ludo',

On 18/07/17 12:09, Ludovic Courtès wrote:
> Can we do ‘string-map’ to replace dash with underscore, just like we did
> ‘normalize-label’?

That would have been a better approach. :-)

Kind regards,

T G-R


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 504 bytes --]

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

* bug#27735: [PATCH 1/2] build, vm: Use a slightly less generic label.
  2017-07-18 12:30       ` Tobias Geerinckx-Rice
@ 2017-07-18 13:48         ` Danny Milosavljevic
  0 siblings, 0 replies; 20+ messages in thread
From: Danny Milosavljevic @ 2017-07-18 13:48 UTC (permalink / raw)
  To: Tobias Geerinckx-Rice; +Cc: 27735

Hi T G-R,
Hi Ludo,

On Tue, 18 Jul 2017 14:30:10 +0200
Tobias Geerinckx-Rice <me@tobias.gr> wrote:

> On 18/07/17 12:09, Ludovic Courtès wrote:
> > Can we do ‘string-map’ to replace dash with underscore, just like we did
> > ‘normalize-label’?  

Yes.

> That would have been a better approach. :-)

I disagree that that would be better.  Every map is a source of error and also makes it difficult to follow what is going on (i.e. difficult to maintain).

Best is only to use the common intersection of all the valid charsets and not map anything to anything else.

We can map it and it's OK but it's not good in the long run.  I've learnt that the hard way over many years.

But since we do string-upcase already, it doesn't matter anymore.  Better hope we don't break it by accident one day (or right away), though.

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

* bug#27735: Unbootable images with GuixSD on... "GuixSD"
  2017-07-18 11:49   ` Ludovic Courtès
@ 2017-07-18 15:09     ` Tobias Geerinckx-Rice
  2017-07-18 18:59       ` Ludovic Courtès
  2017-07-19 19:11     ` Danny Milosavljevic
  1 sibling, 1 reply; 20+ messages in thread
From: Tobias Geerinckx-Rice @ 2017-07-18 15:09 UTC (permalink / raw)
  To: ludo, dannym; +Cc: 27735


[-- Attachment #1.1: Type: text/plain, Size: 1483 bytes --]

Ludo',

On 18/07/17 13:49, Ludovic Courtès wrote:
> What about generating a UUID in a deterministic yet somewhat unique
> fashion along these lines (untested):

Not great, but I can't think of a better way. :-)

> +  (define root-uuid
> +    ;; UUID of the root file system, computed in a deterministic fashion.
> +    (if (string=? "iso9660" file-system-type)
> +        (let ((pad (compose (cut string-pad <> 2 #\0)
> +                            number->string)))
> +          (string->iso9660-uuid
> +           (string-append "1970-01-01-"
> +                          (pad (hash name 24))
> +                          (pad (hash file-system-type 60))
> +                          (pad (hash (operating-system-host-name os) 60)))))
> +        (uint-list->bytevector
> +         (list (hash (string-append file-system-type name)
> +                     (expt 2 64))
> +               (hash (operating-system-host-name os)
> +                     (expt 2 64)))
> +         (endianness little)
> +         8)))
> +

Why not throw SIZE into this mix as well?

When building without ‘--image-size’ (the default nowadays), it's a
function of the exact size of the entire graph and reasonably sensitive
to most kinds of input changes.

> We cannot use the store file name’s hash, unfortunately, because the
> UUID has to be given on the “host side.”

That is unfortunate, but a best-effort heuristic will do.

Kind regards,

T G-R


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 504 bytes --]

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

* bug#27735: Unbootable images with GuixSD on... "GuixSD"
  2017-07-18 15:09     ` Tobias Geerinckx-Rice
@ 2017-07-18 18:59       ` Ludovic Courtès
  2017-07-18 20:42         ` Tobias Geerinckx-Rice
  0 siblings, 1 reply; 20+ messages in thread
From: Ludovic Courtès @ 2017-07-18 18:59 UTC (permalink / raw)
  To: Tobias Geerinckx-Rice; +Cc: 27735

Heya,

Tobias Geerinckx-Rice <me@tobias.gr> skribis:

> On 18/07/17 13:49, Ludovic Courtès wrote:
>> What about generating a UUID in a deterministic yet somewhat unique
>> fashion along these lines (untested):
>
> Not great, but I can't think of a better way. :-)

Yup.

>> +  (define root-uuid
>> +    ;; UUID of the root file system, computed in a deterministic fashion.
>> +    (if (string=? "iso9660" file-system-type)
>> +        (let ((pad (compose (cut string-pad <> 2 #\0)
>> +                            number->string)))
>> +          (string->iso9660-uuid
>> +           (string-append "1970-01-01-"
>> +                          (pad (hash name 24))
>> +                          (pad (hash file-system-type 60))
>> +                          (pad (hash (operating-system-host-name os) 60)))))
>> +        (uint-list->bytevector
>> +         (list (hash (string-append file-system-type name)
>> +                     (expt 2 64))
>> +               (hash (operating-system-host-name os)
>> +                     (expt 2 64)))
>> +         (endianness little)
>> +         8)))
>> +
>
> Why not throw SIZE into this mix as well?

Because it can be the symbol 'guess or a number, so that makes things
needlessly complicated IMO.

> When building without ‘--image-size’ (the default nowadays), it's a
> function of the exact size of the entire graph and reasonably sensitive
> to most kinds of input changes.

The actual size is not known until the derivation is built; we don’t
have access to that information here.

>> We cannot use the store file name’s hash, unfortunately, because the
>> UUID has to be given on the “host side.”
>
> That is unfortunate, but a best-effort heuristic will do.

Yeah, still better than a hard-coded label I guess.

Ludo’.

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

* bug#27735: Unbootable images with GuixSD on... "GuixSD"
  2017-07-18 18:59       ` Ludovic Courtès
@ 2017-07-18 20:42         ` Tobias Geerinckx-Rice
  0 siblings, 0 replies; 20+ messages in thread
From: Tobias Geerinckx-Rice @ 2017-07-18 20:42 UTC (permalink / raw)
  To: ludo; +Cc: 27735


[-- Attachment #1.1: Type: text/plain, Size: 366 bytes --]

Ludo',

On 18/07/17 20:59, Ludovic Courtès wrote:
>> Why not throw SIZE into this mix as well?
> 
> Because it can be the symbol 'guess or a number, so that makes things
> needlessly complicated IMO.

Of course. Thanks for your patience. I got completely lost in the
gexp/ungexps of expression->derivation-in-linux-vm. Stupid.

Kind regards,

T G-R


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 504 bytes --]

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

* bug#27735: Unbootable images with GuixSD on... "GuixSD"
  2017-07-18 11:49   ` Ludovic Courtès
  2017-07-18 15:09     ` Tobias Geerinckx-Rice
@ 2017-07-19 19:11     ` Danny Milosavljevic
  2017-07-19 22:32       ` bug#27735: Lookup by UUID Ludovic Courtès
  1 sibling, 1 reply; 20+ messages in thread
From: Danny Milosavljevic @ 2017-07-19 19:11 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 27735

Hi Ludo,

I think it's a good interim solution.

On Tue, 18 Jul 2017 13:49:01 +0200
ludo@gnu.org (Ludovic Courtès) wrote:

> +  (define root-uuid
> +    ;; UUID of the root file system, computed in a deterministic fashion.
> +    (if (string=? "iso9660" file-system-type)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +        (let ((pad (compose (cut string-pad <> 2 #\0)
> +                            number->string)))
> +          (string->iso9660-uuid
> +           (string-append "1970-01-01-"
> +                          (pad (hash name 24))
> +                          (pad (hash file-system-type 60))
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^ hmm... isn't that constant?

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

* bug#27735: Lookup by UUID
  2017-07-19 19:11     ` Danny Milosavljevic
@ 2017-07-19 22:32       ` Ludovic Courtès
  2017-07-20 17:38         ` Danny Milosavljevic
  0 siblings, 1 reply; 20+ messages in thread
From: Ludovic Courtès @ 2017-07-19 22:32 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 27735

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

Hi!

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> I think it's a good interim solution.

Based on your feedback I’ve come up with the two attached patches.  I’ve
checked at the REPL that ‘operating-system-uuid’ gives reasonable
results for different ‘operating-system’ configs, and deterministic
results for a given config (OSes that are not ‘eq?’ but that are equal.)

On ext4 “guix system disk-image” produces an image that works like a
charm.

With iso9660, it works… by chance, because GRUB’s “search --fs-uuid”
fails.  Guess why?  Because it compares UUIDs as strings, and we format
it as a DCE UUID instead of an ISO UUID.  Sounds familiar no?  :-)

So that’s where we are.  Thoughts on how to address it?

Cheers,
Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: the first patch --]
[-- Type: text/x-patch, Size: 3891 bytes --]

From 00d49f0199dc51b02f2113c3669ea07f4461b102 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Thu, 20 Jul 2017 00:15:43 +0200
Subject: [PATCH] vm: Allow partitions to be initialized with a given UUID.

* gnu/build/vm.scm (<partition>)[uuid]: New field.
(create-ext-file-system): Add #:uuid and honor it.
(create-fat-file-system): Add #:uuid.
(format-partition): Add #:uuid and honor it.
(initialize-partition): Honor the 'uuid' field of PARTITION.
---
 gnu/build/vm.scm | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 727494ad9..8dfaf2789 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -163,6 +163,7 @@ the #:references-graphs parameter of 'derivation'."
   (size        partition-size)
   (file-system partition-file-system (default "ext4"))
   (label       partition-label (default #f))
+  (uuid        partition-uuid (default #f))
   (flags       partition-flags (default '()))
   (initializer partition-initializer (default (const #t))))
 
@@ -236,22 +237,26 @@ actual /dev name based on DEVICE."
 (define MS_BIND 4096)                             ; <sys/mounts.h> again!
 
 (define* (create-ext-file-system partition type
-                                 #:key label)
+                                 #:key label uuid)
   "Create an ext-family filesystem of TYPE on PARTITION.  If LABEL is true,
-use that as the volume name."
+use that as the volume name.  If UUID is true, use it as the partition UUID."
   (format #t "creating ~a partition...\n" type)
   (unless (zero? (apply system* (string-append "mkfs." type)
                         "-F" partition
-                        (if label
-                            `("-L" ,label)
-                            '())))
+                        `(,@(if label
+                                `("-L" ,label)
+                                '())
+                          ,@(if uuid
+                                `("-U" ,(uuid->string uuid))
+                                '()))))
     (error "failed to create partition")))
 
 (define* (create-fat-file-system partition
-                                 #:key label)
+                                 #:key label uuid)
   "Create a FAT filesystem on PARTITION.  The number of File Allocation Tables
 will be determined based on filesystem size.  If LABEL is true, use that as the
 volume name."
+  ;; FIXME: UUID is ignored!
   (format #t "creating FAT partition...\n")
   (unless (zero? (apply system* "mkfs.fat" partition
                         (if label
@@ -260,13 +265,13 @@ volume name."
     (error "failed to create FAT partition")))
 
 (define* (format-partition partition type
-                           #:key label)
+                           #:key label uuid)
   "Create a file system TYPE on PARTITION.  If LABEL is true, use that as the
 volume name."
   (cond ((string-prefix? "ext" type)
-         (create-ext-file-system partition type #:label label))
+         (create-ext-file-system partition type #:label label #:uuid uuid))
         ((or (string-prefix? "fat" type) (string= "vfat" type))
-         (create-fat-file-system partition #:label label))
+         (create-fat-file-system partition #:label label #:uuid uuid))
         (else (error "Unsupported file system."))))
 
 (define (initialize-partition partition)
@@ -275,7 +280,8 @@ it, run its initializer, and unmount it."
   (let ((target "/fs"))
    (format-partition (partition-device partition)
                      (partition-file-system partition)
-                     #:label (partition-label partition))
+                     #:label (partition-label partition)
+                     #:uuid (partition-uuid partition))
    (mkdir-p target)
    (mount (partition-device partition) target
           (partition-file-system partition))
-- 
2.13.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: Type: text/x-patch, Size: 5013 bytes --]

diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 6f979aee4..bd1e1b3e5 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -56,9 +56,12 @@
   #:use-module (gnu system file-systems)
   #:use-module (gnu system)
   #:use-module (gnu services)
+  #:use-module ((gnu build file-systems)
+                #:select (string->iso9660-uuid))
 
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
+  #:use-module (rnrs bytevectors)
   #:use-module (ice-9 match)
 
   #:export (expression->derivation-in-linux-vm
@@ -234,6 +237,7 @@ INPUTS is a list of inputs (as for packages)."
                      (disk-image-format "qcow2")
                      (file-system-type "ext4")
                      file-system-label
+                     file-system-uuid
                      os-drv
                      bootcfg-drv
                      bootloader
@@ -293,6 +297,7 @@ the image."
                   (partitions (list (partition
                                      (size root-size)
                                      (label #$file-system-label)
+                                     (uuid #$file-system-uuid)
                                      (file-system #$file-system-type)
                                      (flags '(boot))
                                      (initializer initialize))
@@ -330,6 +335,31 @@ the image."
 ;;; VM and disk images.
 ;;;
 
+(define* (operating-system-uuid os #:optional (type 'dce))
+  "Compute a deterministic \"UUID\" for OS, of the given TYPE (one of 'iso9660
+or 'dce)."
+  (if (eq? type 'iso9660)
+      (let ((pad (compose (cut string-pad <> 2 #\0)
+                          number->string))
+            (h   (hash (operating-system-services os) 3600)))
+        (string->iso9660-uuid
+         (string-append "1970-01-01-"
+                        (pad (hash (operating-system-host-name os) 24)) "-"
+                        (pad (quotient h 60)) "-"
+                        (pad (modulo h 60)) "-"
+                        (pad (hash (operating-system-file-systems os) 100)))))
+      (uint-list->bytevector
+       (list (hash file-system-type
+                   (expt 2 32))
+             (hash (operating-system-host-name os)
+                   (expt 2 32))
+             (hash (operating-system-services os)
+                   (expt 2 32))
+             (hash (operating-system-file-systems os)
+                   (expt 2 32)))
+       (endianness little)
+       4)))
+
 (define* (system-disk-image os
                             #:key
                             (name "disk-image")
@@ -346,12 +376,20 @@ to USB sticks meant to be read-only."
     (if (string=? "iso9660" file-system-type)
         string-upcase
         identity))
+
   (define root-label
-    ;; Volume name of the root file system.  Since we don't know which device
-    ;; will hold it, we use the volume name to find it (using the UUID would
-    ;; be even better, but somewhat less convenient.)
+    ;; Volume name of the root file system.
     (normalize-label "GuixSD_image"))
 
+  (define root-uuid
+    ;; UUID of the root file system, computed in a deterministic fashion.
+    ;; This is what we use to locate the root file system so it has to be
+    ;; different from the user's own file system UUIDs.
+    (operating-system-uuid os
+                           (if (string=? file-system-type "iso9660")
+                               'iso9660
+                               'dce)))
+
   (define file-systems-to-keep
     (remove (lambda (fs)
               (string=? (file-system-mount-point fs) "/"))
@@ -369,8 +407,8 @@ to USB sticks meant to be read-only."
               ;; Force our own root file system.
               (file-systems (cons (file-system
                                     (mount-point "/")
-                                    (device root-label)
-                                    (title 'label)
+                                    (device root-uuid)
+                                    (title 'uuid)
                                     (type file-system-type))
                                   file-systems-to-keep)))))
 
@@ -379,7 +417,7 @@ to USB sticks meant to be read-only."
       (if (string=? "iso9660" file-system-type)
           (iso9660-image #:name name
                          #:file-system-label root-label
-                         #:file-system-uuid #f
+                         #:file-system-uuid root-uuid
                          #:os-drv os-drv
                          #:bootcfg-drv bootcfg
                          #:bootloader (bootloader-configuration-bootloader
@@ -398,6 +436,7 @@ to USB sticks meant to be read-only."
                                              "ext4"
                                              file-system-type)
                       #:file-system-label root-label
+                      #:file-system-uuid root-uuid
                       #:copy-inputs? #t
                       #:register-closures? #t
                       #:inputs `(("system" ,os-drv)

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

* bug#27735: Lookup by UUID
  2017-07-19 22:32       ` bug#27735: Lookup by UUID Ludovic Courtès
@ 2017-07-20 17:38         ` Danny Milosavljevic
  2017-07-20 20:32           ` Ludovic Courtès
  0 siblings, 1 reply; 20+ messages in thread
From: Danny Milosavljevic @ 2017-07-20 17:38 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 27735

Hi Ludo,

> With iso9660, it works… by chance, because GRUB’s “search --fs-uuid”
> fails.  Guess why?  Because it compares UUIDs as strings, and we format
> it as a DCE UUID instead of an ISO UUID.  Sounds familiar no?  :-)

Yeah :)

> So that’s where we are.  Thoughts on how to address it?

https://www.gnu.org/software/grub/manual/grub.html#Making-a-GRUB-bootable-CD_002dROM says:

>...grub-mkrescue
>This produces a file named grub.iso, which then can be burned into a CD (or a DVD), or written to a USB mass storage device.
>The root device will be set up appropriately on entering your grub.cfg configuration file, so you can refer to file names on the CD without needing to use an explicit device name. This makes it easier to produce rescue images that will work on both optical drives and USB mass storage devices. 

So... just leave the entire "search" instruction off if it's created by grub-mkrescue ?  That's how one could interpret thir "root device" sentence...

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

* bug#27735: Lookup by UUID
  2017-07-20 17:38         ` Danny Milosavljevic
@ 2017-07-20 20:32           ` Ludovic Courtès
  2017-07-20 21:51             ` Danny Milosavljevic
  2017-08-05 17:50             ` Danny Milosavljevic
  0 siblings, 2 replies; 20+ messages in thread
From: Ludovic Courtès @ 2017-07-20 20:32 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 27735

Heya,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

>> With iso9660, it works… by chance, because GRUB’s “search --fs-uuid”
>> fails.  Guess why?  Because it compares UUIDs as strings, and we format
>> it as a DCE UUID instead of an ISO UUID.  Sounds familiar no?  :-)
>
> Yeah :)
>
>> So that’s where we are.  Thoughts on how to address it?
>
> https://www.gnu.org/software/grub/manual/grub.html#Making-a-GRUB-bootable-CD_002dROM says:
>
>>...grub-mkrescue
>>This produces a file named grub.iso, which then can be burned into a CD (or a DVD), or written to a USB mass storage device.
>>The root device will be set up appropriately on entering your grub.cfg configuration file, so you can refer to file names on the CD without needing to use an explicit device name. This makes it easier to produce rescue images that will work on both optical drives and USB mass storage devices. 
>
> So... just leave the entire "search" instruction off if it's created by grub-mkrescue ?  That's how one could interpret thir "root device" sentence...

Oooh, interesting, that comes in handy.  :-)

Now, how can we pass the information to ‘grub-configuration-file’ so
that it doesn’t emit that “search” command?

It seems that the bootloader API doesn’t leave room to pass
bootloader-specific options.

Thoughts?

Ludo’.

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

* bug#27735: Lookup by UUID
  2017-07-20 20:32           ` Ludovic Courtès
@ 2017-07-20 21:51             ` Danny Milosavljevic
  2017-08-05 17:50             ` Danny Milosavljevic
  1 sibling, 0 replies; 20+ messages in thread
From: Danny Milosavljevic @ 2017-07-20 21:51 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 27735

Hi Ludo,

On Thu, 20 Jul 2017 22:32:29 +0200
ludo@gnu.org (Ludovic Courtès) wrote:

> > So... just leave the entire "search" instruction off if it's created by grub-mkrescue ?  That's how one could interpret thir "root device" sentence...  
> 
> Oooh, interesting, that comes in handy.  :-)
> 
> Now, how can we pass the information to ‘grub-configuration-file’ so
> that it doesn’t emit that “search” command?
> 
> It seems that the bootloader API doesn’t leave room to pass
> bootloader-specific options.

I can't believe I'm suggesting this (we've come full circle now) but we could add a new bootloader in gnu/bootloader/grub.scm, 

(define* grub-hybrid-bootloader ; or grub-mkrescue-bootloader, maybe less misleading.
  (bootloader
   (inherit grub-efi-bootloader)
   (configuration-file-generator grub-configuration-file-without-search-emission) ; this here could parametrize grub-configuration-file somehow or maybe we could refactor grub-configuration-file instead.
   ; Note: name is still 'grub-efi, so the public boot parameters API doesn't change.
   (package grub-hybrid))) ; probably; or grub-efi if you just want a quick test with EFI only.

... and use it in iso9660-image or gnu/system/install.scm .

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

* bug#27735: Lookup by UUID
  2017-07-20 20:32           ` Ludovic Courtès
  2017-07-20 21:51             ` Danny Milosavljevic
@ 2017-08-05 17:50             ` Danny Milosavljevic
  1 sibling, 0 replies; 20+ messages in thread
From: Danny Milosavljevic @ 2017-08-05 17:50 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 27735

Hi Ludo,

> > So... just leave the entire "search" instruction off if it's created by grub-mkrescue ?  That's how one could interpret thir "root device" sentence...  
> 
> Oooh, interesting, that comes in handy.  :-)
> 
> Now, how can we pass the information to ‘grub-configuration-file’ so
> that it doesn’t emit that “search” command?

I've gone ahead and pushed bug# 27705 to master.  That means that there's now a new bootloader "grub-mkrescue-bootloader" in gnu/bootloaders/grub.scm . So you can just override its "configuration-file-generator" by one that doesn't emit the "search" command (remainder should be fine as-is).

I wonder what would happen (to Grub) if someone did guix system reconfigure in a system booted from the ISO9660 image, though.  I think it would be better to not install any bootloader in the case of mkrescue and guix system reconfigure.  We might want to do a follow-up commit that adds a dummy installer to the "installer" field of grub-mkrescue-bootloader.  WDYT?

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

end of thread, other threads:[~2017-08-05 17:51 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-17 14:40 bug#27735: Unbootable images with GuixSD on... "GuixSD" Tobias Geerinckx-Rice
2017-07-17 14:51 ` bug#27735: [PATCH 1/2] build, vm: Use a slightly less generic label Tobias Geerinckx-Rice
2017-07-17 17:20   ` Danny Milosavljevic
2017-07-17 17:58     ` Tobias Geerinckx-Rice
2017-07-18 10:09     ` Ludovic Courtès
2017-07-18 12:30       ` Tobias Geerinckx-Rice
2017-07-18 13:48         ` Danny Milosavljevic
2017-07-17 17:17 ` bug#27735: Unbootable images with GuixSD on... "GuixSD" Danny Milosavljevic
2017-07-17 18:12   ` Tobias Geerinckx-Rice
2017-07-17 18:37     ` Tobias Geerinckx-Rice
2017-07-18 11:49   ` Ludovic Courtès
2017-07-18 15:09     ` Tobias Geerinckx-Rice
2017-07-18 18:59       ` Ludovic Courtès
2017-07-18 20:42         ` Tobias Geerinckx-Rice
2017-07-19 19:11     ` Danny Milosavljevic
2017-07-19 22:32       ` bug#27735: Lookup by UUID Ludovic Courtès
2017-07-20 17:38         ` Danny Milosavljevic
2017-07-20 20:32           ` Ludovic Courtès
2017-07-20 21:51             ` Danny Milosavljevic
2017-08-05 17:50             ` Danny Milosavljevic

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