Subject: Re: [bug#40274] [PATCH v4] gnu: Add kernel-module-loader-service.

From: Mathieu Othacehe <[email-masked]>
Date: Wed, 01 Apr 2020 15:30:25 +0200

Hello Brice,

> +@deffn {Scheme Procedure} kernel-module-loader-service @var{modules}
> +Return a service that run @command{modprobe} with arguments
                           ^
                           runs

> +@var{modules} at boot.  For example loading the drivers provided by
        ^
        What's modules?

> +@deffn {Scheme Variable} kernel-module-loader-service-type
> +The service type for loading kernel modules at boot with
> +@command{modprobe}, for module that can't autoload.  Its value must be a
                                ^
                                modules

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

> +;;;
> +;;; Kernel module loader.
> +;;;

> +  (modprobe kernel-module-loader-configuration-modprobe ; path of the 'modprobe' command
> +            (default (file-append kmod "/bin/modprobe")))
> +  (modules kernel-module-loader-configuration-modules ; list of lists of strings
> +           (default '())))

Those lines are above the 78 columns limit.

> +
> +(define kernel-module-loader-shepherd-service
> +  (match-lambda
> +    (($ <kernel-module-loader-configuration> modprobe modules)
> +     (list
> +      (shepherd-service
> +       (documentation "Load kernel modules.")
> +       (provision '(kernel-module-loader))
> +       (respawn? #f)
> +       (one-shot? #t)
> +       (start
> +        #~(lambda _
> +            (and-map (lambda (module-with-parameters)
> +                   (zero? (apply system* #$modprobe "--"
> +                                 module-with-parameters)))
> +                 '#$modules))))))))

I wonder if it would make sense to rmmod the modules on service
stop? Probably not.

>     (value (run-loadable-kernel-modules-test
>             (list acpi-call-linux-module ddcci-driver-linux)
> -           '("acpi_call" "ddcci")))))
> +           '(("acpi_call")
> +             ("ddcci"))))))

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