Re: [PATCH 08/15] gnu: build: Add Linux container module.

[Thompson, David]

On Tue, Jul 7, 2015 at 9:51 AM, Ludovic Courtès <address@hidden> wrote:
> David Thompson <address@hidden> skribis:
>
>> * gnu/build/linux-container.scm: New file.
>> * gnu-system.am (GNU_SYSTEM_MODULES): Add it.
>> * .dir-locals.el: Add Scheme indent rules for 'call-with-clone', 
>> 'with-clone',
>>   'call-with-container', and 'container-excursion'.
>> * tests/containers.scm: New file.
>> * Makefile.am (SCM_TESTS): Add it.
>
> [...]
>
>> +(define (mount-flags->bit-mask flags)
>> +  "Return the number suitable for the 'flags' argument of 'mount' that
>> +corresponds to the symbols listed in FLAGS."
>> +  (let loop ((flags flags))
>> +    (match flags
>> +      (('read-only rest ...)
>> +       (logior MS_RDONLY (loop rest)))
>> +      (('bind-mount rest ...)
>> +       (logior MS_BIND (loop rest)))
>> +      (('no-suid rest ...)
>> +       (logior MS_NOSUID (loop rest)))
>> +      (('no-dev rest ...)
>> +       (logior MS_NODEV (loop rest)))
>> +      (('no-exec rest ...)
>> +       (logior MS_NOEXEC (loop rest)))
>> +      (()
>> +       0))))
>> +
>> +(define* (mount-file-system spec root)
>> +  "Mount the file system described by SPEC under ROOT.  SPEC must have the
>> +form:
>> +
>> +  (DEVICE TITLE MOUNT-POINT TYPE (FLAGS ...) OPTIONS CHECK?)
>
> Could we share these two procedures with (gnu build file-systems)?
>
> I suspect the problem you encountered is that (gnu build file-systems)
> doesn’t use (guix build syscalls), and instead expects the
> statically-linked Guile with the guile-syscalls.patch.

Yes, that is exactly what happened.

> To work around that, I think we should shamelessly add something like
> this in (gnu build file-system):
>
>   (unless (defined? 'mount)
>     (module-use! (current-module)
>                  (resolve-interface '(guix build syscalls))))
>
> WDYT?

Sounds good.  I've attached an additional patch that does this.

>> +(define (namespaces->bit-mask namespaces)
>> +  "Return the number suitable for the 'flags' argument of 'clone' that
>> +corresponds to the symbols in NAMESPACES."
>
> I would be in favor of “name spaces” (two words), but maybe that’s
> because I’m an old fart, so I won’t insist.

All of the Linux documentation uses "namespaces" as a single word, so
I'd prefer to keep it consistent, but I don't care too much.

>> +(test-assert "call-with-container, pid namespace"
>> +  (zero?
>> +   (call-with-container '()
>> +     (lambda ()
>> +       (match (primitive-fork)
>> +         (0
>> +          ;; The first forked process in the new pid namespace is pid 2.
>> +          (assert-exit (= 2 (getpid))))
>
> But its parent doesn’t sees itself as PID 1?

Only if it were to 'exec'.  The reason being that PID namespaces are
special in how they treat the process that created the new namespace.
It's somewhat confusing.

How do the new patches look?

Thanks!

- Dave

0001-build-file-systems-Import-guix-build-syscalls-for-no.patch
Description: Text Data

0002-gnu-build-Add-Linux-container-module.patch
Description: Text Data