[lxc-devel] RFC: Refactoring lxc-attach

[Christian Seiler]

Hi there,

Just a comment on my patch:

> This patch just changes the code in the most simple manner to use
> clone() instead of fork(). Since clone() requires a function to be
> called instead of returning 0, we move the code of the child into a
> function child_main.

I wanted to make the patch as least invasive as possible, for this
reason I've just moved code around a bit, without actually adding other
improvements. (Btw. I've tested that the code works on my system but
since the problem is very difficult to reproduce, so I couldn't
cross-check against that.)

I've gone this route because the roadmap that was posted on the mailing
list [1] contains a point for adding an API call for attach. So my idea
would be to first apply this patch which fixes the immediate problem
(but doesn't improve anything) and then refactor lxc-attach in such a
way that it may be called via the API. (And the patch I just posted
could still be backported to 0.9.1, where refactoring would be wrong.)

The basic idea for the refactored attach API would be:

  - pid_t lxc_attach(const char* name, const char* lxcpath, options...)
        + create a socket pair for sync
        + fork() (get rid of threading, nss stuff, etc.)
        |- same process:
        |      + waits for signal from child
        |      + [optional] puts grandchild in cgroup
        |      + closes synchronisation sockets
        |      + reap child process
        |      + returns pid of grandchild
        |- child:
               + setns()
               + clone() with CLONE_PARENT
               | (child will be child of parent, this process
               |  may exit and the parent won't care)
               |- same process
               |      + signal parent pid of grandchild
               |      + exit
               |- grandchild process
                      + [optional] drop privileges
                      + [optional] apparmor stuff
                      + set environment
                      + execute stuff

  - options paremeter:
        + pointer to some struct that contains all the options that
          may be specified to the current lxc-attach process
        + also contains function pointer to the function that is to be
          executed inside the container
           - lxc_attach_exec
                  default implementation that executes a specific
                  command
           - lxc_attach_shell
                  default implementation that executes a shell
           - this would allow for example the following python code
             using the API (if one doesn't want to exec() a command
             or a shell):
                  def test_file_exists():
                      sys.exit(os.path.exists('/foo/bar') and 0 or 1)
                  proc = container.attach(run=test_file_exists)

lxc-attach's main would then trivially be:

   1. parse parameters, fill the options struct
   2. call lxc_attach()
   3. waitpid() on attached process
   4. pass through exit code

Comments welcome, if you think this is the right way to go, then I'll go
ahead and implement it in the next few days.

Regards,
Christian

[1] https://wiki.ubuntu.com/LXC/1.0-roadmap