[PATCH 0/5] Add tlp service.

[PATCH 0/5] Add tlp service.

[Mathieu Othacehe]

Hi Guix,

Here's a patch serie to add tlp service and fix problems
encountered meanwhile.

While generating tlp documentation, I noticed that parts of the
generated documentation had this format :

@deftypevr @code{tlp-configuration} parameter boolean tlp-enable?

instead of this expected format :

@deftypevr {@code{tlp-configuration} parameter} boolean tlp-enable?

I found that since guile commit 06e4091c9, the expected output is produced.
So I used guile 2.1.7 to generate tlp documentation.

I also re-generated openvpn documentation which was generated with
a guile version producing the wrong output.

Thanks,

Mathieu

Mathieu Othacehe (5):
  gnu: tlp: Read configuration from /etc/tlp.
  services: Factorize define-maybe macro.
  gnu: Add tlp service.
  doc: Re-generate openvpn service documentation.
  services: Fix a typo which was corrected in generated doc.

 doc/guix.texi                  | 587 ++++++++++++++++++++++++++++++++++++++---
 gnu/local.mk                   |   2 +
 gnu/packages/linux.scm         |   2 +-
 gnu/services/configuration.scm |  28 +-
 gnu/services/messaging.scm     |  22 +-
 gnu/services/pm.scm            | 406 ++++++++++++++++++++++++++++
 gnu/services/vpn.scm           |   3 +-
 7 files changed, 977 insertions(+), 73 deletions(-)
 create mode 100644 gnu/services/pm.scm

-- 
2.12.0

[PATCH 1/5] gnu: tlp: Read configuration from /etc/tlp.

[Mathieu Othacehe]

* gnu/packages/linux.scm (tlp): Set TLP_CONF to "/etc/tlp".
---
 gnu/packages/linux.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 28440297e..bb9dbcb3d 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -3494,7 +3494,7 @@ Light is the successor of lightscript.")
                (setenv "TLP_BIN" (string-append out "/bin"))
                (setenv "TLP_TLIB" (string-append out "/share/tlp-pm"))
                (setenv "TLP_ULIB" (string-append out "/lib/udev"))
-               (setenv "TLP_CONF" (string-append out "/etc/tlp"))
+               (setenv "TLP_CONF" "/etc/tlp")
                (setenv "TLP_SHCPL"
                        (string-append out "/share/bash-completion/completions"))
                (setenv "TLP_MAN" (string-append out "/share/man")))))
-- 
2.12.0

Re: [PATCH 1/5] gnu: tlp: Read configuration from /etc/tlp.

[Clément Lassieur]

> * gnu/packages/linux.scm (tlp): Set TLP_CONF to "/etc/tlp".
> ---
>  gnu/packages/linux.scm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
> index 28440297e..bb9dbcb3d 100644
> --- a/gnu/packages/linux.scm
> +++ b/gnu/packages/linux.scm
> @@ -3494,7 +3494,7 @@ Light is the successor of lightscript.")
>                 (setenv "TLP_BIN" (string-append out "/bin"))
>                 (setenv "TLP_TLIB" (string-append out "/share/tlp-pm"))
>                 (setenv "TLP_ULIB" (string-append out "/lib/udev"))
> -               (setenv "TLP_CONF" (string-append out "/etc/tlp"))
> +               (setenv "TLP_CONF" "/etc/tlp")
>                 (setenv "TLP_SHCPL"
>                         (string-append out "/share/bash-completion/completions"))
>                 (setenv "TLP_MAN" (string-append out "/share/man")))))

LGTM!

[PATCH 2/5] services: Factorize define-maybe macro.

[Mathieu Othacehe]

* gnu/services/configuration.scm (define-maybe): New exported
  procedure.
  (id): New procedure extracted from define-configuration.
* gnu/services/messaging.scm (define-all-configurations): Define id
  inside procedure as define-all-configurations is now the only user.
---
 gnu/services/configuration.scm | 28 ++++++++++++++++++++--------
 gnu/services/messaging.scm     | 22 +++-------------------
 2 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index 2ad3a637a..3fdaf705a 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2015 Andy Wingo <wingo@igalia.com>
+;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -36,6 +37,7 @@
             configuration-field-default-value-thunk
             configuration-field-documentation
             serialize-configuration
+            define-maybe
             define-configuration
             validate-configuration
             generate-documentation
@@ -85,16 +87,26 @@
                    (configuration-field-name field) val))))
             fields))
 
+(define-syntax-rule (id ctx parts ...)
+      (datum->syntax ctx (symbol-append (syntax->datum parts) ...)))
+
+(define-syntax define-maybe
+  (lambda (x)
+    (syntax-case x ()
+      ((_ stem)
+       (with-syntax
+           ((stem?                (id #'stem #'stem #'?))
+            (maybe-stem?          (id #'stem #'maybe- #'stem #'?))
+            (serialize-stem       (id #'stem #'serialize- #'stem))
+            (serialize-maybe-stem (id #'stem #'serialize-maybe- #'stem)))
+         #'(begin
+             (define (maybe-stem? val)
+               (or (eq? val 'disabled) (stem? val)))
+             (define (serialize-maybe-stem field-name val)
+               (when (stem? val) (serialize-stem field-name val)))))))))
+
 (define-syntax define-configuration
   (lambda (stx)
-    (define (id ctx part . parts)
-      (let ((part (syntax->datum part)))
-        (datum->syntax
-         ctx
-         (match parts
-           (() part)
-           (parts (symbol-append part
-                                 (syntax->datum (apply id ctx parts))))))))
     (syntax-case stx ()
       ((_ stem (field (field-type def) doc) ...)
        (with-syntax (((field-getter ...)
diff --git a/gnu/services/messaging.scm b/gnu/services/messaging.scm
index 34723dc11..e50eeba8c 100644
--- a/gnu/services/messaging.scm
+++ b/gnu/services/messaging.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2017 Clément Lassieur <clement@lassieur.org>
+;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -49,27 +50,10 @@
 ;;;
 ;;; Code:
 
-(define-syntax-rule (id ctx parts ...)
-  "Assemble PARTS into a raw (unhygienic) identifier."
-  (datum->syntax ctx (symbol-append (syntax->datum parts) ...)))
-
-(define-syntax define-maybe
-  (lambda (x)
-    (syntax-case x ()
-      ((_ stem)
-       (with-syntax
-           ((stem?                (id #'stem #'stem #'?))
-            (maybe-stem?          (id #'stem #'maybe- #'stem #'?))
-            (serialize-stem       (id #'stem #'serialize- #'stem))
-            (serialize-maybe-stem (id #'stem #'serialize-maybe- #'stem)))
-         #'(begin
-             (define (maybe-stem? val)
-               (or (eq? val 'disabled) (stem? val)))
-             (define (serialize-maybe-stem field-name val)
-               (when (stem? val) (serialize-stem field-name val)))))))))
-
 (define-syntax define-all-configurations
   (lambda (stx)
+    (define-syntax-rule (id ctx parts ...)
+      (datum->syntax ctx (symbol-append (syntax->datum parts) ...)))
     (define (make-pred arg)
       (lambda (field target)
         (and (memq (syntax->datum target) `(common ,arg)) field)))
-- 
2.12.0

Re: [PATCH 2/5] services: Factorize define-maybe macro.

[Clément Lassieur]

Mathieu Othacehe <m.othacehe@gmail.com> writes:

> * gnu/services/configuration.scm (define-maybe): New exported
>   procedure.
>   (id): New procedure extracted from define-configuration.
> * gnu/services/messaging.scm (define-all-configurations): Define id
>   inside procedure as define-all-configurations is now the only user.
> ---
>  gnu/services/configuration.scm | 28 ++++++++++++++++++++--------
>  gnu/services/messaging.scm     | 22 +++-------------------
>  2 files changed, 23 insertions(+), 27 deletions(-)
>
> diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
> index 2ad3a637a..3fdaf705a 100644
> --- a/gnu/services/configuration.scm
> +++ b/gnu/services/configuration.scm
> @@ -1,5 +1,6 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2015 Andy Wingo <wingo@igalia.com>
> +;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -36,6 +37,7 @@
>              configuration-field-default-value-thunk
>              configuration-field-documentation
>              serialize-configuration
> +            define-maybe
>              define-configuration
>              validate-configuration
>              generate-documentation
> @@ -85,16 +87,26 @@
>                     (configuration-field-name field) val))))
>              fields))
>  
> +(define-syntax-rule (id ctx parts ...)
> +      (datum->syntax ctx (symbol-append (syntax->datum parts) ...)))

The docstring was helpful!  Could you put it too?

> +(define-syntax define-maybe
> +  (lambda (x)
> +    (syntax-case x ()
> +      ((_ stem)
> +       (with-syntax
> +           ((stem?                (id #'stem #'stem #'?))
> +            (maybe-stem?          (id #'stem #'maybe- #'stem #'?))
> +            (serialize-stem       (id #'stem #'serialize- #'stem))
> +            (serialize-maybe-stem (id #'stem #'serialize-maybe- #'stem)))
> +         #'(begin
> +             (define (maybe-stem? val)
> +               (or (eq? val 'disabled) (stem? val)))
> +             (define (serialize-maybe-stem field-name val)
> +               (when (stem? val) (serialize-stem field-name val)))))))))
> +
>  (define-syntax define-configuration
>    (lambda (stx)
> -    (define (id ctx part . parts)
> -      (let ((part (syntax->datum part)))
> -        (datum->syntax
> -         ctx
> -         (match parts
> -           (() part)
> -           (parts (symbol-append part
> -                                 (syntax->datum (apply id ctx parts))))))))

I think this part, that replaces the old version of "id" with the new
one deserves a specific patch, because it is not directly related to
moving define-maybe, and because it does more that just "moving code".

>      (syntax-case stx ()
>        ((_ stem (field (field-type def) doc) ...)
>         (with-syntax (((field-getter ...)
> diff --git a/gnu/services/messaging.scm b/gnu/services/messaging.scm
> index 34723dc11..e50eeba8c 100644
> --- a/gnu/services/messaging.scm
> +++ b/gnu/services/messaging.scm
> @@ -1,5 +1,6 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2017 Clément Lassieur <clement@lassieur.org>
> +;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -49,27 +50,10 @@
>  ;;;
>  ;;; Code:
>  
> -(define-syntax-rule (id ctx parts ...)
> -  "Assemble PARTS into a raw (unhygienic) identifier."
> -  (datum->syntax ctx (symbol-append (syntax->datum parts) ...)))
> -
> -(define-syntax define-maybe
> -  (lambda (x)
> -    (syntax-case x ()
> -      ((_ stem)
> -       (with-syntax
> -           ((stem?                (id #'stem #'stem #'?))
> -            (maybe-stem?          (id #'stem #'maybe- #'stem #'?))
> -            (serialize-stem       (id #'stem #'serialize- #'stem))
> -            (serialize-maybe-stem (id #'stem #'serialize-maybe- #'stem)))
> -         #'(begin
> -             (define (maybe-stem? val)
> -               (or (eq? val 'disabled) (stem? val)))
> -             (define (serialize-maybe-stem field-name val)
> -               (when (stem? val) (serialize-stem field-name val)))))))))
> -

>  (define-syntax define-all-configurations
>    (lambda (stx)
> +    (define-syntax-rule (id ctx parts ...)
> +      (datum->syntax ctx (symbol-append (syntax->datum parts) ...)))

This could go in the second patch as well.

>      (define (make-pred arg)
>        (lambda (field target)
>          (and (memq (syntax->datum target) `(common ,arg)) field)))

[PATCH 3/5] gnu: Add tlp service.

[Mathieu Othacehe]

* gnu/services/pm.scm: New file.
* gnu/local.mk (GNU_SYSTEM_MODULES): Add gnu/services/tlp.scm.
* doc/guix.texi (Power management Services): New section.
---
 doc/guix.texi       | 499 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 gnu/local.mk        |   2 +
 gnu/services/pm.scm | 406 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 907 insertions(+)
 create mode 100644 gnu/services/pm.scm

diff --git a/doc/guix.texi b/doc/guix.texi
index f1d247644..d8d9d7a98 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -215,6 +215,7 @@ Services
 * VPN Services::                VPN daemons.
 * Network File System::         NFS related services.
 * Continuous Integration::      The Cuirass service.
+* Power management Services::   The TLP tool.
 * Miscellaneous Services::      Other services.
 
 Defining Services
@@ -8390,6 +8391,7 @@ declaration.
 * VPN Services::                VPN daemons.
 * Network File System::         NFS related services.
 * Continuous Integration::      The Cuirass service.
+* Power management Services::   The TLP tool.
 * Miscellaneous Services::      Other services.
 @end menu
 
@@ -13605,6 +13607,503 @@ The Cuirass package to use.
 @end table
 @end deftp
 
+@node Power management Services
+@subsubsection Power management Services
+
+@cindex power management with TLP
+The @code{(gnu services pm)} module provides a Guix service definition
+for the Linux power management tool TLP.
+
+@deffn {Scheme Variable} tlp-service-type
+The service type for the TLP tool.  Its value should be a valid
+TLP configuration (see below).  For example:
+@example
+(service tlp-service-type (tlp-configuration))
+@end example
+@end deffn
+
+By default TLP does not need much configuration but most TLP parameters
+can be tweaked using @code{tlp-configuration}.
+
+Each parameter definition is preceded by its type; for example,
+@samp{boolean foo} indicates that the @code{foo} parameter
+should be specified as a boolean.  Types starting with
+@code{maybe-} denote parameters that won't show up in TLP config file
+when their value is @code{'disabled}.
+
+@c The following documentation was initially generated by
+@c (generate-tlp-documentation) in (gnu services pm).  Manually maintained
+@c documentation is better, so we shouldn't hesitate to edit below as
+@c needed.  However if the change you want to make to this documentation
+@c can be done in an automated way, it's probably easier to change
+@c (generate-documentation) than to make it below and have to deal with
+@c the churn as TLP updates.
+
+Available @code{tlp-configuration} fields are:
+
+@deftypevr {@code{tlp-configuration} parameter} package tlp
+The TLP package.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} boolean tlp-enable?
+Set to true if you wish to enable TLP.
+
+Defaults to @samp{#t}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string tlp-default-mode
+Default mode when no power supply can be detected.  Alternatives are AC
+and BAT.
+
+Defaults to @samp{"AC"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} non-negative-integer disk-idle-secs-on-ac
+Number of seconds Linux kernel has to wait after the disk goes idle,
+before syncing on AC.
+
+Defaults to @samp{0}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} non-negative-integer disk-idle-secs-on-bat
+Same as @code{disk-idle-ac} but on BAT mode.
+
+Defaults to @samp{2}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} non-negative-integer max-lost-work-secs-on-ac
+Dirty pages flushing periodicity, expressed in seconds.
+
+Defaults to @samp{15}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} non-negative-integer max-lost-work-secs-on-bat
+Same as @code{max-lost-work-secs-on-ac} but on BAT mode.
+
+Defaults to @samp{60}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-space-separated-string-list cpu-scaling-governor-on-ac
+CPU frequency scaling governor on AC mode.  With intel_pstate driver,
+alternatives are powersave and performance.  With acpi-cpufreq driver,
+alternatives are ondemand, powersave, performance and conservative.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-space-separated-string-list cpu-scaling-governor-on-bat
+Same as @code{cpu-scaling-governor-on-ac} but on BAT mode.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-non-negative-integer cpu-scaling-min-freq-on-ac
+Set the min available frequency for the scaling governor on AC.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-non-negative-integer cpu-scaling-max-freq-on-ac
+Set the max available frequency for the scaling governor on AC.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-non-negative-integer cpu-scaling-min-freq-on-bat
+Set the min available frequency for the scaling governor on BAT.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-non-negative-integer cpu-scaling-max-freq-on-bat
+Set the max available frequency for the scaling governor on BAT.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-non-negative-integer cpu-min-perf-on-ac
+Limit the min P-state to control the power dissipation of the CPU, in AC
+mode.  Values are stated as a percentage of the available performance.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-non-negative-integer cpu-max-perf-on-ac
+Limit the max P-state to control the power dissipation of the CPU, in AC
+mode.  Values are stated as a percentage of the available performance.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-non-negative-integer cpu-min-perf-on-bat
+Same as @code{cpu-min-perf-on-ac} on BAT mode.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-non-negative-integer cpu-max-perf-on-bat
+Same as @code{cpu-max-perf-on-ac} on BAT mode.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-boolean cpu-boost-on-ac?
+Enable CPU turbo boost feature on AC mode.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-boolean cpu-boost-on-bat?
+Same as @code{cpu-boost-on-ac?} on BAT mode.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} boolean sched-powersave-on-ac?
+Allow Linux kernel to minimize the number of CPU cores/hyper-threads
+used under light load conditions.
+
+Defaults to @samp{#f}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} boolean sched-powersave-on-bat?
+Same as @code{sched-powersave-on-ac?} but on BAT mode.
+
+Defaults to @samp{#t}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} boolean nmi-watchdog?
+Enable Linux kernel NMI watchdog.
+
+Defaults to @samp{#f}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-string phc-controls
+For Linux kernels with PHC patch applied, change CPU voltages.  An
+example value would be @samp{"F:V F:V F:V F:V"}.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string energy-perf-policy-on-ac
+Set CPU performance versus energy saving policy on AC.  Alternatives are
+performance, normal, powersave.
+
+Defaults to @samp{"performance"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string energy-perf-policy-on-bat
+Same as @code{energy-perf-policy-ac} but on BAT mode.
+
+Defaults to @samp{"powersave"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} space-separated-string-list disks-devices
+Hard disk devices.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} space-separated-string-list disk-apm-level-on-ac
+Hard disk advanced power management level.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} space-separated-string-list disk-apm-level-on-bat
+Same as @code{disk-apm-bat} but on BAT mode.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-space-separated-string-list disk-spindown-timeout-on-ac
+Hard disk spin down timeout.  One value has to be specified for each
+declared hard disk.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-space-separated-string-list disk-spindown-timeout-on-bat
+Same as @code{disk-spindown-timeout-on-ac} but on BAT mode.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-space-separated-string-list disk-iosched
+Select IO scheduler for disk devices.  One value has to be specified for
+each declared hard disk.  Example alternatives are cfq, deadline and
+noop.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string sata-linkpwr-on-ac
+SATA aggressive link power management (ALPM) level.  Alternatives are
+min_power, medium_power, max_performance.
+
+Defaults to @samp{"max_performance"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string sata-linkpwr-on-bat
+Same as @code{sata-linkpwr-ac} but on BAT mode.
+
+Defaults to @samp{"min_power"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-string sata-linkpwr-blacklist
+Exclude specified SATA host devices for link power management.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-on-off-boolean ahci-runtime-pm-on-ac?
+Enable Runtime Power Management for AHCI controller and disks on AC
+mode.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-on-off-boolean ahci-runtime-pm-on-bat?
+Same as @code{ahci-runtime-pm-on-ac} on BAT mode.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} non-negative-integer ahci-runtime-pm-timeout
+Seconds of inactivity before disk is suspended.
+
+Defaults to @samp{15}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string pcie-aspm-on-ac
+PCI Express Active State Power Management level.  Alternatives are
+default, performance, powersave.
+
+Defaults to @samp{"performance"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string pcie-aspm-on-bat
+Same as @code{pcie-aspm-ac} but on BAT mode.
+
+Defaults to @samp{"powersave"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string radeon-power-profile-on-ac
+Radeon graphics clock speed level.  Alternatives are low, mid, high,
+auto, default.
+
+Defaults to @samp{"high"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string radeon-power-profile-on-bat
+Same as @code{radeon-power-ac} but on BAT mode.
+
+Defaults to @samp{"low"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string radeon-dpm-state-on-ac
+Radeon dynamic power management method (DPM).  Alternatives are battery,
+performance.
+
+Defaults to @samp{"performance"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string radeon-dpm-state-on-bat
+Same as @code{radeon-dpm-state-ac} but on BAT mode.
+
+Defaults to @samp{"battery"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string radeon-dpm-perf-level-on-ac
+Radeon DPM performance level.  Alternatives are auto, low, high.
+
+Defaults to @samp{"auto"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string radeon-dpm-perf-level-on-bat
+Same as @code{radeon-dpm-perf-ac} but on BAT mode.
+
+Defaults to @samp{"auto"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} on-off-boolean wifi-pwr-on-ac?
+Wifi power saving mode.
+
+Defaults to @samp{#f}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} on-off-boolean wifi-pwr-on-bat?
+Same as @code{wifi-power-ac?} but on BAT mode.
+
+Defaults to @samp{#t}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} y-n-boolean wol-disable?
+Disable wake on LAN.
+
+Defaults to @samp{#t}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} non-negative-integer sound-power-save-on-ac
+Timeout duration in seconds before activating audio power saving on
+Intel HDA and AC97 devices.  A value of 0 disables power saving.
+
+Defaults to @samp{0}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} non-negative-integer sound-power-save-on-bat
+Same as @code{sound-powersave-ac} but on BAT mode.
+
+Defaults to @samp{1}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} y-n-boolean sound-power-save-controller?
+Disable controller in powersaving mode on Intel HDA devices.
+
+Defaults to @samp{#t}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} boolean bay-poweroff-on-bat?
+Enable optical drive in UltraBay/MediaBay on BAT mode.  Drive can be
+powered on again by releasing (and reinserting) the eject lever or by
+pressing the disc eject button on newer models.
+
+Defaults to @samp{#f}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string bay-device
+Name of the optical drive device to power off.
+
+Defaults to @samp{"sr0"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string runtime-pm-on-ac
+Runtime Power Management for PCI(e) bus devices.  Alternatives are on
+and auto.
+
+Defaults to @samp{"on"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} string runtime-pm-on-bat
+Same as @code{runtime-pm-ac} but on BAT mode.
+
+Defaults to @samp{"auto"}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} boolean runtime-pm-all?
+Runtime Power Management for all PCI(e) bus devices, except blacklisted
+ones.
+
+Defaults to @samp{#t}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-space-separated-string-list runtime-pm-blacklist
+Exclude specified PCI(e) devices adresses from Runtime Power Management.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} space-separated-string-list runtime-pm-driver-blacklist
+Exclude PCI(e) devices assigned to the specified drivers from Runtime
+Power Management.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} boolean usb-autosuspend?
+Enable USB autosuspend feature.
+
+Defaults to @samp{#t}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-string usb-blacklist
+Exclude specified devices from USB autosuspend.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} boolean usb-blacklist-wwan?
+Exclude WWAN devices from USB autosuspend.
+
+Defaults to @samp{#t}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-string usb-whitelist
+Include specified devices into USB autosuspend, even if they are already
+excluded by the driver or via @code{usb-blacklist-wwan?}.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} maybe-boolean usb-autosuspend-disable-on-shutdown?
+Enable USB autosuspend before shutdown.
+
+Defaults to @samp{disabled}.
+
+@end deftypevr
+
+@deftypevr {@code{tlp-configuration} parameter} boolean restore-device-state-on-startup?
+Restore radio device state (bluetooth, wifi, wwan) from previous
+shutdown on system startup.
+
+Defaults to @samp{#f}.
+
+@end deftypevr
+
 @node Miscellaneous Services
 @subsubsection Miscellaneous Services
 
diff --git a/gnu/local.mk b/gnu/local.mk
index c1b076a5f..299ade67b 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -9,6 +9,7 @@
 # Copyright © 2016 Adonay "adfeno" Felipe Nogueira <https://libreplanet.org/wiki/User:Adfeno> <adfeno@openmailbox.org>
 # Copyright © 2016, 2017 Ricardo Wurmus <rekado@elephly.net>
 # Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com>
+# Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 #
 # This file is part of GNU Guix.
 #
@@ -421,6 +422,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/services/nfs.scm			\
   %D%/services/shepherd.scm			\
   %D%/services/herd.scm				\
+  %D%/services/pm.scm				\
   %D%/services/sddm.scm				\
   %D%/services/spice.scm				\
   %D%/services/ssh.scm				\
diff --git a/gnu/services/pm.scm b/gnu/services/pm.scm
new file mode 100644
index 000000000..e3b654de6
--- /dev/null
+++ b/gnu/services/pm.scm
@@ -0,0 +1,406 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software: you can redistribute it and/or modify
+;;; it under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation, either version 3 of the License, or
+;;; (at your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful,
+;;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu services pm)
+  #:use-module (guix gexp)
+  #:use-module (guix packages)
+  #:use-module (guix records)
+  #:use-module (gnu packages linux)
+  #:use-module (gnu services)
+  #:use-module (gnu services base)
+  #:use-module (gnu services configuration)
+  #:use-module (gnu services shepherd)
+  #:use-module (gnu system shadow)
+  #:export (tlp-service-type
+            tlp-configuration
+            generate-tlp-documentation))
+
+(define (uglify-field-name field-name)
+  (let ((str (symbol->string field-name)))
+    (string-join (string-split
+                  (string-upcase
+                   (if (string-suffix? "?" str)
+                       (substring str 0 (1- (string-length str)))
+                       str))
+                  #\-)
+                 "_")))
+
+(define (serialize-field field-name val)
+  (format #t "~a=~a\n" (uglify-field-name field-name) val))
+
+(define (serialize-boolean field-name val)
+  (serialize-field field-name (if val "1" "0")))
+(define-maybe boolean)
+
+(define (serialize-string field-name val)
+  (serialize-field field-name val))
+(define-maybe string)
+
+(define (space-separated-string-list? val)
+  (and (list? val)
+       (and-map (lambda (x)
+                  (and (string? x) (not (string-index x #\space))))
+                val)))
+(define (serialize-space-separated-string-list field-name val)
+  (serialize-field field-name
+                   (format #f "~s"
+                           (string-join val " "))))
+(define-maybe space-separated-string-list)
+
+(define (non-negative-integer? val)
+  (and (exact-integer? val) (not (negative? val))))
+(define (serialize-non-negative-integer field-name val)
+  (serialize-field field-name val))
+(define-maybe non-negative-integer)
+
+(define (on-off-boolean? val)
+  (boolean? val))
+(define (serialize-on-off-boolean field-name val)
+  (serialize-field field-name (if val "on" "off")))
+(define-maybe on-off-boolean)
+
+(define (y-n-boolean? val)
+  (boolean? val))
+(define (serialize-y-n-boolean field-name val)
+  (serialize-field field-name (if val "Y" "N")))
+
+(define-configuration tlp-configuration
+  (tlp
+   (package tlp)
+   "The TLP package.")
+
+  (tlp-enable?
+   (boolean #t)
+   "Set to true if you wish to enable TLP.")
+
+   (tlp-default-mode
+    (string "AC")
+    "Default mode when no power supply can be detected. Alternatives are
+AC and BAT.")
+
+   (disk-idle-secs-on-ac
+    (non-negative-integer 0)
+    "Number of seconds Linux kernel has to wait after the disk goes idle,
+before syncing on AC.")
+
+   (disk-idle-secs-on-bat
+    (non-negative-integer 2)
+    "Same as @code{disk-idle-ac} but on BAT mode.")
+
+   (max-lost-work-secs-on-ac
+    (non-negative-integer 15)
+    "Dirty pages flushing periodicity, expressed in seconds.")
+
+   (max-lost-work-secs-on-bat
+    (non-negative-integer 60)
+    "Same as @code{max-lost-work-secs-on-ac} but on BAT mode.")
+
+   (cpu-scaling-governor-on-ac
+    (maybe-space-separated-string-list 'disabled)
+    "CPU frequency scaling governor on AC mode. With intel_pstate
+driver, alternatives are powersave and performance. With acpi-cpufreq driver,
+alternatives are ondemand, powersave, performance and conservative.")
+
+   (cpu-scaling-governor-on-bat
+    (maybe-space-separated-string-list 'disabled)
+    "Same as @code{cpu-scaling-governor-on-ac} but on BAT mode.")
+
+   (cpu-scaling-min-freq-on-ac
+    (maybe-non-negative-integer 'disabled)
+    "Set the min available frequency for the scaling governor on AC.")
+
+   (cpu-scaling-max-freq-on-ac
+    (maybe-non-negative-integer 'disabled)
+    "Set the max available frequency for the scaling governor on AC.")
+
+   (cpu-scaling-min-freq-on-bat
+    (maybe-non-negative-integer 'disabled)
+    "Set the min available frequency for the scaling governor on BAT.")
+
+   (cpu-scaling-max-freq-on-bat
+    (maybe-non-negative-integer 'disabled)
+    "Set the max available frequency for the scaling governor on BAT.")
+
+   (cpu-min-perf-on-ac
+    (maybe-non-negative-integer 'disabled)
+    "Limit the min P-state to control the power dissipation of the CPU,
+in AC mode. Values are stated as a percentage of the available performance.")
+
+   (cpu-max-perf-on-ac
+    (maybe-non-negative-integer 'disabled)
+    "Limit the max P-state to control the power dissipation of the CPU,
+in AC mode. Values are stated as a percentage of the available performance.")
+
+   (cpu-min-perf-on-bat
+    (maybe-non-negative-integer 'disabled)
+    "Same as @code{cpu-min-perf-on-ac} on BAT mode.")
+
+   (cpu-max-perf-on-bat
+    (maybe-non-negative-integer 'disabled)
+    "Same as @code{cpu-max-perf-on-ac} on BAT mode.")
+
+   (cpu-boost-on-ac?
+    (maybe-boolean 'disabled)
+    "Enable CPU turbo boost feature on AC mode.")
+
+   (cpu-boost-on-bat?
+    (maybe-boolean 'disabled)
+    "Same as @code{cpu-boost-on-ac?} on BAT mode.")
+
+   (sched-powersave-on-ac?
+    (boolean #f)
+    "Allow Linux kernel to minimize the number of CPU cores/hyper-threads
+used under light load conditions.")
+
+   (sched-powersave-on-bat?
+    (boolean #t)
+    "Same as @code{sched-powersave-on-ac?} but on BAT mode.")
+
+   (nmi-watchdog?
+    (boolean #f)
+    "Enable Linux kernel NMI watchdog.")
+
+   (phc-controls
+    (maybe-string 'disabled)
+    "For Linux kernels with PHC patch applied, change CPU voltages.
+An example value would be @samp{\"F:V F:V F:V F:V\"}.")
+
+   (energy-perf-policy-on-ac
+    (string "performance")
+    "Set CPU performance versus energy saving policy on AC. Alternatives are
+performance, normal, powersave.")
+
+   (energy-perf-policy-on-bat
+    (string "powersave")
+    "Same as @code{energy-perf-policy-ac} but on BAT mode.")
+
+   (disks-devices
+    (space-separated-string-list '("sda"))
+    "Hard disk devices.")
+
+   (disk-apm-level-on-ac
+    (space-separated-string-list '("254" "254"))
+    "Hard disk advanced power management level.")
+
+   (disk-apm-level-on-bat
+    (space-separated-string-list '("128" "128"))
+    "Same as @code{disk-apm-bat} but on BAT mode.")
+
+   (disk-spindown-timeout-on-ac
+    (maybe-space-separated-string-list 'disabled)
+    "Hard disk spin down timeout. One value has to be specified for
+each declared hard disk.")
+
+   (disk-spindown-timeout-on-bat
+    (maybe-space-separated-string-list 'disabled)
+    "Same as @code{disk-spindown-timeout-on-ac} but on BAT mode.")
+
+   (disk-iosched
+    (maybe-space-separated-string-list 'disabled)
+    "Select IO scheduler for disk devices. One value has to be specified
+for each declared hard disk. Example alternatives are cfq, deadline and noop.")
+
+   (sata-linkpwr-on-ac
+    (string "max_performance")
+    "SATA aggressive link power management (ALPM) level. Alternatives are
+min_power, medium_power, max_performance.")
+
+   (sata-linkpwr-on-bat
+    (string "min_power")
+    "Same as @code{sata-linkpwr-ac} but on BAT mode.")
+
+   (sata-linkpwr-blacklist
+    (maybe-string 'disabled)
+    "Exclude specified SATA host devices for link power management.")
+
+   (ahci-runtime-pm-on-ac?
+    (maybe-on-off-boolean 'disabled)
+    "Enable Runtime Power Management for AHCI controller and disks
+on AC mode.")
+
+   (ahci-runtime-pm-on-bat?
+    (maybe-on-off-boolean 'disabled)
+    "Same as @code{ahci-runtime-pm-on-ac} on BAT mode.")
+
+   (ahci-runtime-pm-timeout
+    (non-negative-integer 15)
+    "Seconds of inactivity before disk is suspended.")
+
+   (pcie-aspm-on-ac
+    (string "performance")
+    "PCI Express Active State Power Management level. Alternatives are
+default, performance, powersave.")
+
+   (pcie-aspm-on-bat
+    (string "powersave")
+    "Same as @code{pcie-aspm-ac} but on BAT mode.")
+
+   (radeon-power-profile-on-ac
+    (string "high")
+    "Radeon graphics clock speed level. Alternatives are
+low, mid, high, auto, default.")
+
+   (radeon-power-profile-on-bat
+    (string "low")
+    "Same as @code{radeon-power-ac} but on BAT mode.")
+
+   (radeon-dpm-state-on-ac
+    (string "performance")
+    "Radeon dynamic power management method (DPM). Alternatives are
+battery, performance.")
+
+   (radeon-dpm-state-on-bat
+    (string "battery")
+    "Same as @code{radeon-dpm-state-ac} but on BAT mode.")
+
+   (radeon-dpm-perf-level-on-ac
+    (string "auto")
+    "Radeon DPM performance level. Alternatives are
+auto, low, high.")
+
+   (radeon-dpm-perf-level-on-bat
+    (string "auto")
+    "Same as @code{radeon-dpm-perf-ac} but on BAT mode.")
+
+   (wifi-pwr-on-ac?
+    (on-off-boolean #f)
+    "Wifi power saving mode.")
+
+   (wifi-pwr-on-bat?
+    (on-off-boolean #t)
+    "Same as @code{wifi-power-ac?} but on BAT mode.")
+
+   (wol-disable?
+    (y-n-boolean #t)
+    "Disable wake on LAN.")
+
+   (sound-power-save-on-ac
+    (non-negative-integer 0)
+    "Timeout duration in seconds before activating audio power saving
+ on Intel HDA and AC97 devices. A value of 0 disables power saving.")
+
+   (sound-power-save-on-bat
+    (non-negative-integer 1)
+    "Same as @code{sound-powersave-ac} but on BAT mode.")
+
+   (sound-power-save-controller?
+    (y-n-boolean #t)
+    "Disable controller in powersaving mode on Intel HDA devices.")
+
+   (bay-poweroff-on-bat?
+    (boolean #f)
+    "Enable optical drive in UltraBay/MediaBay on BAT mode.
+Drive can be powered on again by releasing (and reinserting) the eject lever
+or by pressing the disc eject button on newer models.")
+
+   (bay-device
+    (string "sr0")
+    "Name of the optical drive device to power off.")
+
+   (runtime-pm-on-ac
+    (string "on")
+    "Runtime Power Management for PCI(e) bus devices. Alternatives are
+on and auto.")
+
+   (runtime-pm-on-bat
+    (string "auto")
+    "Same as @code{runtime-pm-ac} but on BAT mode.")
+
+   (runtime-pm-all?
+    (boolean #t)
+    "Runtime Power Management for all PCI(e) bus devices, except
+blacklisted ones.")
+
+   (runtime-pm-blacklist
+    (maybe-space-separated-string-list 'disabled)
+    "Exclude specified PCI(e) devices adresses from Runtime Power Management.")
+
+   (runtime-pm-driver-blacklist
+    (space-separated-string-list '("radeon" "nouveau"))
+    "Exclude PCI(e) devices assigned to the specified drivers from
+Runtime Power Management.")
+
+   (usb-autosuspend?
+    (boolean #t)
+    "Enable USB autosuspend feature.")
+
+   (usb-blacklist
+    (maybe-string 'disabled)
+    "Exclude specified devices from USB autosuspend.")
+
+   (usb-blacklist-wwan?
+    (boolean #t)
+    "Exclude WWAN devices from USB autosuspend.")
+
+   (usb-whitelist
+    (maybe-string 'disabled)
+    "Include specified devices into USB autosuspend, even if they are
+already excluded by the driver or via @code{usb-blacklist-wwan?}.")
+
+   (usb-autosuspend-disable-on-shutdown?
+    (maybe-boolean 'disabled)
+    "Enable USB autosuspend before shutdown.")
+
+  (restore-device-state-on-startup?
+   (boolean #f)
+   "Restore radio device state (bluetooth, wifi, wwan) from previous
+shutdown on system startup."))
+
+
+(define (tlp-shepherd-service config)
+  (let* ((tlp-bin (file-append
+                   (tlp-configuration-tlp config) "/bin/tlp"))
+         (tlp-action (lambda args
+                              #~(lambda _
+                                  (zero? (system* #$tlp-bin #$@args))))))
+    (list (shepherd-service
+           (documentation "Run TLP script.")
+           (provision '(tlp))
+           (requirement '(syslogd user-processes))
+           (start (tlp-action "init" "start"))
+           (stop  (tlp-action "init" "stop"))))))
+
+(define (tlp-activation config)
+  (let* ((config-dir "/etc")
+         (config-str (with-output-to-string
+                       (lambda ()
+                         (serialize-configuration
+                          config
+                          tlp-configuration-fields))))
+         (config-file (plain-file "tlp" config-str)))
+  #~(begin
+      (use-modules (guix build utils))
+      (mkdir-p #$config-dir)
+      (copy-file #$config-file (string-append #$config-dir "/tlp")))))
+
+(define tlp-service-type
+  (service-type
+   (name 'tlp)
+   (extensions
+    (list
+     (service-extension shepherd-root-service-type
+                        tlp-shepherd-service)
+     (service-extension udev-service-type
+                        (compose list tlp-configuration-tlp))
+     (service-extension activation-service-type
+                        tlp-activation)))))
+
+(define (generate-tlp-documentation)
+  (generate-documentation
+   `((tlp-configuration ,tlp-configuration-fields))
+    'tlp-configuration))
-- 
2.12.0

Re: [PATCH 3/5] gnu: Add tlp service.

[Clément Lassieur]

Mathieu Othacehe <m.othacehe@gmail.com> writes:

> * gnu/services/pm.scm: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add gnu/services/tlp.scm.
> * doc/guix.texi (Power management Services): New section.

Hi Mathieu,

Very nice!

On my x200, with tlp, powertop reports:

    The battery reports a discharge rate of 10.1 W
    The estimated remaining time is 4 hours, 52 minutes

Without tlp:

    The battery reports a discharge rate of 11.9 W
    The estimated remaining time is 4 hours, 11 minutes


Here are a few comments:

[...]

> +(define-module (gnu services pm)
> +  #:use-module (guix gexp)
> +  #:use-module (guix packages)
> +  #:use-module (guix records)
> +  #:use-module (gnu packages linux)
> +  #:use-module (gnu services)
> +  #:use-module (gnu services base)
> +  #:use-module (gnu services configuration)
> +  #:use-module (gnu services shepherd)
> +  #:use-module (gnu system shadow)
> +  #:export (tlp-service-type
> +            tlp-configuration
> +            generate-tlp-documentation))

I don't think generate-tlp-documentation needs to be exported.

> +(define (uglify-field-name field-name)
> +  (let ((str (symbol->string field-name)))
> +    (string-join (string-split
> +                  (string-upcase
> +                   (if (string-suffix? "?" str)
> +                       (substring str 0 (1- (string-length str)))
> +                       str))
> +                  #\-)
> +                 "_")))
> +
> +(define (serialize-field field-name val)
> +  (format #t "~a=~a\n" (uglify-field-name field-name) val))
> +
> +(define (serialize-boolean field-name val)
> +  (serialize-field field-name (if val "1" "0")))
> +(define-maybe boolean)
> +
> +(define (serialize-string field-name val)
> +  (serialize-field field-name val))
> +(define-maybe string)
> +
> +(define (space-separated-string-list? val)
> +  (and (list? val)
> +       (and-map (lambda (x)
> +                  (and (string? x) (not (string-index x #\space))))
> +                val)))
> +(define (serialize-space-separated-string-list field-name val)
> +  (serialize-field field-name
> +                   (format #f "~s"
> +                           (string-join val " "))))
> +(define-maybe space-separated-string-list)
> +
> +(define (non-negative-integer? val)
> +  (and (exact-integer? val) (not (negative? val))))
> +(define (serialize-non-negative-integer field-name val)
> +  (serialize-field field-name val))
> +(define-maybe non-negative-integer)
> +
> +(define (on-off-boolean? val)
> +  (boolean? val))
> +(define (serialize-on-off-boolean field-name val)
> +  (serialize-field field-name (if val "on" "off")))
> +(define-maybe on-off-boolean)
> +
> +(define (y-n-boolean? val)
> +  (boolean? val))
> +(define (serialize-y-n-boolean field-name val)
> +  (serialize-field field-name (if val "Y" "N")))
> +
> +(define-configuration tlp-configuration
> +  (tlp
> +   (package tlp)
> +   "The TLP package.")
> +
> +  (tlp-enable?
> +   (boolean #t)
> +   "Set to true if you wish to enable TLP.")

From this point, the indentation is broken.

> +   (tlp-default-mode
> +    (string "AC")
> +    "Default mode when no power supply can be detected. Alternatives are
> +AC and BAT.")
> +
> +   (disk-idle-secs-on-ac
> +    (non-negative-integer 0)
> +    "Number of seconds Linux kernel has to wait after the disk goes idle,
> +before syncing on AC.")
> +
> +   (disk-idle-secs-on-bat
> +    (non-negative-integer 2)
> +    "Same as @code{disk-idle-ac} but on BAT mode.")
> +
> +   (max-lost-work-secs-on-ac
> +    (non-negative-integer 15)
> +    "Dirty pages flushing periodicity, expressed in seconds.")
> +
> +   (max-lost-work-secs-on-bat
> +    (non-negative-integer 60)
> +    "Same as @code{max-lost-work-secs-on-ac} but on BAT mode.")
> +
> +   (cpu-scaling-governor-on-ac
> +    (maybe-space-separated-string-list 'disabled)
> +    "CPU frequency scaling governor on AC mode. With intel_pstate
> +driver, alternatives are powersave and performance. With acpi-cpufreq driver,
> +alternatives are ondemand, powersave, performance and conservative.")

Please, could you put two spaces between phrases? :) As in "and
performance.  With...".  I've seen this in other parts of your patch as
well.

[...]

> +(define (tlp-shepherd-service config)
> +  (let* ((tlp-bin (file-append
> +                   (tlp-configuration-tlp config) "/bin/tlp"))
> +         (tlp-action (lambda args
> +                              #~(lambda _
> +                                  (zero? (system* #$tlp-bin #$@args))))))
> +    (list (shepherd-service
> +           (documentation "Run TLP script.")
> +           (provision '(tlp))
> +           (requirement '(syslogd user-processes))

I have not been able to see anything related to tlp in
/var/log/messages.  And if I set trace mode (TLP_DEBUG, in /etc/tlp), I
have an error message: "logger: unknown facility name: debug", which I
think could be patched in tlp source, maybe by removing "-p debug".  Or
maybe it is logger that needs to be patched, I don't know.

Otherwise syslogd would not be needed here.  WDYT?

BTW, could you consider adding TLP_DEBUG to the service?

> +           (start (tlp-action "init" "start"))
> +           (stop  (tlp-action "init" "stop"))))))
> +
> +(define (tlp-activation config)
> +  (let* ((config-dir "/etc")
> +         (config-str (with-output-to-string
> +                       (lambda ()
> +                         (serialize-configuration
> +                          config
> +                          tlp-configuration-fields))))
> +         (config-file (plain-file "tlp" config-str)))
> +  #~(begin
> +      (use-modules (guix build utils))
> +      (mkdir-p #$config-dir)
> +      (copy-file #$config-file (string-append #$config-dir "/tlp")))))

Here I think it is better to wrap the gexp in (with-imported-modules
'((guix build utils)) ...), as in the CUPS service for example, even
though I don't understand fully the consequences of not doing it.

[...]

Thanks for this :)
Clément

Re: [PATCH 3/5] gnu: Add tlp service.

[Mathieu Othacehe]

Hi Clément !

Thanks for the review. I agree with all
your points, I'll publish a v2 but on debbugs this time !

Thank you,

Mathieu

> Here are a few comments:
>
> [...]
>
>> +(define-module (gnu services pm)
>> +  #:use-module (guix gexp)
>> +  #:use-module (guix packages)
>> +  #:use-module (guix records)
>> +  #:use-module (gnu packages linux)
>> +  #:use-module (gnu services)
>> +  #:use-module (gnu services base)
>> +  #:use-module (gnu services configuration)
>> +  #:use-module (gnu services shepherd)
>> +  #:use-module (gnu system shadow)
>> +  #:export (tlp-service-type
>> +            tlp-configuration
>> +            generate-tlp-documentation))
>
> I don't think generate-tlp-documentation needs to be exported.
>
>> +(define (uglify-field-name field-name)
>> +  (let ((str (symbol->string field-name)))
>> +    (string-join (string-split
>> +                  (string-upcase
>> +                   (if (string-suffix? "?" str)
>> +                       (substring str 0 (1- (string-length str)))
>> +                       str))
>> +                  #\-)
>> +                 "_")))
>> +
>> +(define (serialize-field field-name val)
>> +  (format #t "~a=~a\n" (uglify-field-name field-name) val))
>> +
>> +(define (serialize-boolean field-name val)
>> +  (serialize-field field-name (if val "1" "0")))
>> +(define-maybe boolean)
>> +
>> +(define (serialize-string field-name val)
>> +  (serialize-field field-name val))
>> +(define-maybe string)
>> +
>> +(define (space-separated-string-list? val)
>> +  (and (list? val)
>> +       (and-map (lambda (x)
>> +                  (and (string? x) (not (string-index x #\space))))
>> +                val)))
>> +(define (serialize-space-separated-string-list field-name val)
>> +  (serialize-field field-name
>> +                   (format #f "~s"
>> +                           (string-join val " "))))
>> +(define-maybe space-separated-string-list)
>> +
>> +(define (non-negative-integer? val)
>> +  (and (exact-integer? val) (not (negative? val))))
>> +(define (serialize-non-negative-integer field-name val)
>> +  (serialize-field field-name val))
>> +(define-maybe non-negative-integer)
>> +
>> +(define (on-off-boolean? val)
>> +  (boolean? val))
>> +(define (serialize-on-off-boolean field-name val)
>> +  (serialize-field field-name (if val "on" "off")))
>> +(define-maybe on-off-boolean)
>> +
>> +(define (y-n-boolean? val)
>> +  (boolean? val))
>> +(define (serialize-y-n-boolean field-name val)
>> +  (serialize-field field-name (if val "Y" "N")))
>> +
>> +(define-configuration tlp-configuration
>> +  (tlp
>> +   (package tlp)
>> +   "The TLP package.")
>> +
>> +  (tlp-enable?
>> +   (boolean #t)
>> +   "Set to true if you wish to enable TLP.")
>
> From this point, the indentation is broken.
>
>> +   (tlp-default-mode
>> +    (string "AC")
>> +    "Default mode when no power supply can be detected. Alternatives are
>> +AC and BAT.")
>> +
>> +   (disk-idle-secs-on-ac
>> +    (non-negative-integer 0)
>> +    "Number of seconds Linux kernel has to wait after the disk goes idle,
>> +before syncing on AC.")
>> +
>> +   (disk-idle-secs-on-bat
>> +    (non-negative-integer 2)
>> +    "Same as @code{disk-idle-ac} but on BAT mode.")
>> +
>> +   (max-lost-work-secs-on-ac
>> +    (non-negative-integer 15)
>> +    "Dirty pages flushing periodicity, expressed in seconds.")
>> +
>> +   (max-lost-work-secs-on-bat
>> +    (non-negative-integer 60)
>> +    "Same as @code{max-lost-work-secs-on-ac} but on BAT mode.")
>> +
>> +   (cpu-scaling-governor-on-ac
>> +    (maybe-space-separated-string-list 'disabled)
>> +    "CPU frequency scaling governor on AC mode. With intel_pstate
>> +driver, alternatives are powersave and performance. With acpi-cpufreq driver,
>> +alternatives are ondemand, powersave, performance and conservative.")
>
> Please, could you put two spaces between phrases? :) As in "and
> performance.  With...".  I've seen this in other parts of your patch as
> well.
>
> [...]
>
>> +(define (tlp-shepherd-service config)
>> +  (let* ((tlp-bin (file-append
>> +                   (tlp-configuration-tlp config) "/bin/tlp"))
>> +         (tlp-action (lambda args
>> +                              #~(lambda _
>> +                                  (zero? (system* #$tlp-bin #$@args))))))
>> +    (list (shepherd-service
>> +           (documentation "Run TLP script.")
>> +           (provision '(tlp))
>> +           (requirement '(syslogd user-processes))
>
> I have not been able to see anything related to tlp in
> /var/log/messages.  And if I set trace mode (TLP_DEBUG, in /etc/tlp), I
> have an error message: "logger: unknown facility name: debug", which I
> think could be patched in tlp source, maybe by removing "-p debug".  Or
> maybe it is logger that needs to be patched, I don't know.
>
> Otherwise syslogd would not be needed here.  WDYT?
>
> BTW, could you consider adding TLP_DEBUG to the service?
>
>> +           (start (tlp-action "init" "start"))
>> +           (stop  (tlp-action "init" "stop"))))))
>> +
>> +(define (tlp-activation config)
>> +  (let* ((config-dir "/etc")
>> +         (config-str (with-output-to-string
>> +                       (lambda ()
>> +                         (serialize-configuration
>> +                          config
>> +                          tlp-configuration-fields))))
>> +         (config-file (plain-file "tlp" config-str)))
>> +  #~(begin
>> +      (use-modules (guix build utils))
>> +      (mkdir-p #$config-dir)
>> +      (copy-file #$config-file (string-append #$config-dir "/tlp")))))
>
> Here I think it is better to wrap the gexp in (with-imported-modules
> '((guix build utils)) ...), as in the CUPS service for example, even
> though I don't understand fully the consequences of not doing it.
>
> [...]
>
> Thanks for this :)
> Clément

Re: [PATCH 3/5] gnu: Add tlp service.

[Mathieu Othacehe]

> I have not been able to see anything related to tlp in
> /var/log/messages.  And if I set trace mode (TLP_DEBUG, in /etc/tlp), I
> have an error message: "logger: unknown facility name: debug", which I
> think could be patched in tlp source, maybe by removing "-p debug".  Or
> maybe it is logger that needs to be patched, I don't know.
>
> Otherwise syslogd would not be needed here.  WDYT?

Well when you call logger -p debug, it means with facility = debug and
priority = default one = notice. syslog does only support predefined
facilities like auth, cron, kern ...
The complete list can be found here (section 19.1):

https://www.gnu.org/software/inetutils/manual/html_node/syslogd-invocation.html

The facility debug does not exist and cannot be created.
So logger -p debug makes no sense to me.

However, debug is a priority, so logger -p user.debug or daemon.debug
would be correct.

I could patch those logger calls in guix tlp package but I prefer to
contact tlp maintainer to see if he can fix it upstream.

>
> BTW, could you consider adding TLP_DEBUG to the service?

It is not needed for now as it enables a broken feature !

I will remove syslogd requirement from this service for now.

Thanks for testing this out !

Mathieu

Re: [PATCH 3/5] gnu: Add tlp service.

[Clément Lassieur]

Sorry I forgot two things:

> +(define (tlp-activation config)
> +  (let* ((config-dir "/etc")
> +         (config-str (with-output-to-string
> +                       (lambda ()
> +                         (serialize-configuration
> +                          config
> +                          tlp-configuration-fields))))
> +         (config-file (plain-file "tlp" config-str)))
> +  #~(begin
> +      (use-modules (guix build utils))
> +      (mkdir-p #$config-dir)
            ^
Is this necessary?  I think /etc should exist already.

Also, "tlp stat" does not work.

Re: [PATCH 3/5] gnu: Add tlp service.

[Mathieu Othacehe]

> Also, "tlp stat" does not work.

It seems to work for me, could you elaborate on that ?

Note that you need to be root.

Mathieu

Re: [PATCH 3/5] gnu: Add tlp service.

[Clément Lassieur]

>> Also, "tlp stat" does not work.
>
> It seems to work for me, could you elaborate on that ?

Oh I see, it was because tlp wasn't installed...

Running it directly from the store like this:

    /gnu/store/ymq6khzhnyxsq5z1mi2jnri93ww0ysji-tlp-0.9/bin/tlp init stat

Produced:

    /gnu/store/ymq6khzhnyxsq5z1mi2jnri93ww0ysji-tlp-0.9/bin/.tlp-real: line 322: tlp-stat: command not found

But I guess it is not a problem if the service does not use it.  And a
user willing to call tlp directy should install it.  So please ignore my
comment!

[PATCH 4/5] doc: Re-generate openvpn service documentation.

[Mathieu Othacehe]

* doc/guix.texi (VPN Services): Generate documentation with
generate-openvpn-server-documentation and
generate-openvpn-client-documentation helpers and guile 2.1.7.
---
 doc/guix.texi | 88 +++++++++++++++++++++++++++++------------------------------
 1 file changed, 44 insertions(+), 44 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index d8d9d7a98..0df8913c4 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -13010,19 +13010,19 @@ Both can be run simultaneously.
 
 Available @code{openvpn-client-configuration} fields are:
 
-@deftypevr @code{openvpn-client-configuration} parameter package openvpn
+@deftypevr {@code{openvpn-client-configuration} parameter} package openvpn
 The OpenVPN package.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-client-configuration} parameter string pid-file
+@deftypevr {@code{openvpn-client-configuration} parameter} string pid-file
 The OpenVPN pid file.
 
 Defaults to @samp{"/var/run/openvpn/openvpn.pid"}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-client-configuration} parameter proto proto
+@deftypevr {@code{openvpn-client-configuration} parameter} proto proto
 The protocol (UDP or TCP) used to open a channel between clients and
 servers.
 
@@ -13030,21 +13030,21 @@ Defaults to @samp{udp}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-client-configuration} parameter dev dev
+@deftypevr {@code{openvpn-client-configuration} parameter} dev dev
 The device type used to represent the VPN connection.
 
 Defaults to @samp{tun}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-client-configuration} parameter string ca
+@deftypevr {@code{openvpn-client-configuration} parameter} string ca
 The certificate authority to check connections against.
 
 Defaults to @samp{"/etc/openvpn/ca.crt"}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-client-configuration} parameter string cert
+@deftypevr {@code{openvpn-client-configuration} parameter} string cert
 The certificate of the machine the daemon is running on.  It should be
 signed by the authority given in @code{ca}.
 
@@ -13052,7 +13052,7 @@ Defaults to @samp{"/etc/openvpn/client.crt"}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-client-configuration} parameter string key
+@deftypevr {@code{openvpn-client-configuration} parameter} string key
 The key of the machine the daemon is running on.  It must be the key whose
 certificate is @code{cert}.
 
@@ -13060,21 +13060,21 @@ Defaults to @samp{"/etc/openvpn/client.key"}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-client-configuration} parameter boolean comp-lzo?
+@deftypevr {@code{openvpn-client-configuration} parameter} boolean comp-lzo?
 Whether to use the lzo compression algorithm.
 
 Defaults to @samp{#t}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-client-configuration} parameter boolean persist-key?
+@deftypevr {@code{openvpn-client-configuration} parameter} boolean persist-key?
 Don't re-read key files across SIGUSR1 or --ping-restart.
 
 Defaults to @samp{#t}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-client-configuration} parameter boolean persist-tun?
+@deftypevr {@code{openvpn-client-configuration} parameter} boolean persist-tun?
 Don't close and reopen TUN/TAP device or run up/down scripts across
 SIGUSR1 or --ping-restart restarts.
 
@@ -13082,14 +13082,14 @@ Defaults to @samp{#t}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-client-configuration} parameter number verbosity
+@deftypevr {@code{openvpn-client-configuration} parameter} number verbosity
 Verbosity level.
 
 Defaults to @samp{3}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-client-configuration} parameter tls-auth-client tls-auth
+@deftypevr {@code{openvpn-client-configuration} parameter} tls-auth-client tls-auth
 Add an additional layer of HMAC authentication on top of the TLS control
 channel to protect against DoS attacks.
 
@@ -13097,42 +13097,42 @@ Defaults to @samp{#f}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-client-configuration} parameter key-usage verify-key-usage?
+@deftypevr {@code{openvpn-client-configuration} parameter} key-usage verify-key-usage?
 Whether to check the server certificate has server usage extension.
 
 Defaults to @samp{#t}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-client-configuration} parameter bind bind?
+@deftypevr {@code{openvpn-client-configuration} parameter} bind bind?
 Bind to a specific local port number.
 
 Defaults to @samp{#f}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-client-configuration} parameter resolv-retry resolv-retry?
+@deftypevr {@code{openvpn-client-configuration} parameter} resolv-retry resolv-retry?
 Retry resolving server address.
 
 Defaults to @samp{#t}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-client-configuration} parameter openvpn-remote-list remote
+@deftypevr {@code{openvpn-client-configuration} parameter} openvpn-remote-list remote
 A list of remote servers to connect to.
 
 Defaults to @samp{()}.
 
 Available @code{openvpn-remote-configuration} fields are:
 
-@deftypevr @code{openvpn-remote-configuration} parameter string name
+@deftypevr {@code{openvpn-remote-configuration} parameter} string name
 Server name.
 
 Defaults to @samp{"my-server"}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-remote-configuration} parameter number port
+@deftypevr {@code{openvpn-remote-configuration} parameter} number port
 Port number the server listens to.
 
 Defaults to @samp{1194}.
@@ -13146,19 +13146,19 @@ Defaults to @samp{1194}.
 
 Available @code{openvpn-server-configuration} fields are:
 
-@deftypevr @code{openvpn-server-configuration} parameter package openvpn
+@deftypevr {@code{openvpn-server-configuration} parameter} package openvpn
 The OpenVPN package.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter string pid-file
+@deftypevr {@code{openvpn-server-configuration} parameter} string pid-file
 The OpenVPN pid file.
 
 Defaults to @samp{"/var/run/openvpn/openvpn.pid"}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter proto proto
+@deftypevr {@code{openvpn-server-configuration} parameter} proto proto
 The protocol (UDP or TCP) used to open a channel between clients and
 servers.
 
@@ -13166,21 +13166,21 @@ Defaults to @samp{udp}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter dev dev
+@deftypevr {@code{openvpn-server-configuration} parameter} dev dev
 The device type used to represent the VPN connection.
 
 Defaults to @samp{tun}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter string ca
+@deftypevr {@code{openvpn-server-configuration} parameter} string ca
 The certificate authority to check connections against.
 
 Defaults to @samp{"/etc/openvpn/ca.crt"}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter string cert
+@deftypevr {@code{openvpn-server-configuration} parameter} string cert
 The certificate of the machine the daemon is running on.  It should be
 signed by the authority given in @code{ca}.
 
@@ -13188,7 +13188,7 @@ Defaults to @samp{"/etc/openvpn/client.crt"}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter string key
+@deftypevr {@code{openvpn-server-configuration} parameter} string key
 The key of the machine the daemon is running on.  It must be the key whose
 certificate is @code{cert}.
 
@@ -13196,21 +13196,21 @@ Defaults to @samp{"/etc/openvpn/client.key"}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter boolean comp-lzo?
+@deftypevr {@code{openvpn-server-configuration} parameter} boolean comp-lzo?
 Whether to use the lzo compression algorithm.
 
 Defaults to @samp{#t}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter boolean persist-key?
+@deftypevr {@code{openvpn-server-configuration} parameter} boolean persist-key?
 Don't re-read key files across SIGUSR1 or --ping-restart.
 
 Defaults to @samp{#t}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter boolean persist-tun?
+@deftypevr {@code{openvpn-server-configuration} parameter} boolean persist-tun?
 Don't close and reopen TUN/TAP device or run up/down scripts across
 SIGUSR1 or --ping-restart restarts.
 
@@ -13218,14 +13218,14 @@ Defaults to @samp{#t}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter number verbosity
+@deftypevr {@code{openvpn-server-configuration} parameter} number verbosity
 Verbosity level.
 
 Defaults to @samp{3}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter tls-auth-server tls-auth
+@deftypevr {@code{openvpn-server-configuration} parameter} tls-auth-server tls-auth
 Add an additional layer of HMAC authentication on top of the TLS control
 channel to protect against DoS attacks.
 
@@ -13233,56 +13233,56 @@ Defaults to @samp{#f}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter number port
+@deftypevr {@code{openvpn-server-configuration} parameter} number port
 Specifies the port number on which the server listens.
 
 Defaults to @samp{1194}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter ip-mask server
+@deftypevr {@code{openvpn-server-configuration} parameter} ip-mask server
 An ip and mask specifying the subnet inside the virtual network.
 
 Defaults to @samp{"10.8.0.0 255.255.255.0"}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter cidr6 server-ipv6
+@deftypevr {@code{openvpn-server-configuration} parameter} cidr6 server-ipv6
 A CIDR notation specifying the IPv6 subnet inside the virtual network.
 
 Defaults to @samp{#f}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter string dh
+@deftypevr {@code{openvpn-server-configuration} parameter} string dh
 The Diffie-Hellman parameters file.
 
 Defaults to @samp{"/etc/openvpn/dh2048.pem"}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter string ifconfig-pool-persist
+@deftypevr {@code{openvpn-server-configuration} parameter} string ifconfig-pool-persist
 The file that records client IPs.
 
 Defaults to @samp{"/etc/openvpn/ipp.txt"}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter gateway redirect-gateway?
+@deftypevr {@code{openvpn-server-configuration} parameter} gateway redirect-gateway?
 When true, the server will act as a gateway for its clients.
 
 Defaults to @samp{#f}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter boolean client-to-client?
+@deftypevr {@code{openvpn-server-configuration} parameter} boolean client-to-client?
 When true, clients are alowed to talk to each other inside the VPN.
 
 Defaults to @samp{#f}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter keepalive keepalive
+@deftypevr {@code{openvpn-server-configuration} parameter} keepalive keepalive
 Causes ping-like messages to be sent back and forth over the link so
 that each side knows when the other side has gone down.  @code{keepalive}
 requires a pair.  The first element is the period of the ping sending,
@@ -13291,14 +13291,14 @@ down.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter number max-clients
+@deftypevr {@code{openvpn-server-configuration} parameter} number max-clients
 The maximum number of clients.
 
 Defaults to @samp{100}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter string status
+@deftypevr {@code{openvpn-server-configuration} parameter} string status
 The status file.  This file shows a small report on current connection.
 It is trunkated and rewritten every minute.
 
@@ -13306,28 +13306,28 @@ Defaults to @samp{"/var/run/openvpn/status"}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-server-configuration} parameter openvpn-ccd-list client-config-dir
+@deftypevr {@code{openvpn-server-configuration} parameter} openvpn-ccd-list client-config-dir
 The list of configuration for some clients.
 
 Defaults to @samp{()}.
 
 Available @code{openvpn-ccd-configuration} fields are:
 
-@deftypevr @code{openvpn-ccd-configuration} parameter string name
+@deftypevr {@code{openvpn-ccd-configuration} parameter} string name
 Client name.
 
 Defaults to @samp{"client"}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-ccd-configuration} parameter ip-mask iroute
+@deftypevr {@code{openvpn-ccd-configuration} parameter} ip-mask iroute
 Client own network
 
 Defaults to @samp{#f}.
 
 @end deftypevr
 
-@deftypevr @code{openvpn-ccd-configuration} parameter ip-mask ifconfig-push
+@deftypevr {@code{openvpn-ccd-configuration} parameter} ip-mask ifconfig-push
 Client VPN IP.
 
 Defaults to @samp{#f}.
-- 
2.12.0

Re: [PATCH 4/5] doc: Re-generate openvpn service documentation.

[Clément Lassieur]

Mathieu Othacehe <m.othacehe@gmail.com> writes:

> * doc/guix.texi (VPN Services): Generate documentation with
> generate-openvpn-server-documentation and
> generate-openvpn-client-documentation helpers and guile 2.1.7.
> ---
>  doc/guix.texi | 88 +++++++++++++++++++++++++++++------------------------------
>  1 file changed, 44 insertions(+), 44 deletions(-)

LGTM, thanks :)

[PATCH 5/5] services: Fix a typo which was corrected in generated doc.

[Mathieu Othacehe]

* gnu/services/vpn.scm (define-split-configuration): Fix typo.
---
 gnu/services/vpn.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gnu/services/vpn.scm b/gnu/services/vpn.scm
index 844a11b3d..e1a04a985 100644
--- a/gnu/services/vpn.scm
+++ b/gnu/services/vpn.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2017 Julien Lepiller <julien@lepiller.eu>
 ;;; Copyright © 2017 Clément Lassieur <clement@lassieur.org>
+;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -279,7 +280,7 @@ by the authority given in @code{ca}.")
 
    (key
     (string "/etc/openvpn/client.key")
-    "The key of the machine the daemon is running on. It must be the whose
+    "The key of the machine the daemon is running on. It must be the key whose
 certificate is @code{cert}.")
 
    (comp-lzo?
-- 
2.12.0

Re: [PATCH 5/5] services: Fix a typo which was corrected in generated doc.

[Clément Lassieur]

> * gnu/services/vpn.scm (define-split-configuration): Fix typo.
> ---
>  gnu/services/vpn.scm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/services/vpn.scm b/gnu/services/vpn.scm
> index 844a11b3d..e1a04a985 100644
> --- a/gnu/services/vpn.scm
> +++ b/gnu/services/vpn.scm
> @@ -1,6 +1,7 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2017 Julien Lepiller <julien@lepiller.eu>
>  ;;; Copyright © 2017 Clément Lassieur <clement@lassieur.org>
> +;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -279,7 +280,7 @@ by the authority given in @code{ca}.")
>  
>     (key
>      (string "/etc/openvpn/client.key")
> -    "The key of the machine the daemon is running on. It must be the whose
> +    "The key of the machine the daemon is running on. It must be the key whose
>  certificate is @code{cert}.")
>  
>     (comp-lzo?

The commit message should be: "services: openvpn: ...", but otherwise
LGTM :)