[RFC] new initctl command

Colin Watson cjwatson at ubuntu.com
Tue Jun 21 00:19:54 UTC 2011


On Mon, Jun 20, 2011 at 08:15:47PM +0300, Surbhi Palande wrote:
> On 06/15/2011 07:47 PM, Scott James Remnant wrote:
> >  - the D-Bus command needs to issue a method return to allow the
> >initctl command to exit ;-)
> 
> Ok! I have uploaded a new diff with this change. Since we need to
> return (along with sending a reply) before doing an execvp, we need
> two threads of execution. I have used fork to achieve this.
> 
> Before returning to initctl, the child process changes the default
> SIGTERM handler to a new one which on invocation calls exit(). Then
> the child process returns to the initctl command whereas the parent
> process waits for this child process to exit. When initctl gets a
> successful return, initctl sends the child process a SIGTERM signal
> upon which the child process exits. The parent process continues to
> execvp the new requested init after it unblocks from the blocking
> waitpid().

Relying on signals for this is problematic.  The child process here is
essentially another init in the old filesystem namespace, and even
though it isn't pid 1 it might well carry on to try to start other jobs
and generally cause havoc if the signal isn't delivered in a timely
fashion.  Remember that the kernel is entirely entitled to schedule
things as follows:

  initctl             init parent            init child
     |
     |   method call
     |-------------------->|       fork
     |                     |--------------------->|
     |                     |                      |
     |                     | (waitpid)            |
     |                     |                      |
     |                     |  method return       |
     |<-------------------------------------------|
     |                     |                      |
     |                     |                      | (continue as init)
     |                     |                      |
     (... twiddle thumbs for some time ...)
     |                     |                      |
     |   SIGTERM           |                      |
     |------------------------------------------->| (dies)
     |                     |
     |                     | (execvp)

As far as I can tell, it simply isn't safe to rely on sending a signal
from initctl for this; the ordering can't be made correct.  It would
work if you could pause the child while waiting for a signal, but if you
could do that easily then you could simply exit from the child.

On IRC a few days ago, I suggested using a D-Bus signal (not to be
confused with Unix signals, of course), so that initctl doesn't have to
wait for a reply from init and you don't need this awkward pair of
processes.  You said today that that had proved to be difficult because
there was no implementation that let you send a signal to upstart from
initctl.  Could you explain in more detail why this didn't work out?
If you declare it as <signal> in com.ubuntu.Upstart.xml, nih-dbus-tool
should give you a function called something like
upstart_emit_pivot_to_new_rootfs; style in other parts of Upstart seems
to be to wrap calls to these *_emit_* functions in NIH_ZERO, but that
should be about it:

  NIH_ZERO (upstart_emit_pivot_to_new_rootfs (NULL, upstart, args[0],
                                              args[1]));

Wouldn't that be easier?  It seems a lot simpler than any scheme
involving concurrency.

(As I said on IRC: using a D-Bus signal sacrifices the ability to tell
whether the pivot succeeded or failed.  My feeling is that implementing
that correctly in this case would be pretty tortuous, probably involving
inserting some code near the start of main and having Upstart know
whether it was started via pivot or not so that it could send the
success reply, and that the benefit is pretty marginal since all you'll
probably be able to do anyway in response to a failure is crash.)

Regards,

-- 
Colin Watson                                       [cjwatson at ubuntu.com]



More information about the upstart-devel mailing list