GNU bug report logs - #41924
[PATCH] profiles: Make linux-module-database skip inappropriate inputs

Previous Next

Package: guix-patches;

Reported by: Ivan Kozlov <kanichos <at> yandex.ru>

Date: Wed, 17 Jun 2020 18:45:02 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <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 41924 in the body.
You can then email your comments to 41924 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#41924; Package guix-patches. (Wed, 17 Jun 2020 18:45:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Ivan Kozlov <kanichos <at> yandex.ru>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Wed, 17 Jun 2020 18:45:02 GMT) Full text and rfc822 format available.

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

From: Ivan Kozlov <kanichos <at> yandex.ru>
To: guix-patches <guix-patches <at> gnu.org>
Subject: [PATCH] profiles: Make linux-module-database skip inappropriate inputs
Date: Wed, 17 Jun 2020 21:44:35 +0300
This allows a Linux package with CONFIG_MODULES=n, that doesn’t contain the ‘lib/modules’ directory, to be used.

* guix/profiles.scm (linux-module-database): Add if clause to ignore unrelated inputs. Allow empty result.
---
 guix/profiles.scm | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 25ff146bdf..a3868e8343 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -1220,9 +1220,11 @@ This is meant to be used as a profile hook."
                        inputs))
                  (directory-entries
                   (lambda (directory)
-                    (scandir directory (lambda (basename)
-                                         (not
-                                           (string-prefix? "." basename))))))
+                    (if (file-exists? directory)
+                        (scandir directory (lambda (basename)
+                                             (not
+                                              (string-prefix? "." basename))))
+                        '())))
                  ;; Note: Should usually result in one entry.
                  (versions (delete-duplicates
                             (append-map directory-entries
@@ -1233,6 +1235,8 @@ This is meant to be used as a profile hook."
                   (setenv "PATH" #+(file-append kmod "/bin"))
                   (make-linux-module-directory inputs version #$output)
                   (setenv "PATH" old-path)))
+               ;; Do nothing when there is nothing to do
+               (() (mkdir #$output))
                (_ (error "Specified Linux kernel and Linux kernel modules
 are not all of the same version")))))))
   (gexp->derivation "linux-module-database" build
-- 
2.26.2






Information forwarded to guix-patches <at> gnu.org:
bug#41924; Package guix-patches. (Wed, 17 Jun 2020 21:20:02 GMT) Full text and rfc822 format available.

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

From: Ivan Kozlov <kanichos <at> yandex.ru>
To: "41924 <at> debbugs.gnu.org" <41924 <at> debbugs.gnu.org>
Subject: Re: [bug#41924] [PATCH] profiles: Make linux-module-database skip
 inappropriate inputs
Date: Thu, 18 Jun 2020 00:18:55 +0300
mkdir should of course be changed to mkdir-p.

17.06.2020, 21:45, "Ivan Kozlov" <kanichos <at> yandex.ru>:
> This allows a Linux package with CONFIG_MODULES=n, that doesn’t contain the ‘lib/modules’ directory, to be used.
>
> * guix/profiles.scm (linux-module-database): Add if clause to ignore unrelated inputs. Allow empty result.
> ---
>  guix/profiles.scm | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/guix/profiles.scm b/guix/profiles.scm
> index 25ff146bdf..a3868e8343 100644
> --- a/guix/profiles.scm
> +++ b/guix/profiles.scm
> @@ -1220,9 +1220,11 @@ This is meant to be used as a profile hook."
>                         inputs))
>                   (directory-entries
>                    (lambda (directory)
> - (scandir directory (lambda (basename)
> - (not
> - (string-prefix? "." basename))))))
> + (if (file-exists? directory)
> + (scandir directory (lambda (basename)
> + (not
> + (string-prefix? "." basename))))
> + '())))
>                   ;; Note: Should usually result in one entry.
>                   (versions (delete-duplicates
>                              (append-map directory-entries
> @@ -1233,6 +1235,8 @@ This is meant to be used as a profile hook."
>                    (setenv "PATH" #+(file-append kmod "/bin"))
>                    (make-linux-module-directory inputs version #$output)
>                    (setenv "PATH" old-path)))
> + ;; Do nothing when there is nothing to do
> + (() (mkdir #$output))
>                 (_ (error "Specified Linux kernel and Linux kernel modules
>  are not all of the same version")))))))
>    (gexp->derivation "linux-module-database" build
> --
> 2.26.2




Information forwarded to guix-patches <at> gnu.org:
bug#41924; Package guix-patches. (Thu, 18 Jun 2020 18:17:02 GMT) Full text and rfc822 format available.

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

From: Danny Milosavljevic <dannym <at> scratchpost.org>
To: Ivan Kozlov <kanichos <at> yandex.ru>
Cc: 41924 <at> debbugs.gnu.org
Subject: Re: [bug#41924] [PATCH] profiles: Make linux-module-database skip
 inappropriate inputs
Date: Thu, 18 Jun 2020 20:16:47 +0200
[Message part 1 (text/plain, inline)]
Hi Ivan,

thanks for the patch, and for persevering on this.

Could you also add a system test to gnu/tests/linux-modules.scm ?

That would also mean that you'd create a variant of linux-libre that has
CONFIG_MODULES=n.

>                   (directory-entries
>                    (lambda (directory)
> -                    (scandir directory (lambda (basename)
> -                                         (not
> -                                           (string-prefix? "." basename))))))
> +                    (if (file-exists? directory)
> +                        (scandir directory (lambda (basename)
> +                                             (not
> +                                              (string-prefix? "." basename))))
> +                        '())))

That would silently skip packages that don't contain kernel modules (but for
example supertux or something), too, right?
(so misconfigured guix wouldn't be detected)

Also, if you tried to use a kernel without module support and then added modules
(for the wrong kernel, one with CONFIG_MODULES=y) via extra guix packages,
this would erroneously succeed, right?
(I guess depmod would have something against it, though.  Still, I'd like a test
for that)

Usually I'd be fine with these unsupported constellations, but misconfigured
kernel vs module ABI makes me reconsider my life choices, so probably not here :->

What I would do is try to find a file that's only in the kernel when there's
module support (/lib/modules doesn't count since there could be a kernel with
module support but no modules compiled), and then try to ascertain whether
CONFIG_MODULES=y that way, so
(define config-modules? (any (append-map directory-entries with marker-file)))
or something.

Then succeed either with config-modules? and at least one file per entry, or
with (not config-modules?) and no files anywhere.

Alternatively, we could at least make it only possible for the FIRST entry
in the list of module packages (which is the Linux kernel) to have no
/lib/modules, and not care about any of the other entries in that way.

> +               ;; Do nothing when there is nothing to do
> +               (() (mkdir #$output))

Good idea.
[Message part 2 (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#41924; Package guix-patches. (Fri, 19 Jun 2020 07:48:01 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 41924 <at> debbugs.gnu.org, Ivan Kozlov <kanichos <at> yandex.ru>
Subject: Re: [bug#41924] [PATCH] profiles: Make linux-module-database skip
 inappropriate inputs
Date: Fri, 19 Jun 2020 09:47:15 +0200
Hi Ivan & Danny,

I forgot to email you back (sorry about that!): I pushed a variant of
this patch here:

  https://git.savannah.gnu.org/cgit/guix.git/commit/?id=05f79da93fb4fd5216feb41510bf0a41f8eedf5b

WDYT?

Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#41924; Package guix-patches. (Fri, 19 Jun 2020 08:21:01 GMT) Full text and rfc822 format available.

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

From: Ivan Kozlov <kanichos <at> yandex.ru>
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: "41924 <at> debbugs.gnu.org" <41924 <at> debbugs.gnu.org>
Subject: Re: [bug#41924] [PATCH] profiles: Make linux-module-database skip
 inappropriate inputs
Date: Fri, 19 Jun 2020 11:20:36 +0300
Hi,

> What I would do is try to find a file that's only in the kernel when there's
> module support (/lib/modules doesn't count since there could be a kernel with
> module support but no modules compiled), and then try to ascertain whether
> CONFIG_MODULES=y that way, so
> (define config-modules? (any (append-map directory-entries with marker-file)))
> or something.

The only way to do this sort of thing is to have the config. Otherwise the kernel will always sort of exist in a separate world. You can always compile it in a way that breaks the rest of the Guix system’s assumptions, e.g. by disabling the necessary system calls or cgroups controllers, and get no warnings/build failures. This applies to any extra kernel modules, too. I don’t think LKM support stands out. I actually don’t think it is a big problem that using an unofficial Linux package puts some burden on the maintainer, I merely want such setups to be possible.

The problem in question really applies to all cases when there aren’t any modules to deal with, as you correctly point out, not necessary having to do with CONFIG_MODULES; e.g. building a system with CONFIG_MODULES=y and no loadable modules, and no extra modules, is perfectly legitimate.

> That would silently skip packages that don't contain kernel modules (but for
> example supertux or something), too, right?
> (so misconfigured guix wouldn't be detected)

It is possible to rely on the way operating-system-directory-base-entries builds the manifest to avoid this specific issue: (car #$(manifest-inputs manifest)) should be the kernel.

I haven’t really looked into the test suite, here is my guess:

diff --git a/gnu/tests/linux-modules.scm b/gnu/tests/linux-modules.scm
index 953b132ef7..2fc8f52b90 100644
--- a/gnu/tests/linux-modules.scm
+++ b/gnu/tests/linux-modules.scm
@@ -25,6 +25,7 @@
   #:use-module (gnu system)
   #:use-module (gnu system vm)
   #:use-module (gnu tests)
+  #:use-module (gnu tests base)
   #:use-module (guix derivations)
   #:use-module (guix gexp)
   #:use-module (guix modules)
@@ -129,3 +130,37 @@ with two extra modules.")
                                                  (package-arguments
                                                   ddcci-driver-linux))))))
            '("acpi_call" "ddcci")))))
+
+(define %test-no-loadable-kernel-modules
+  (let* ((kernel (package
+                   (inherit linux-libre)
+                   (arguments
+                    (substitute-keyword-arguments (package-arguments linux-libre)
+                      ((#:phases phases)
+                       `(modify-phases ,phases
+                          (add-before 'build 'no-lkm-config
+                            (lambda _
+                              (substitute* ".config"
+                                (("=m") "=y"))
+                              (let ((port (open-file ".config" "a")))
+                                (display "CONFIG_MODULES=n" port)
+                                (close-port port))))
+                          (replace 'install
+                            (lambda* (#:key outputs #:allow-other-keys)
+                              (for-each
+                               (let ((out (assoc-ref outputs "out")))
+                                 (lambda (file) (install-file file out)))
+                               (find-files "." "^(\\.config|bzImage|zImage|Image|vmlinuz|System\\.map|Module\\.symvers)$"))))))))))
+         (os (marionette-operating-system
+              (operating-system
+                (inherit (simple-operating-system))
+                (kernel kernel)
+                (initrd-modules '()))
+              #:imported-modules '((gnu services herd)
+                                   (guix combinators))))
+         (vm (virtual-machine os)))
+    (system-test
+     (name "no-loadable-kernel-modules")
+     (description "Build and run a basic system without loadable kernel module support")
+     (value (run-basic-test (virtualized-operating-system os '())
+                            #~(list #$vm))))))
diff --git a/guix/profiles.scm b/guix/profiles.scm
index 25ff146bdf..abf0cf7f27 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -1201,6 +1201,8 @@ for both major versions of GTK+."
   "Return a derivation that unites all the kernel modules of the manifest
 and creates the dependency graph of all these kernel modules.

+The first entry in the manifest must be a Linux kernel package.
+
 This is meant to be used as a profile hook."
   (define kmod  ; lazy reference
     (module-ref (resolve-interface '(gnu packages linux)) 'kmod))
@@ -1226,14 +1228,22 @@ This is meant to be used as a profile hook."
                  ;; Note: Should usually result in one entry.
                  (versions (delete-duplicates
                             (append-map directory-entries
-                                        module-directories))))
-              (match versions
-               ((version)
-                (let ((old-path (getenv "PATH")))
-                  (setenv "PATH" #+(file-append kmod "/bin"))
-                  (make-linux-module-directory inputs version #$output)
-                  (setenv "PATH" old-path)))
-               (_ (error "Specified Linux kernel and Linux kernel modules
+                                        (if (file-exists? (car module-directories))
+                                            module-directories
+                                            (cdr module-directories))))))
+            (match versions
+              ((version)
+               (let ((old-path (getenv "PATH")))
+                 (setenv "PATH" #+(file-append kmod "/bin"))
+                 (make-linux-module-directory inputs version #$output)
+                 (setenv "PATH" old-path)))
+              ;; Do nothing when there is nothing to do
+              (() (mkdir #$output))
+              ;; This might not catch a version incompatibility
+              ;; if all kernel modules reside in extra packages
+              ;; Would checking the kernel package version have
+              ;; undesirable effects?
+              (_ (error "Specified Linux kernel and Linux kernel modules
 are not all of the same version")))))))
   (gexp->derivation "linux-module-database" build
                     #:local-build? #t





Information forwarded to guix-patches <at> gnu.org:
bug#41924; Package guix-patches. (Fri, 19 Jun 2020 14:24:01 GMT) Full text and rfc822 format available.

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

From: Ivan Kozlov <kanichos <at> yandex.ru>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Danny Milosavljevic <dannym <at> scratchpost.org>,
 "41924 <at> debbugs.gnu.org" <41924 <at> debbugs.gnu.org>
Subject: Re: [bug#41924] [PATCH] profiles: Make linux-module-database skip
 inappropriate inputs
Date: Fri, 19 Jun 2020 17:23:49 +0300
Hi,

Thanks, I’m happy enough with this.

19.06.2020, 10:47, "Ludovic Courtès" <ludo <at> gnu.org>:
> Hi Ivan & Danny,
>
> I forgot to email you back (sorry about that!): I pushed a variant of
> this patch here:
>
>   https://git.savannah.gnu.org/cgit/guix.git/commit/?id=05f79da93fb4fd5216feb41510bf0a41f8eedf5b
>
> WDYT?
>
> Ludo’.




Reply sent to Ludovic Courtès <ludo <at> gnu.org>:
You have taken responsibility. (Fri, 19 Jun 2020 20:28:01 GMT) Full text and rfc822 format available.

Notification sent to Ivan Kozlov <kanichos <at> yandex.ru>:
bug acknowledged by developer. (Fri, 19 Jun 2020 20:28:01 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Ivan Kozlov <kanichos <at> yandex.ru>
Cc: Danny Milosavljevic <dannym <at> scratchpost.org>,
 "41924 <at> debbugs.gnu.org" <41924-done <at> debbugs.gnu.org>
Subject: Re: [bug#41924] [PATCH] profiles: Make linux-module-database skip
 inappropriate inputs
Date: Fri, 19 Jun 2020 22:27:08 +0200
Hi,

Ivan Kozlov <kanichos <at> yandex.ru> skribis:

> Thanks, I’m happy enough with this.

Cool, closing.  Thank you.

Ludo’.




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

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

Previous Next


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