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

Proposed by James Hunt
Status: Merged
Merged at revision: 1397
Proposed branch: lp:~jamesodhunt/upstart/bug-1083723-reprise
Merge into: lp:~upstart-devel/upstart/1.6
Diff against target: 62 lines (+22/-3)
3 files modified
ChangeLog (+5/-0)
dbus/com.ubuntu.Upstart.xml (+3/-1)
util/telinit.c (+14/-2)
To merge this branch: bzr merge lp:~jamesodhunt/upstart/bug-1083723-reprise
Reviewer Review Type Date Requested Status
Stéphane Graber (community) Approve
Colin Watson (community) Approve
Upstart Developers Pending
Review via email: mp+138780@code.launchpad.net

Description of the change

* 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.

To post a comment you must log in.
1399. By James Hunt

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

Revision history for this message
Colin Watson (cjwatson) wrote :

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

review: Approve
Revision history for this message
Stéphane Graber (stgraber) 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.

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
Revision history for this message
Steve Langasek (vorlon) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'ChangeLog'
--- ChangeLog 2012-12-07 09:44:58 +0000
+++ ChangeLog 2012-12-07 17:12:22 +0000
@@ -3,6 +3,11 @@
3 * init/Makefile.am:3 * init/Makefile.am:
4 - TEST_DATA_DIR: use $srcdir, not $PWD.4 - TEST_DATA_DIR: use $srcdir, not $PWD.
5 - TEST_DATA_FILES: Corrected filename.5 - TEST_DATA_FILES: Corrected filename.
6 * dbus/com.ubuntu.Upstart.xml: Restart: Add annotation to make it
7 manifest this is an async call.
8 * util/telinit.c: restart_upstart(): Use the async call to avoid the
9 client-side complaining if it detects that Upstart has severed all
10 D-Bus connections in preparation for the re-exec.
611
72012-12-06 James Hunt <james.hunt@ubuntu.com>122012-12-06 James Hunt <james.hunt@ubuntu.com>
813
914
=== 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 @@
38 <arg name="state" type="s" direction="out" />38 <arg name="state" type="s" direction="out" />
39 </method>39 </method>
4040
41 <method name="Restart"/>41 <method name="Restart">
42 <annotation name="com.netsplit.Nih.Method.Async" value="true" />
43 </method>
4244
43 <!-- Signals for changes to the job list -->45 <!-- Signals for changes to the job list -->
44 <signal name="JobAdded">46 <signal name="JobAdded">
4547
=== 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 @@
161restart_upstart (void)161restart_upstart (void)
162{162{
163 nih_local NihDBusProxy *upstart = NULL;163 nih_local NihDBusProxy *upstart = NULL;
164 DBusPendingCall *ret;
164165
165 upstart = upstart_open (NULL);166 upstart = upstart_open (NULL);
166 if (! upstart)167 if (! upstart)
167 return -1;168 return -1;
168169
169 if (upstart_restart_sync (NULL, upstart) < 0)170 /* Fire and forget:
170 return -1;171 *
172 * Ask Upstart to restart itself using the async interface to
173 * avoid the client-side complaining if and when it detects that
174 * Upstart has severed all connections to perform the re-exec.
175 */
176 ret = upstart_restart (upstart, NULL, NULL, NULL, 0);
177
178 /* We don't care about the return code, but we have to keep
179 * the compiler happy.
180 */
181 if (ret != (DBusPendingCall *)TRUE)
182 return 0;
171183
172 return 0;184 return 0;
173}185}

Subscribers

People subscribed via source and target branches

to all changes: