GNU bug report logs - #40662
[PATCH 0/2] Add efivarfs support.

Previous Next

Package: guix-patches;

Reported by: Mathieu Othacehe <m.othacehe <at> gmail.com>

Date: Thu, 16 Apr 2020 15:05:02 UTC

Severity: normal

Tags: patch

Done: Mathieu Othacehe <othacehe <at> gnu.org>

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 40662 in the body.
You can then email your comments to 40662 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#40662; Package guix-patches. (Thu, 16 Apr 2020 15:05:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Mathieu Othacehe <m.othacehe <at> gmail.com>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Thu, 16 Apr 2020 15:05:02 GMT) Full text and rfc822 format available.

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

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: Mathieu Othacehe <m.othacehe <at> gmail.com>
Subject: [PATCH 0/2] Add efivarfs support.
Date: Thu, 16 Apr 2020 17:04:36 +0200
Hello,

Here's a small serie to add support for efivarfs and fix the issue reported
here: https://lists.gnu.org/archive/html/bug-guix/2020-04/msg00274.html.

The first patch is a bit hacky, so I didn't bother to write the documentation,
in case we find another solution.

Thanks,

Mathieu

Mathieu Othacehe (2):
  file-system: Add mount-may-fail? option.
  file-system: Add efivarfs support.

 gnu/build/file-systems.scm  | 49 +++++++++++++++++++++----------------
 gnu/system/file-systems.scm | 23 ++++++++++++++---
 gnu/system/install.scm      |  1 +
 3 files changed, 49 insertions(+), 24 deletions(-)

-- 
2.26.0





Information forwarded to guix-patches <at> gnu.org:
bug#40662; Package guix-patches. (Thu, 16 Apr 2020 15:07:01 GMT) Full text and rfc822 format available.

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

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: 40662 <at> debbugs.gnu.org.
Cc: Mathieu Othacehe <m.othacehe <at> gmail.com>
Subject: [PATCH 1/2] file-system: Add mount-may-fail? option.
Date: Thu, 16 Apr 2020 17:05:47 +0200
* gnu/system/file-systems.scm (<file-system>): Add a mount-may-fail? field.
(file-system->spec): adapt accordingly,
(spec->file-system): ditto.
* gnu/build/file-systems.scm (mount-file-system): If 'system-error is raised
and mount-may-fail? is true, ignore it. Otherwise, re-raise the exception.
---
 gnu/build/file-systems.scm  | 49 +++++++++++++++++++++----------------
 gnu/system/file-systems.scm | 11 ++++++---
 2 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/gnu/build/file-systems.scm b/gnu/build/file-systems.scm
index 902563b219..2f7987f334 100644
--- a/gnu/build/file-systems.scm
+++ b/gnu/build/file-systems.scm
@@ -673,26 +673,33 @@ corresponds to the symbols listed in FLAGS."
     (when (file-system-check? fs)
       (check-file-system source type))
 
-    ;; Create the mount point.  Most of the time this is a directory, but
-    ;; in the case of a bind mount, a regular file or socket may be needed.
-    (if (and (= MS_BIND (logand flags MS_BIND))
-             (not (file-is-directory? source)))
-        (unless (file-exists? mount-point)
-          (mkdir-p (dirname mount-point))
-          (call-with-output-file mount-point (const #t)))
-        (mkdir-p mount-point))
-
-    (cond
-     ((string-prefix? "nfs" type)
-      (mount-nfs source mount-point type flags options))
-     (else
-      (mount source mount-point type flags options)))
-
-    ;; For read-only bind mounts, an extra remount is needed, as per
-    ;; <http://lwn.net/Articles/281157/>, which still applies to Linux 4.0.
-    (when (and (= MS_BIND (logand flags MS_BIND))
-               (= MS_RDONLY (logand flags MS_RDONLY)))
-      (let ((flags (logior MS_BIND MS_REMOUNT MS_RDONLY)))
-        (mount source mount-point type flags #f)))))
+    (catch 'system-error
+      (lambda ()
+        ;; Create the mount point.  Most of the time this is a directory, but
+        ;; in the case of a bind mount, a regular file or socket may be
+        ;; needed.
+        (if (and (= MS_BIND (logand flags MS_BIND))
+                 (not (file-is-directory? source)))
+            (unless (file-exists? mount-point)
+              (mkdir-p (dirname mount-point))
+              (call-with-output-file mount-point (const #t)))
+            (mkdir-p mount-point))
+
+        (cond
+         ((string-prefix? "nfs" type)
+          (mount-nfs source mount-point type flags options))
+         (else
+          (mount source mount-point type flags options)))
+
+        ;; For read-only bind mounts, an extra remount is needed, as per
+        ;; <http://lwn.net/Articles/281157/>, which still applies to Linux
+        ;; 4.0.
+        (when (and (= MS_BIND (logand flags MS_BIND))
+                   (= MS_RDONLY (logand flags MS_RDONLY)))
+          (let ((flags (logior MS_BIND MS_REMOUNT MS_RDONLY)))
+            (mount source mount-point type flags #f))))
+      (lambda args
+        (or (file-system-mount-may-fail? fs)
+            (apply throw args))))))
 
 ;;; file-systems.scm ends here
diff --git a/gnu/system/file-systems.scm b/gnu/system/file-systems.scm
index 3b599efa8e..5180eca8d9 100644
--- a/gnu/system/file-systems.scm
+++ b/gnu/system/file-systems.scm
@@ -38,6 +38,7 @@
             file-system-flags
             file-system-options
             file-system-mount?
+            file-system-mount-may-fail?
             file-system-check?
             file-system-create-mount-point?
             file-system-dependencies
@@ -101,6 +102,8 @@
                     (default #f))
   (mount?           file-system-mount?            ; Boolean
                     (default #t))
+  (mount-may-fail?  file-system-mount-may-fail?   ; Boolean
+                    (default #f))
   (needed-for-boot? %file-system-needed-for-boot? ; Boolean
                     (default #f))
   (check?           file-system-check?            ; Boolean
@@ -261,18 +264,19 @@ store--e.g., if FS is the root file system."
   "Return a list corresponding to file-system FS that can be passed to the
 initrd code."
   (match fs
-    (($ <file-system> device mount-point type flags options _ _ check?)
+    (($ <file-system> device mount-point type flags options mount?
+                      mount-may-fail? needed-for-boot? check?)
      (list (cond ((uuid? device)
                   `(uuid ,(uuid-type device) ,(uuid-bytevector device)))
                  ((file-system-label? device)
                   `(file-system-label ,(file-system-label->string device)))
                  (else device))
-           mount-point type flags options check?))))
+           mount-point type flags options mount-may-fail? check?))))
 
 (define (spec->file-system sexp)
   "Deserialize SEXP, a list, to the corresponding <file-system> object."
   (match sexp
-    ((device mount-point type flags options check?)
+    ((device mount-point type flags options mount-may-fail? check?)
      (file-system
        (device (match device
                  (('uuid (? symbol? type) (? bytevector? bv))
@@ -283,6 +287,7 @@ initrd code."
                   device)))
        (mount-point mount-point) (type type)
        (flags flags) (options options)
+       (mount-may-fail? mount-may-fail?)
        (check? check?)))))
 
 (define (specification->file-system-mapping spec writable?)
-- 
2.26.0





Information forwarded to guix-patches <at> gnu.org:
bug#40662; Package guix-patches. (Thu, 16 Apr 2020 15:07:02 GMT) Full text and rfc822 format available.

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

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: 40662 <at> debbugs.gnu.org.
Cc: Mathieu Othacehe <m.othacehe <at> gmail.com>
Subject: [PATCH 2/2] file-system: Add efivarfs support.
Date: Thu, 16 Apr 2020 17:05:48 +0200
Tools such as efibootmgr rely on the deprecated /sys/firmware/efi/vars API as
well as on the new /sys/firmware/efi/efivars API. The later needs to be
mounted.

Reported by Keyhenge here:
https://lists.gnu.org/archive/html/bug-guix/2020-04/msg00274.html

Here is the efivarfs documentation:
https://www.kernel.org/doc/Documentation/filesystems/efivarfs.txt.

* gnu/system/file-systems.scm (%efivars-file-system): New exported variable,
(%base-file-systems): add it.
* gnu/system/install.scm (%efivars-file-system): Add it.
---
 gnu/system/file-systems.scm | 12 ++++++++++++
 gnu/system/install.scm      |  1 +
 2 files changed, 13 insertions(+)

diff --git a/gnu/system/file-systems.scm b/gnu/system/file-systems.scm
index 5180eca8d9..644a90d42b 100644
--- a/gnu/system/file-systems.scm
+++ b/gnu/system/file-systems.scm
@@ -57,6 +57,7 @@
             %pseudo-file-system-types
             %fuse-control-file-system
             %binary-format-file-system
+            %efivars-file-system
             %shared-memory-file-system
             %pseudo-terminal-file-system
             %tty-gid
@@ -334,6 +335,16 @@ TARGET in the other system."
     (type "binfmt_misc")
     (check? #f)))
 
+(define %efivars-file-system
+  ;; Support for EFI variables file system.
+  (file-system
+    (device "efivarfs")
+    (mount-point "/sys/firmware/efi/efivars")
+    (type "efivarfs")
+    (mount-may-fail? #t)
+    (needed-for-boot? #f)
+    (check? #f)))
+
 (define %tty-gid
   ;; ID of the 'tty' group.  Allocate it statically to make it easy to refer
   ;; to it from here and from the 'tty' group definitions.
@@ -434,6 +445,7 @@ TARGET in the other system."
   ;; currently mounted by the initrd.
   (list %pseudo-terminal-file-system
         %shared-memory-file-system
+        %efivars-file-system
         %immutable-store))
 
 ;; File systems for Linux containers differ from %base-file-systems in that
diff --git a/gnu/system/install.scm b/gnu/system/install.scm
index 0965c4d237..e78c61ed62 100644
--- a/gnu/system/install.scm
+++ b/gnu/system/install.scm
@@ -501,6 +501,7 @@ Access documentation at any time by pressing Alt-F2.\x1b[0m
             ;; elogind's cgroup file systems.
             (list %pseudo-terminal-file-system
                   %shared-memory-file-system
+                  %efivars-file-system
                   %immutable-store)))
 
     (users (list (user-account
-- 
2.26.0





Information forwarded to guix-patches <at> gnu.org:
bug#40662; Package guix-patches. (Thu, 30 Apr 2020 12:47:02 GMT) Full text and rfc822 format available.

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

From: "pelzflorian (Florian Pelz)" <pelzflorian <at> pelzflorian.de>
To: Mathieu Othacehe <m.othacehe <at> gmail.com>
Cc: 40662 <at> debbugs.gnu.org
Subject: Re: [bug#40662] [PATCH 0/2] Add efivarfs support.
Date: Thu, 30 Apr 2020 14:46:18 +0200
On Thu, Apr 16, 2020 at 05:04:36PM +0200, Mathieu Othacehe wrote:
> Hello,
> 
> Here's a small serie to add support for efivarfs and fix the issue reported
> here: https://lists.gnu.org/archive/html/bug-guix/2020-04/msg00274.html.

I have no idea why I only ever got the [Patch 0/2] e-mail delivered to
my inbox.  I look at <https://issues.guix.info/issue/40662#1> now.


> [PATCH 2/2] file-system: Add efivarfs support.
> […]
> Tools such as efibootmgr rely on the deprecated /sys/firmware/efi/vars API as
> well as on the new /sys/firmware/efi/efivars API. The later needs to be
> mounted.

Typo: The latter needs to be mounted.


Well I had no issues with efivars on install without your patch.  I
will try to provoke the error by adding more efivars.  I did not yet
manage to create a new efivar via dd or via the efivar program though.

Regards,
Florian




Information forwarded to guix-patches <at> gnu.org:
bug#40662; Package guix-patches. (Thu, 30 Apr 2020 15:42:01 GMT) Full text and rfc822 format available.

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

From: "pelzflorian (Florian Pelz)" <pelzflorian <at> pelzflorian.de>
To: Mathieu Othacehe <m.othacehe <at> gmail.com>
Cc: 40662 <at> debbugs.gnu.org
Subject: Re: [bug#40662] [PATCH 0/2] Add efivarfs support.
Date: Thu, 30 Apr 2020 17:41:50 +0200
On Thu, Apr 30, 2020 at 02:46:18PM +0200, pelzflorian (Florian Pelz) wrote:
> On Thu, Apr 16, 2020 at 05:04:36PM +0200, Mathieu Othacehe wrote:
> > Here's a small serie to add support for efivarfs and fix the issue reported
> > here: https://lists.gnu.org/archive/html/bug-guix/2020-04/msg00274.html.
> Well I had no issues with efivars on install without your patch.  I
> will try to provoke the error by adding more efivars.  I did not yet
> manage to create a new efivar via dd or via the efivar program though.

Actually I see no reason to test after provoking Keyhenge’s error.
How does your patch fix a full NVRAM?  I thought Keyhenge resolved
their error by deleting “files” from the full NVRAM.  Do I
misunderstand Keyhenge’s issue?

But /sys/firmware/efi/efivars probably should be mounted anyway.

Do you know why

efivarfs on /sys/firmware/efi/efivars type efivarfs (rw,nosuid,nodev,noexec,relatime)

is mounted even without your patch once I add %desktop-services to my
config.scm?

Regards,
Florian




Information forwarded to guix-patches <at> gnu.org:
bug#40662; Package guix-patches. (Thu, 30 Apr 2020 16:07:02 GMT) Full text and rfc822 format available.

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

From: "pelzflorian (Florian Pelz)" <pelzflorian <at> pelzflorian.de>
To: Mathieu Othacehe <m.othacehe <at> gmail.com>
Cc: 40662 <at> debbugs.gnu.org
Subject: Re: [bug#40662] [PATCH 0/2] Add efivarfs support.
Date: Thu, 30 Apr 2020 18:06:32 +0200
On Thu, Apr 16, 2020 at 05:04:36PM +0200, Mathieu Othacehe wrote:
> Here's a small serie to add support for efivarfs and fix the issue reported
> here: https://lists.gnu.org/archive/html/bug-guix/2020-04/msg00274.html.

When I reconfigure the first time with your patch, I get a harmless error:

building /gnu/store/klnbdxbw2jqglrdqyslv87c9y72g80f7-upgrade-shepherd-services.scm.drv
guix system: error: exception caught while executing 'start' on service 'file-system-/sys/firmware/efi/efivars':
Throw to key `match-error' with args `("match" "no matching pattern" ("efivarfs" "/sys/firmware/efi/efivars" "efivarfs" () #f #t #f))

I don’t know why it happens.

Regards,
Florian




Information forwarded to guix-patches <at> gnu.org:
bug#40662; Package guix-patches. (Fri, 01 May 2020 09:17:02 GMT) Full text and rfc822 format available.

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

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: "pelzflorian \(Florian Pelz\)" <pelzflorian <at> pelzflorian.de>
Cc: 40662 <at> debbugs.gnu.org
Subject: Re: [bug#40662] [PATCH 0/2] Add efivarfs support.
Date: Fri, 01 May 2020 11:16:34 +0200
Hey Florian,

Thanks a lot for having a look.

> Actually I see no reason to test after provoking Keyhenge’s error.
> How does your patch fix a full NVRAM?  I thought Keyhenge resolved
> their error by deleting “files” from the full NVRAM.  Do I
> misunderstand Keyhenge’s issue?

I think the full NVRAM is only one part of the issue. There could also
be errors for variables > 1024 bytes using the legacy sysfs API, see:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=933523.

> is mounted even without your patch once I add %desktop-services to my
> config.scm?

Oh, it seems that elogind has support to mount efivarfs.

Mathieu




Information forwarded to guix-patches <at> gnu.org:
bug#40662; Package guix-patches. (Fri, 01 May 2020 09:40:02 GMT) Full text and rfc822 format available.

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

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: "pelzflorian \(Florian Pelz\)" <pelzflorian <at> pelzflorian.de>
Cc: 40662 <at> debbugs.gnu.org
Subject: Re: [bug#40662] [PATCH 0/2] Add efivarfs support.
Date: Fri, 01 May 2020 11:39:42 +0200
> guix system: error: exception caught while executing 'start' on service 'file-system-/sys/firmware/efi/efivars':
> Throw to key `match-error' with args `("match" "no matching pattern" ("efivarfs" "/sys/firmware/efi/efivars" "efivarfs" () #f #t #f))
>
> I don’t know why it happens.

Seems like an ABI incompatibility between the running shepherd and this
patch adding a new field in "file-system->spec" procedure. Not sure it
can be worked around.

Mathieu




Reply sent to Mathieu Othacehe <othacehe <at> gnu.org>:
You have taken responsibility. (Fri, 31 Jul 2020 12:20:01 GMT) Full text and rfc822 format available.

Notification sent to Mathieu Othacehe <m.othacehe <at> gmail.com>:
bug acknowledged by developer. (Fri, 31 Jul 2020 12:20:02 GMT) Full text and rfc822 format available.

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

From: Mathieu Othacehe <othacehe <at> gnu.org>
To: 40662-done <at> debbugs.gnu.org
Cc: "pelzflorian \(Florian Pelz\)" <pelzflorian <at> pelzflorian.de>
Subject: Re: [bug#40662] [PATCH 0/2] Add efivarfs support.
Date: Fri, 31 Jul 2020 14:19:40 +0200
Hello,

I just pushed this one to master. Closing!

Mathieu




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

This bug report was last modified 3 years and 212 days ago.

Previous Next


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