[bug#50133] [PATCH 0/6] Add lxd package and service.

["Ludovic Courtès" <ludo@gnu.org>]

Hi Andrew,

The whole series LGTM and I’ve already applied patch 1–5.

Some comments below about the service:

Andrew Whatson <whatson@gmail.com> skribis:

> * gnu/services/virtualization.scm (lxd-configuration): New type.
> (%lxd-accounts, lxd-service-type): New variables.
> (%lxd-activation, lxd-shepherd-service): New procedures.
> * gnu/system/file-systems.scm (%elogind-file-systems): Add
> "/sys/fs/cgroup/systemd" file-system.

[...]

> +(define lxd-service-type
> +  (service-type
> +   (name 'lxd)
> +   (extensions
> +    (list (service-extension activation-service-type
> +                             %lxd-activation)
> +          (service-extension shepherd-root-service-type
> +                             lxd-shepherd-service)
> +          (service-extension account-service-type
> +                             (const %lxd-accounts))))
> +   (default-value (lxd-configuration))))

Please add a ‘description’ field.

> index b9eda80958..7c3777298b 100644
> --- a/gnu/system/file-systems.scm
> +++ b/gnu/system/file-systems.scm
> @@ -499,6 +499,17 @@ TARGET in the other system."
>             (check? #f)
>             (options "none,name=elogind")
>             (create-mount-point? #t)
> +           (dependencies (list (car %control-groups))))
> +         ;; The systemd cgroup needs to exist to run systemd inside linux
> +         ;; containers (eg. via LXD).  This is *not* required for elogind, but
> +         ;; keeping it with the other systemd hacks seemed sensible, for now.
> +         (file-system
> +           (device "cgroup")
> +           (mount-point "/sys/fs/cgroup/systemd")
> +           (type "cgroup")
> +           (check? #f)
> +           (options "none,name=systemd")
> +           (create-mount-point? #t)
>             (dependencies (list (car %control-groups)))))
>     %control-groups))

Instead of adding it here, how about extending
‘file-system-service-type’ instead, similar to what
‘qemu-binfmt-service-type’ does?  That way, the extra requirement would
be limited to LXD.

Two other things, could you add:

  1. documentation in the manual under “Virtualization Services”, with a
     commented config example?

  2. (ideally) a system test to ensure that the basics of the service
     are working?

TIA!

Ludo’.