rkt run: acquire lock on container directory and record pid

[vcaputo]

stage0 acquires an exclusive advisory lock on the container directory,
leaving the fd open when executing stage1/init with the fd value stored in
the environment variable RKT_LOCK_FD.

stage1's fakesdboot.so shim prevents closure of RKT_LOCK_FD in nspawn by
replacing the close with a O_CLOEXEC fcntl, retaining the lock handle until
nspawn exits.

fakesdboot.so has also been modified to intercept the clone syscall from
nspawn, recording the pid of the container's PID 1 in
"/var/lib/rkt/containers/$cuuid/pid"

With these changes one can do things like:

query container status:

 shopt -s nullglob
 for c in /var/lib/rkt/containers/*; do
  [ -e "$c/pid" ] || continue
  flock --exclusive --nonblock "$c" /bin/true || {
   cpid=$(cat "$c/pid")
   nsenter --mount --uts --ipc --pid --root --wd --target "${cpid}" systemctl status
  }
 done

gc dead containers:

 shopt -s nullglob
 for c in /var/lib/rkt/containers/*; do
  [ -e "$c/stage1" ] || continue
  flock --exclusive --nonblock "$c" rm -Rf "$c"
 done

[jzelinskie]

@vcaputo in the future, you can format code snippets on GitHub by doing

``lang
code goes here
``

but with 3 ticks instead of 2.

[vcaputo]

markdown in the commit message?

[jzelinskie]

I don't know if it'll render in the commit message, but in the PR body it would help.
I guess it's better to have a nice commit message than a vague commit message and a nice PR in case the code ever leaves GitHub.

[vcaputo]

Well I had noticed the formatting from the commit message was lost in giithub, but didn't add markdown since I wasn't sure if those changes would be retained in the merged PR commit message if it were merged.

[jonboulle]

why return here? won't this stop other syscalls? (or is that the goal?)

[vcaputo]

It should probably be an assert actually. This is not intended to be a general syscall() wrapper, it's specific to the clone syscall number, and in the case of systemd-nspawn that's perfectly ok because it's the only way syscall() is used in nspawn.

[cmaster1086]

Ok listen I,do remember going to this site but I really don't have any
ideal what it is or what it's about or what y'all keep texting me or what
it means so please stop thinking me... and again I'm sorry I....
On Dec 8, 2014 11:49 PM, "Vito Caputo" notifications@github.com wrote:

In stage1/mkrootfs.sh:

+int close(int fd)
+{

• if(lock_fd != -1 && fd == lock_fd)

•   return fcntl(fd, F_SETFD, FD_CLOEXEC);
  
• return libc_close(fd);
  +}

+long syscall(long number, ...)
+{

• unsigned long clone_flags;
• va_list ap;
• long ret;

• if(number != __NR_clone)

It should probably be an assert actually. This is not intended to be a
general syscall() wrapper, it's specific to the clone syscall number, and
in the case of systemd-nspawn that's perfectly ok.

—
Reply to this email directly or view it on GitHub
https://github.com/coreos/rocket/pull/244/files#r21507003.

[jonboulle]

looks awesome and/or scary!

[jonboulle]

Nit, could you separate the lock + the pid recording into separate commits? Seems like it might make it a bit easier to trace back should the need arise one day.

[philips]

This scares me too much. I was OK relying on the LD_PRELOAD hack until we can fix-up nspawn but this is going a bit too far. What about if we send a "life cycle" FD to systemd-nspawn that is socket activated to some unix domain inside the stage1 instead?

[vcaputo]

@philips that may be workable as well, though I don't think this is really all that scary, and what I really wanted to solidify with this was things like: is there a pid flie? where do we put it? do we need to communicate the lock fdnum to stage1? through an environment variable?

It's not immediately clear to me how the socket-activated unit would discover PID1's "outside" PID, whereas it's trivially done @ clone() in the systemd-nspawn parent context. We need to write that down somewhere for our nsenter-like operations.

Ultimately I see this all vanishing in the not too distant future with a systemd-nspawn replacement, where all these fakesdboot.so intercepts become the replacement's normal implementation.

[vcaputo]

Merging this for now, we'll revisit this, but in the interim we can get all the lifecycle details sorted and built around advisory locks and the $cuuid/pid file.

[jonboulle]