Re: [PATCH] setsid() exception in xend

[Horms <horms@verge.net.au>]

Sorry if this is a duplicate, I am sure I sent it but
it doesn't seem to have shown up on the list.

On Mon, Nov 28, 2005 at 03:57:01PM +0000, Ewan Mellor wrote:
> On Mon, Nov 28, 2005 at 10:58:09PM +0900, Horms wrote:
> 
> > [Snip!]
> >
> > Sure, I can eliminate the intermediate file descriptor,
> > I'll send a fresh patch tomorrow. 
> 
> That's great, thank you.
> 
> > On a related issue, can you clarify what the race is that it
> > is there to avoid? It seems cumbersome as you point out.
> 
> It means that the xend wrapper, i.e. tools/misc/xend aka /usr/sbin/xend does
> not exit until the daemon is ready to receive commands.  If you were using the
> init.d scripts to start Xend and then start some guest domains, you would have
> a race in that case between daemon startup and the subsequent script that
> would be trying to execute commands at the same time.

Thanks for the clarification.

Below is a patch that removes the unecessary fd handling in
the intermediate parent that was present in my previous patch.
It just calls self.exit(). I did consider doing some fd closing,
but exit() shold be sufficient. I tested this patch and it seems
to work just fine.


# HG changeset patch
# User Horms <horms@verge.net.au>
# Node ID 4c45fd3fd1e0916a34b614c95ca28b7d44cd8e35
# Parent  ddd958718cde22f20371a58359e173fd21c5da2e
[xend] Detach from terminal

* For setsid to effectivley detach a process from the terminal,
  the following needs to occur:

    fork ()   Return control to the shell
    setsid () New session with no controlling terminal
    fork ()   The session leader (parent) exits and thus
              the resulting child process can never regain the terminal

  This patch adds the second fork
  
* The call to self.daemonize(), which now forks, is moved to
  run before self.tracing(), as not that it actually disconnects
  from the terminal, and thus the prevailing process, the trace 
  looses the processes created in self.run().

diff -r ddd958718cde -r 4c45fd3fd1e0 tools/python/xen/xend/server/SrvDaemon.py
--- a/tools/python/xen/xend/server/SrvDaemon.py	Mon Nov 28 12:35:11 2005
+++ b/tools/python/xen/xend/server/SrvDaemon.py	Tue Nov 29 01:48:14 2005
@@ -123,9 +123,18 @@
 
     def daemonize(self):
         if not XEND_DAEMONIZE: return
+ 
         # Detach from TTY.
+
+        # Become the group leader (already a child process)
         os.setsid()
 
+        # Fork, this allows the group leader to exit,
+        # which means the child can never again regain control of the
+        # terminal
+        if self.fork_pid(XEND_PID_FILE):
+            self.exit()
+ 
         # Detach from standard file descriptors, and redirect them to
         # /dev/null or the log as appropriate.
         os.close(0)
@@ -164,7 +173,7 @@
         # we can avoid a race condition during startup
         
         r,w = os.pipe()
-        if self.fork_pid(XEND_PID_FILE):
+        if os.fork():
             os.close(w)
             r = os.fdopen(r, 'r')
             try:
@@ -178,6 +187,7 @@
         else:
             os.close(r)
             # Child
+            self.daemonize()
             self.tracing(trace)
             self.run(os.fdopen(w, 'w'))
 
@@ -274,7 +284,6 @@
 
             relocate.listenRelocation()
             servers = SrvServer.create()
-            self.daemonize()
             servers.start(status)
         except Exception, ex:
             print >>sys.stderr, 'Exception starting xend:', ex