Merge lp:~jamesodhunt/upstart/bug-1083723-reprise into lp:~upstart-devel/upstart/1.6

[James Hunt]

* dbus/com.ubuntu.Upstart.xml: Restart: Add annotation to make it
  manifest this is an async call.
* util/telinit.c: restart_upstart(): Use the async call to avoid the
  client-side complaining if it detects that Upstart has severed all
  D-Bus connections in preparation for the re-exec.

[lp:~jamesodhunt/upstart/bug-1083723-reprise]

1399. By James Hunt on 2012-12-07

* Resync with lp:~upstart-devel/upstart/1.6.

[Colin Watson (cjwatson)]

I haven't checked all the generated function types, but this looks broadly reasonable to me.

review: Approve

[Stéphane Graber (stgraber)]

For now, it seems like it's the only way to workaround that problem.
Obviously this means that telinit won't know whether the restart was successful or not.

I'm currently finishing the tests of my dbus-events branch which adds a new Restarted event emitted right after upstart did a successful re-exec.
In the future, we'll be able to have telinit listen on the system bus (if present) and catch that Restarted signal to confirm that upstart re-exec properly.

review: Approve

[Steve Langasek (vorlon)]

On Fri, Dec 07, 2012 at 05:21:22PM -0000, Stéphane Graber wrote:
> For now, it seems like it's the only way to workaround that problem.
> Obviously this means that telinit won't know whether the restart was
> successful or not.

This is not a major problem. The previous interface used a signal, which
doesn't tell you whether the restart was successful or not either.

=== modified file 'ChangeLog'
--- ChangeLog	2012-12-07 09:44:58 +0000
+++ ChangeLog	2012-12-07 17:12:22 +0000
@@ -3,6 +3,11 @@
 	* init/Makefile.am:
 	  - TEST_DATA_DIR: use $srcdir, not $PWD.
 	  - TEST_DATA_FILES: Corrected filename.
+	* dbus/com.ubuntu.Upstart.xml: Restart: Add annotation to make it
+	  manifest this is an async call.
+	* util/telinit.c: restart_upstart(): Use the async call to avoid the
+	  client-side complaining if it detects that Upstart has severed all
+	  D-Bus connections in preparation for the re-exec.
 
 2012-12-06  James Hunt  <[e-mail]>
 

=== modified file 'dbus/com.ubuntu.Upstart.xml'
--- dbus/com.ubuntu.Upstart.xml	2012-12-06 17:27:08 +0000
+++ dbus/com.ubuntu.Upstart.xml	2012-12-07 17:12:22 +0000
@@ -38,7 +38,9 @@
       <arg name="state" type="s" direction="out" />
     </method>
 
-    <method name="Restart"/>
+    <method name="Restart">
+      <annotation name="com.netsplit.Nih.Method.Async" value="true" />
+    </method>
 
     <!-- Signals for changes to the job list -->
     <signal name="JobAdded">

=== modified file 'util/telinit.c'
--- util/telinit.c	2012-12-06 11:38:02 +0000
+++ util/telinit.c	2012-12-07 17:12:22 +0000
@@ -161,13 +161,25 @@
 restart_upstart (void)
 {
 	nih_local NihDBusProxy *upstart = NULL;
+	DBusPendingCall        *ret;
 
 	upstart = upstart_open (NULL);
 	if (! upstart)
 		return -1;
 
-	if (upstart_restart_sync (NULL, upstart) < 0)
-		return -1;
+	/* Fire and forget:
+	 *
+	 * Ask Upstart to restart itself using the async interface to
+	 * avoid the client-side complaining if and when it detects that
+	 * Upstart has severed all connections to perform the re-exec.
+	 */
+	ret = upstart_restart (upstart, NULL, NULL, NULL, 0);
+
+	/* We don't care about the return code, but we have to keep
+	 * the compiler happy.
+	 */
+	if (ret != (DBusPendingCall *)TRUE)
+		return 0;
 
 	return 0;
 }