[PATCH] gnu: Add kernel-module-loader-service.

new service and its configuration.

(kernel-module-loader-service-type, kernel-module-loader-configuration):

New types.

---

This patch is related to the following thread:

doc/guix.texi | 48 ++++++++++++++++++++++++++++++++++

gnu/services/linux.scm | 58 +++++++++++++++++++++++++++++++++++++++++-

2 files changed, 105 insertions(+), 1 deletion(-)

Hello Brice,

Thanks for this patch! I think you could add this service to (gnu tests

linux-modules). This way we would have a full-coverage of the mechanism.

Mathieu

new service and its configuration.

(kernel-module-loader-service-type, kernel-module-loader-configuration):

New types.

removed.

(modules-loaded?-program): New procedure.

(run-loadable-kernel-modules-test): 'module-loader-program' procedure

replaced by the new one. New format for last arguments

[os]: Use 'kernel-module-loader-service'.

(%test-loadable-kernel-modules-0, %test-loadable-kernel-modules-1,

%test-loadable-kernel-modules-2): Use the new argument format.

---

doc/guix.texi | 52 +++++++++++++++++++++++++++++++

gnu/services/linux.scm | 61 ++++++++++++++++++++++++++++++++++++-

gnu/tests/linux-modules.scm | 45 ++++++++++++++++-----------

3 files changed, 140 insertions(+), 18 deletions(-)

Hello Mathieu,

On 2020-03-28 20:51, Mathieu Othacehe wrote:

I have done what you suggested in v2. It also fix some issues and add

the

ability to optionally specify parameters for each module.

- Brice

new service and its configuration.

(kernel-module-loader-service-type, kernel-module-loader-configuration):

New types.

removed.

(modules-loaded?-program): New procedure.

(run-loadable-kernel-modules-test): 'module-loader-program' procedure

replaced by the new one. New format for last arguments

[os]: Use 'kernel-module-loader-service'.

(%test-loadable-kernel-modules-0, %test-loadable-kernel-modules-1,

%test-loadable-kernel-modules-2): Use the new argument format.

---

This version fix some markup issue in doc/guix.texi.

doc/guix.texi | 54 ++++++++++++++++++++++++++++++++

gnu/services/linux.scm | 61 ++++++++++++++++++++++++++++++++++++-

gnu/tests/linux-modules.scm | 45 ++++++++++++++++-----------

3 files changed, 142 insertions(+), 18 deletions(-)

Hi, I found a typo

/usefull/useful/

Nicolò

Brice Waegeneire <brice@waegenei.re> writes:

new service and its configuration.

(kernel-module-loader-service-type, kernel-module-loader-configuration):

New types.

removed.

(modules-loaded?-program): New procedure.

(run-loadable-kernel-modules-test): 'module-loader-program' procedure

replaced by the new one. New format for last arguments

[os]: Use 'kernel-module-loader-service'.

(%test-loadable-kernel-modules-0, %test-loadable-kernel-modules-1,

%test-loadable-kernel-modules-2): Use the new argument format.

---

Fix typo in doc/guix.texi.

doc/guix.texi | 54 ++++++++++++++++++++++++++++++++

gnu/services/linux.scm | 61 ++++++++++++++++++++++++++++++++++++-

gnu/tests/linux-modules.scm | 45 ++++++++++++++++-----------

3 files changed, 142 insertions(+), 18 deletions(-)

Hello,

On 2020-03-31 20:10, Nicolò Balzarotti wrote:

Thank you Nicolò I was missing flyspell in texinfo-mode, it's fixed in

v4.

- Brice

Hello Brice,

^

runs

^

What's modules?

^

modules

You can add a separator here, C-q C-l with Emacs :)

Those lines are above the 78 columns limit.

I wonder if it would make sense to rmmod the modules on service

stop? Probably not.

When using multiple modules without any configuration, I find the

notation '(("module-a") ("module-b")) a bit unclear. Maybe we could add

a special case handling '("module-a" "module-b") as two kernel modules

taking no arguments.

Or we could define a record with two fields: module-name and arguments.

Not sure what's better.

Anyway, this looks good to me :). But let's wait for Danny opinion

maybe.

Thanks,

Mathieu

Hello Mathieu,

On 2020-04-01 13:30, Mathieu Othacehe wrote:

I never used pagination in Emacs, maybe I should start using it.

This is a one-shot service so no.

I asked about it on IRC before starting v2 but nobody responded so I

went on

with this “list of lists of string” implementation. I'll be happy to use

a

better format, the special case look nice but maybe confusing for the

user.

I have fixed the typo you reported but I'll wait for further suggestion

about

the format before sending the new version.

- Brice

Thank you for your patch.

I tried with your patch to make the Guix install image load the

uvesafb kernel module, but it fails and I get no log. Is it possible

to make the service log failures?

I just get:

failed to load service: kernel-module-loader

printed on the console (but not to /var/log/messages).

I believe I pass parameters wrongly. I attach how I try to use your

kernel-module-loader.

Regards,

Florian

On Wed, Apr 01, 2020 at 09:34:58PM +0200, pelzflorian (Florian Pelz) wrote:

Of course my quoting was wrong.

Nonetheless could you add logging/errors if possible?

On Wed, Apr 01, 2020 at 09:34:58PM +0200, pelzflorian (Florian Pelz) wrote:

Also I cannot use gexps in module parameters.

Regards,

Florian

new service and its configuration.

(kernel-module-loader-service-type, kernel-module-loader-configuration):

New types.

removed.

(modules-loaded?-program): New procedure.

(run-loadable-kernel-modules-test): 'module-loader-program' procedure

replaced by the new one. New format for last arguments

[os]: Use 'kernel-module-loader-service'.

(%test-loadable-kernel-modules-0, %test-loadable-kernel-modules-1,

%test-loadable-kernel-modules-2): Use the new argument format.

---

In this version the typos reported by Mathieu have been fixed and if modprobe

fail the error is logged by shepherd.

doc/guix.texi | 53 +++++++++++++++++++++++++++

gnu/services/linux.scm | 72 ++++++++++++++++++++++++++++++++++++-

gnu/tests/linux-modules.scm | 45 ++++++++++++++---------

3 files changed, 152 insertions(+), 18 deletions(-)

Hello Florian,

Thank you for trying out this new service.

On 2020-04-01 19:34, pelzflorian (Florian Pelz) wrote:

This a message saying it can't load that service, in this case because

it's parameters weren't formatted correctly, this is not specific to

this service that's how shepherd handle failing to load a service.

Please open a new issue about it if you think (as I do) that shepherd

should be more explicit when it fails to load a service.

Regarding this service, logging has been implemented in v5; the output

of modprobe will be logged if it fails, for example when trying to load

an invalid kernel module like “foo”.

- Brice

Hi Brice,

I wonder how common it is to pass arguments to the modules explicitly in normal

operation.

I haven't done it often even in other distributions--and I'm a kernel hacker.

See also https://linux.die.net/man/5/modprobe.dfor an alternative.

I'm not necessarily against doing it like you do it, but just want to bring up

the possibility of just omitting the functionality and let it be

someone-else's-problem, possibly another guix service that prepares

/etc/modprobe.d with module options and other things (aliases, installation and

removal invocations).

That's also important because Linux tries to (lazy-)autoload modules whenever

possible (via invoking modprobe). In that case, the argument handling would

be inconsistent between if it was lazy-autoloaded compared to if it was loaded

by your loader.

(I even wonder if it were better for kernel-module-loader-service to read the

modprobe to use from /proc/sys/kernel/modprobe in order to make the situations

a little more consistent)

For example let's say the following happened:

(1) Linux boots up.

(2) Someone accesses some device so "modprobe foo" is invoked by Linux.

(3) foo is loaded, gets options from /etc/modprobe.d (usually none).

[Time passes, other stuff happens]

(4) Your kernel-module-loader-service is started, invokes "modprobe foo x=2".

(5) x=2 is not passed to the Linux kernel module ever.

I'm just saying maybe not invite this kind of trouble in the first place.

I don't think it fits Guix's declarative configuration style to do that

either.

Also, when reconfiguring the Guix system, kernel-module-loader-service won't

unload the kernel modules and thus also wouldn't load it with new options.

Also, it could happen that two different guix services require the same module

but with different options. That's an insane problem to have and I wouldn't

try to support it.

(I've reviewed your patch, otherwise OK!)

Hello Brice,

Thanks for this new version.

Maybe `(format (current-error-port) "~a~%" (condition-message c))'

instead of display.

Thanks,

Mathieu

On 2020-04-02 13:56, Danny Milosavljevic wrote:

Hello Danny,

Thank for taking the time to review this patch. Since I'm definitely

*not*

a kernel hacker --just a casual user-- I wasn't aware of the uselessness

of

specifying the module arguments to modprobe in such service. I wrote

this

patch just to load this pesky non auto-loading `ddcci-backlight` module

and

I have no current use of specifying module arguments. I just thought it

*could* be useful, to some, to pass arguments to modprobe since it is

present in its API; but the edge-cases you brought up show that it

wasn't a

good idea after all.

Should I just go back to the first format, with just a list of module

names, and we merge this patch? Or would it be better, regarding the

user

interface, to start this patch anew by using `modprobe.d` API as a base.

By that I mean defining a `modprobe-service-type` which populates

`/etc/modprobe.d/` and can manually load a module at boot if needed

(like

kernel-module-loader does)? Would it be overkill? Following is an

example of

what such service could look like:

#+begin_src scheme

(service modprobe-service-type

(list (modprobe-entry

(module "ddcci")

(load? #t)

(options '("dyndbg" "delay=120"))

(alises '("ddc/ci"))

(install "") ; default

(remove "")) ; default

(modprobe-entry

(module "acpi-call")

(blacklist? #t))))

#+end_src

- Brice

Hi Brice,

On Thu, 02 Apr 2020 17:13:05 +0000

Brice Waegeneire <brice@waegenei.re> wrote:

Definitely could be useful. The question is whether the complexity justifies it

and whether it can be reliable.

I would like that more since it's very improbable to ever need changing.

I think the two things (loading a module and configuring a module) can be

seperate, so I wouldn't mix it.

For example why would you load a module before the service that needs

it actually starts up (if ever)? That just increases the attack surface

for no reason. On the other hand, module configuration will probably be

done as part of some guix service presence--used or not.

That said, maybe there are upsides to--among other things easier-understandable

configuration if it's central.

A service to do that would still be nice. But maybe

"kernel-module-configuration-service" ?

(Please don't think I have all the answers, let's see which way is better)

On Fri, Apr 03, 2020 at 12:20:44PM +0200, Danny Milosavljevic wrote:

I need modprobe arguments in some way via some service to pass to the

uvesafb module to make graphics and kmscon work on machines without

Kernel Mode Setting (see attachment), but the (string-append …) is not

executed as a procedure, it seems.

Apr 3 14:54:45 localhost vmunix: [ 9.860831] shepherd[1]: Service user-homes has been started.

Apr 3 14:54:45 localhost vmunix: [ 9.880505] shepherd[1]: '/gnu/store/d064bv2b1hrb07j2zj78i608db7qldx9-kmod-26/bin/modprobe -- uvesafb (string-append v86d= /gnu/store/0m780n5q4cr7rqmjn5zzd6adr3042f97-v86d-0.1.10 /sbin/v86d) mode_option=1024x768' exited with status 127; output follows:

Apr 3 14:54:45 localhost vmunix: [ 9.881867] shepherd[1]:

Apr 3 14:54:45 localhost vmunix: [ 9.883595] shepherd[1]: Service kernel-module-loader could not be started.

Regards,

Florian

On Thu, Apr 02, 2020 at 12:40:00PM +0000, Brice Waegeneire wrote:

I meant compilation could fail if the parameters are incorrect. I do

not know how else Shepherd should fail. But this is not very

important and applies to many other services as well.

Thank you for adding logging. All this works well (in testing, I have

not read the code).

Regards,

Florian

Furthermore, many modules could be built-into the kernel (instead of loadable), too. Then the "module" options somehow have to make it into the kernel command line as "module_name.parameter_name=parameter_value".

Hello,

The following modification have been made since the previous patch version:

- The format to specify kernel modules has been reverted to a simple list of

strings.

- The `modprobe` command used is retrieved from "/proc/sys/kernel/modprobe".

- Thanks to the previously listed modifications,

`kernel-module-loader-service-type` has been removed together with the

helper function `kernel-module-loader-service`. No need to bother with a

record now that we only need a list of strings.

- A temporary crutch to force `modprobe` from loading "/etc/modprobe.d" has

been added. The permanent fix has been submitted as #40422[0] but it needs

to go trough the core-updates branch.

- The documentation has been updated accordingly.

- An example showing how to specify module arguments using "/etc/modprobe.d/"

has been added to the documentation.

[0]: https://issues.guix.info/issue/40422

- Brice

Brice Waegeneire (2):

services: Allow modprobe to use "/etc/modprobe.d".

gnu: Add kernel-module-loader-service.

doc/guix.texi | 38 +++++++++++++++++++++++++++

gnu/services.scm | 4 +++

gnu/services/linux.scm | 52 ++++++++++++++++++++++++++++++++++++-

gnu/tests/linux-modules.scm | 33 ++++++++++++++---------

4 files changed, 114 insertions(+), 13 deletions(-)

--

2.26.0

This is a temporary crutch allowing 'modprobe' to load it's

configuration from "/etc/modprobe.d". The definitive fix is contained in

https://issues.guix.info/issue/40422 which need to go trough the

core-upates branch as it rebuild over 2200 packages.

environment variable.

---

gnu/services.scm | 4 ++++

1 file changed, 4 insertions(+)

new service and its configuration.

(kernel-module-loader-shepherd-service): New procedure.

removed.

(modules-loaded?-program): New procedure.

(run-loadable-kernel-modules-test): 'module-loader-program' procedure

replaced by the new one.

[os]: Use 'kernel-module-loader-service'.

---

doc/guix.texi | 38 +++++++++++++++++++++++++++

gnu/services/linux.scm | 52 ++++++++++++++++++++++++++++++++++++-

gnu/tests/linux-modules.scm | 33 ++++++++++++++---------

3 files changed, 110 insertions(+), 13 deletions(-)

Hello Brice,

I think what you discussed with Danny, separating module loading from

configuration is very wise.

Using kernel-arguments you can configure built-in modules but you cannot

force them to show-up, at least for most device drivers. For those

device drivers, loading of built-in modules is based on hardware probing

at boot-time.

So I would just stick with:

"The kernel module loader service allows one to load, at boot time,

specific kernel modules."

Having the modprobe-service-type you proposed earlier, would be super

nice to do that :)

Great!

This seems really nice to me.

Mathieu

On Sat, Apr 04, 2020 at 05:17:30PM +0200, Brice Waegeneire wrote:

You mean you removed kernel-module-loader-configuration.

Regarding patch 2/2:

On Sat, Apr 04, 2020 at 05:17:32PM +0200, Brice Waegeneire wrote:

This is a typo. I can only find ddcci-driver-linux.

Thank you for this patch. I do not know enough to judge the code. I

will try again if it helps me load uvesafb.

Regards,

Florian

Hello Florian,

Thank you for the interest you have in this patch series.

On 2020-04-04 16:09, pelzflorian (Florian Pelz) wrote:

Yes.

Good catch! DDC/CI what a silly name...

Please do send feedback on using it, especially since you are the only

one

in need of specifying module parameters ATM. I kept your usage in mind

when

writing this patch version.

- Brice

On Sat, Apr 04, 2020 at 04:49:16PM +0000, Brice Waegeneire wrote:

It does not seem to work. There is nothing in /var/log/messages but

when I do

root@gnu.org # herd start kernel-module-loader

herd: exception caught while executing 'start' on service 'kernel-module-loader':

In procedure module-lookup: Unbound variable: get-line

I attach my changes.

Regards,

Florian

Hello,

On 2020-04-03 10:20, Danny Milosavljevic wrote:

A lengthy one but a better one for sure.

I don't but you seems to have more answers than I have on this specific

subject.

We are on the same page here, the best interface should be the one that

get

merged!

As you suggested, I choose to go back to the list of string format. It

simplified

the boilerplate a lot: no record and no helper function. Yet it still

allows passing

arguments to modules by extending `etc-service-type` to create

"/etc/modprobe.d/*.conf" files, and avoids all the pitfalls of the

previous method

you made me aware off.

As `kernel-module-loader-service-type` is extensible it would be

possible, in an

other patch series, if the need arises, to create a

`kernel-module-configuration-service`

centralizing loadable modules configuration. But like you noted it would

be specific

to loadable modules and not built-in ones since `kernel-arguments` is

just an OS

field, which AFAIK can't be influenced by a service.

Thank you for your help Danny.

- Brice

Hi Brice,

There's no one stopping us from extending operating-system-kernel-arguments,

which already exists as a helper procedure, to also call

operating-system-services and check those for the command line arguments

to add (see gnu/system.scm).

On Sat, 4 Apr 2020 17:17:32 +0200

Brice Waegeneire <brice@waegenei.re> wrote:

Missing (ice-9 rdelim) ?

On Sat, 4 Apr 2020 23:10:04 +0200

Danny Milosavljevic <dannym@scratchpost.org> wrote:

I meant (ice-9 textual-ports)

What happens in the case that kernel-module-loader-service is used but no

modules to load are specified? IMO that should be succeeding even in the

presence of a Linux kernel with no loadable module support.

On Sat, Apr 04, 2020 at 11:11:44PM +0200, Danny Milosavljevic wrote:

Yes. I change Brice’s kernel-module-loader-shepherd-service (see

attachment) and everything works perfectly.

Thank you!

Regards,

Florian

This new revision:

- fix a typo in the documentation

- remove the part about built-in modules in the documentation

- add a missing Guile module to `kernel-module-loader-shepherd-service`

- `kernel-module-loader-shepherd-service` doesn't fail when the kernel don't

support loadable module and there if no module are to be loaded

Brice Waegeneire (2):

services: Allow modprobe to use "/etc/modprobe.d".

gnu: Add kernel-module-loader-service.

doc/guix.texi | 37 +++++++++++++++++++++++++

gnu/services.scm | 4 +++

gnu/services/linux.scm | 54 ++++++++++++++++++++++++++++++++++++-

gnu/tests/linux-modules.scm | 33 ++++++++++++++---------

4 files changed, 115 insertions(+), 13 deletions(-)

--

2.26.0

This is a temporary crutch allowing 'modprobe' to load it's

configuration from "/etc/modprobe.d". The definitive fix is contained in

https://issues.guix.info/issue/40422 which need to go trough the

core-upates branch as it rebuild over 2200 packages.

environment variable.

---

gnu/services.scm | 4 ++++

1 file changed, 4 insertions(+)

new service and its configuration.

(kernel-module-loader-shepherd-service): New procedure.

removed.

(modules-loaded?-program): New procedure.

(run-loadable-kernel-modules-test): 'module-loader-program' procedure

replaced by the new one.

[os]: Use 'kernel-module-loader-service'.

---

doc/guix.texi | 37 +++++++++++++++++++++++++

gnu/services/linux.scm | 54 ++++++++++++++++++++++++++++++++++++-

gnu/tests/linux-modules.scm | 33 ++++++++++++++---------

3 files changed, 111 insertions(+), 13 deletions(-)

On 2020-04-04 21:11, Danny Milosavljevic wrote:

The tests reported a warning when (ice-9 textual-ports) was imported,

using (rnrs io ports) instead didn't.

On 2020-04-04 18:31, Danny Milosavljevic wrote:

Thank you for pointing this to me, I didn't thought it was working this

way.

So writing such a service is totally doable, interesting.

On Sun, Apr 05, 2020 at 07:28:03AM +0200, Brice Waegeneire wrote:

I can use kernel-module-loader v6 without modifications just fine for

uvesafb. Thank you!

Regards,

Florian

Hi Brice,

I've pushed a variant of this to guix master as

commit 044d1478c9a63a64547c9cc320008f8d8fbf6791 with the following changes:

Thanks!

Pushed to guix master as commit 8c88e242292db9b35b4ae6ad788a0f8f3c94bb53.

I think core-updates will be merged to master pretty soon anyway, but the

question is whether we want to update it in core-updates before that, given

how many dependencies that has (WTF! What's up with that?).

On 2020-04-05 11:11, Danny Milosavljevic wrote:

Thank you for helping with that!

I thought that core-updates was frozen which would delay #40422 for some

time.

I wrote the crutch patch to avoid having to wait before supporting

module

arguments, especially since Florian already had a need for it.