Merge lp:~jamesodhunt/upstart/bug-1079715 into lp:upstart

[James Hunt]

* init/job_class.c:
  - job_class_consider(): Removed redundant braces.
  - job_class_reconsider(): Removed redundant braces.
  - job_class_add_safe(): Consider session before asserting
    (LP: #1079715).
  - job_class_serialise():
    - Explicitly disallow user and chroot sessions
      from being serialised since this scenario is not supported
      (due to our not serialising ConfSource objects yet).
  - job_class_deserialise():
    - Check session as early as possible.
    - Assert that we do not have user and chroot sessions to deal with.
    - Fix potential invalid free if error occurs before JobClass
      is created.
* init/session.h: Comment.
* init/state.c:
  - state_deserialise_resolve_deps(): Specify new session parameter to
    state_get_job().
  - state_serialise_blocked(): Encode session index for BLOCKED_JOB.
  - state_deserialise_blocked(): Extract session from index index for
    BLOCKED_JOB to pass to state_get_job().
  - state_get_job(): Add @session parameter to allow exact job match.

[Steve Langasek (vorlon)]

@@ -378,6 +374,10 @@

  existing = (JobClass *)nih_hash_search (job_classes, class->name, NULL);

+ /* Ensure no existing class exists for the same session */
+ while (existing && existing->session != class->session)
+ existing = (JobClass *)nih_hash_search (job_classes, class->name, &existing->entry);
+
  nih_assert (! existing);

  job_class_add (class);

I suggest the following instead:

@@ -372,11 +372,10 @@

  control_init ();

- existing = (JobClass *)nih_hash_search (job_classes, class->name, NULL);
-
  /* Ensure no existing class exists for the same session */
- while (existing && existing->session != class->session)
- existing = (JobClass *)nih_hash_search (job_classes, class->name, &existing->entry);
+ do {
+ existing = (JobClass *)nih_hash_search (job_classes, class->name, existing ? &existing->entry : NULL);
+ } while (existing && existing->session != class->session);

  nih_assert (! existing);

    - Fix potential invalid free if error occurs before JobClass
      is created.

class is initialized to NULL at the top of the function, so this seems to be no-op churn.

        if (session_index < 0)
- goto error;
+ goto out;
+
+ session = session_from_index (session_index);
+
+ /* XXX: user and chroot jobs are not currently supported
+ * due to ConfSources not currently being serialised.
+ */
+ nih_assert (session == NULL);

This is a warning on serialization and you're making it a fatal error on deserialization. Please fall back to skipping deserialization of user and chroot jobs (if found) instead of breaking init in this case.

@@ -1228,6 +1232,20 @@
                                                blocked->job->class->name))
                                goto error;

+ session = blocked->job->class->session;
+
+ session_index = session_get_index (session);

Can be written more succinctly as:

+ session_index = session_get_index (blocked->job->class->session);

@@ -1430,7 +1450,15 @@
                                                "class", NULL, job_class_name))
                                goto error;

- job = state_get_job (job_class_name, job_name);
+ if (! state_get_json_int_var (json_blocked_data, "session", session_index))
+ goto error;
+
+ if (session_index < 0)
+ goto error;
+

This is not part of the serialization format for upstart 1.6, so the absence of this field must not be considered an error.

This also underscores the need for test cases that embed serialized json data as generated by different releases of upstart, to test deserialization capabilities when faced with historical data.

@@ -1643,7 +1674,13 @@
        nih_assert (job_class);
        nih_assert (job_classes);

- class = (JobClass *)nih_hash_lookup (job_classes, job_class);
+ class = (JobClass *)nih_hash_search (job_classes, job_class, NULL);
+ if (! class)
+ goto error;
+
+ while (class && class->session != session)
+ class = (JobClass *)...

review: Needs Fixing

[lp:~jamesodhunt/upstart/bug-1079715]

1390. By James Hunt on 2012-11-22

* init/job_class.c:
  - job_class_consider(): Removed redundant braces.
  - job_class_reconsider(): Removed redundant braces.
  - job_class_add_safe(): Consider session before asserting
    (LP: #1079715).
  - job_class_serialise():
    - Explicitly disallow user and chroot sessions
      from being serialised since this scenario is not supported
      (due to our not serialising ConfSource objects yet).
  - job_class_deserialise():
    - Check session as early as possible.
    - Removed NIH_MUST() to allow check on job_class_new().
    - Assert that we do not have user and chroot sessions to deal with.
    - Fix potential invalid free if error occurs before JobClass
      is created.
  - job_class_deserialise_all():
    - Explicitly ignore attempted deserialisation of user and chroot
      sessions.
    - Removed invalid comment.
* init/session.h: Comment.
* init/state.c:
  - state_deserialise_resolve_deps():
    - Ignore classes associated with a user or chroot session.
    - Specify new session parameter to state_get_job().
  - state_serialise_blocked(): Encode session index for BLOCKED_JOB.
  - state_deserialise_blocked(): Extract session from index index for
    BLOCKED_JOB to pass to state_get_job().
  - state_get_job(): Add @session parameter to allow exact job match.

[James Hunt (jamesodhunt)]

> @@ -378,6 +374,10 @@
>
> existing = (JobClass *)nih_hash_search (job_classes, class->name, NULL);
>
> + /* Ensure no existing class exists for the same session */
> + while (existing && existing->session != class->session)
> + existing = (JobClass *)nih_hash_search (job_classes, class->name, &existing->entry);
> +
> nih_assert (! existing);
>
> job_class_add (class);
>
> I suggest the following instead:
>
> @@ -372,11 +372,10 @@
>
> control_init ();
>
> - existing = (JobClass *)nih_hash_search (job_classes, class->name, NULL);
> -
> /* Ensure no existing class exists for the same session */
> - while (existing && existing->session != class->session)
> - existing = (JobClass *)nih_hash_search (job_classes, class->name, &existing->entry);
> + do {
> + existing = (JobClass *)nih_hash_search (job_classes, class->name, existing ? &existing->entry : NULL);
> + } while (existing && existing->session != class->session);
Agreed - this is cleaner: fixed.

>
> nih_assert (! existing);
>
> - Fix potential invalid free if error occurs before JobClass
> is created.
>
> class is initialized to NULL at the top of the function, so this seems to be no-op churn.
Actually, no - nih_free() has different semantics to free(3): you cannot
legitimately pass NULL.

>
> if (session_index < 0)
> - goto error;
> + goto out;
> +
> + session = session_from_index (session_index);
> +
> + /* XXX: user and chroot jobs are not currently supported
> + * due to ConfSources not currently being serialised.
> + */
> + nih_assert (session == NULL);
>
> This is a warning on serialization and you're making it a fatal error on deserialization. Please fall back to skipping deserialization of user and chroot jobs (if found) instead of breaking init in this case.
Done.

>
> @@ -1228,6 +1232,20 @@
> blocked->job->class->name))
> goto error;
>
> + session = blocked->job->class->session;
> +
> + session_index = session_get_index (session);
>
> Can be written more succinctly as:
>
> + session_index = session_get_index (blocked->job->class->session);
Of course. I wrote it that way to make it clearer and to avoid
particularly long lines. However, since we're not really adhering to any
line-length policies these days, I've changed it as suggested.

>
> @@ -1430,7 +1450,15 @@
> "class", NULL, job_class_name))
> goto error;
>
> - job = state_get_job (job_class_name, job_name);
> + if (! state_get_json_int_var (json_blocked_data, "session", session_index))
> + goto error;
> +
> + if (session_index < 0)
> + goto error;
> +
>
> This is not part of the serialization format for upstart 1.6, so the absence of this field must not be considered an error.
I've retained this as an error, but changed the logic in
state_deserialise_blocking() to ignore failures from
state_deserialise_blocked(). This is better than pretending the job has
a NULL session and then waiting for state_get_job() to error (maybe) and
has parity with the way we handle JobClasses.

>
> This also underscores the need for tes...

[Steve Langasek (vorlon)]

On Thu, Nov 22, 2012 at 04:36:19PM -0000, James Hunt wrote:

> Agreed - this is cleaner: fixed.

It looks like you fixed this via a push --overwrite of the branch. The
merge proposal machinery is designed with the assumption that you will
instead push new commits to the branch on top of what is already there.
Could you please do that, in the future?

> > nih_assert (! existing);
> >
> > - Fix potential invalid free if error occurs before JobClass
> > is created.
> >
> > class is initialized to NULL at the top of the function, so this seems
> > to be no-op churn.
> Actually, no - nih_free() has different semantics to free(3): you cannot
> legitimately pass NULL.

Ah, I was unaware. Thanks for setting me straight. :)

> >
> > @@ -1228,6 +1232,20 @@
> > blocked->job->class->name))
> > goto error;
> >
> > + session = blocked->job->class->session;
> > +
> > + session_index = session_get_index (session);
> >
> > Can be written more succinctly as:
> >
> > + session_index = session_get_index (blocked->job->class->session);
> Of course. I wrote it that way to make it clearer and to avoid
> particularly long lines. However, since we're not really adhering to any
> line-length policies these days, I've changed it as suggested.

Well, I think we should avoid overly-long lines, I just don't think creating
single-use temp variables is a good solution for oversized lines. It would
IMHO also be fine to wrap this as:

   session_index = session_get_index
                         (blocked->job->class->session);

> > @@ -1430,7 +1450,15 @@
> > "class", NULL, job_class_name))
> > goto error;
> >
> > - job = state_get_job (job_class_name, job_name);
> > + if (! state_get_json_int_var (json_blocked_data, "session", session_index))
> > + goto error;
> > +
> > + if (session_index < 0)
> > + goto error;
> > +
> >
> > This is not part of the serialization format for upstart 1.6, so the
> > absence of this field must not be considered an error.

> I've retained this as an error, but changed the logic in
> state_deserialise_blocking() to ignore failures from
> state_deserialise_blocked(). This is better than pretending the job has
> a NULL session and then waiting for state_get_job() to error (maybe) and
> has parity with the way we handle JobClasses.

AIUI this means all information about blocked jobs will still be lost on
upgrade from 1.6. I don't think that's the right trade-off; I think it's
better to assume that a missing session field means the default session
(which it will, for the vast majority of users) and dump the "blocked" state
non-fatally only if we don't find a matching job on the default session.

> > This also underscores the need for test cases that embed serialized json data as generated by different releases of upstart, to test deserialization capabilities when faced with historical data.
> Quite - there is no such thing as too much testing ;D

There definitely is such a thing as too much testing. But when it comes to
changes to the serialization format, we definitely...

[lp:~jamesodhunt/upstart/bug-1079715]

1391. By James Hunt on 2012-11-28

* init/state.c: state_get_job(): Initialised class to ensure
  defined behaviour.

1392. By James Hunt on 2012-11-28

* init/state.c: state_deserialise_blocked(): Set session to NULL to
  handle Upstart-1.6 serialisation.
* init/tests/test_job_process.c: child(): Remove unused variables in and
  buffer.
* init/tests/test_state.c: test_blocking(): Additional tests to check
  that it is possible to deserialise Upstart 1.6 JSON format (which does
  not include the "session" JSON attribute for blocked objects. New
  tests:
  - "BLOCKED_JOB with no JSON session object".
  - "BLOCKED_JOB with JSON session object".

1393. By James Hunt on 2012-11-29

* init/tests/test_state.c: test_blocked():
  - Improved comments.
  - New test: "ensure BLOCKED_JOB with non-NULL session is ignored".

1394. By James Hunt on 2012-12-04

* dbus/com.ubuntu.Upstart.xml: Added 'GetState' method that returns
  internal state in JSON format.
* init/Makefile.am:
  - Added TEST_DATA_DIR to allow tests to find data files.
  - Added test data files to distribution.
* init/control.c: control_get_state(): Implementation for D-Bus
  'GetState' method.
* init/control.h: Prototype for control_get_state().
* init/state.c:
  - state_serialise_blocked: Remove static to allow tests to access them.
  - state_deserialise_blocked: Remove static to allow tests to access them.
  - state_read_objects(): Attempt to write the state to file
    STATE_FILE if deserialisation fails as an aid
    to diagnosing the cause of the failure.
* init/state.h: Define STATE_FILE ('/var/log/upstart/upstart.state').
* init/tests/test_state.c:
  - TestDataFile: New type to represent data files.
  - test_data_files: Array of data files to test.
  - test_blocking():
    - New tests:
      - "BLOCKED_JOB serialisation and deserialisation".
      - "BLOCKED_EVENT serialisation and deserialisation".
    - Removed test now handled by test_upstart1_6_upgrade():
      "BLOCKED_JOB iwth no JSON session object".
  - test_upgrade(): Iterate test_data_files, processing data files.
  - test_upstart1_6_upgrade(): Test for handling Upstart 1.6
    serialisation format.
  - main(): Call test_upgrade().
* init/tests/data/upstart-1.6.json: Test data used by test_state.c for
  upgrade testing.

1395. By James Hunt on 2012-12-06

* init/control.c: control_get_state():
  - Simplified session check.
  - Changed error message to refer to 'state', not 'restart'.
  - Don't call control_bus_flush() since it's not technically required
    in this non-re-exec scenario.
* init/state.c: state_write_file():
  - Added comment.
  - Check write for EINTR, not EAGAIN/EWOULDBLOCK.

=== modified file 'dbus/com.ubuntu.Upstart.xml'
--- dbus/com.ubuntu.Upstart.xml	2012-09-09 21:22:06 +0000
+++ dbus/com.ubuntu.Upstart.xml	2012-12-06 09:42:23 +0000
@@ -34,6 +34,10 @@
       <arg name="jobs" type="ao" direction="out" />
     </method>
 
+    <method name="GetState">
+      <arg name="state" type="s" direction="out" />
+    </method>
+
     <!-- Signals for changes to the job list -->
     <signal name="JobAdded">
       <arg name="job" type="o" />
 
=== modified file 'init/Makefile.am'
--- init/Makefile.am	2012-11-18 05:57:58 +0000
+++ init/Makefile.am	2012-12-06 09:42:23 +0000
@@ -126,8 +126,15 @@
 	$(com_ubuntu_Upstart_Job_OUTPUTS) \
 	$(com_ubuntu_Upstart_Instance_OUTPUTS)
 
-
-EXTRA_DIST = init.supp
+TEST_DATA_DIR = $(PWD)/tests/data
+
+AM_CPPFLAGS += -DTEST_DATA_DIR="\"$(TEST_DATA_DIR)\""
+
+TEST_DATA_FILES = \
+	$(TEST_DATA_DIR)/upstart-1.6-blocked.json
+
+EXTRA_DIST = init.supp $(TEST_DATA_FILES)
+	
 
 test_util_SOURCES = \
 	tests/test_util.c tests/test_util.h
 
=== modified file 'init/control.c'
--- init/control.c	2012-09-20 15:16:52 +0000
+++ init/control.c	2012-12-06 09:42:23 +0000
@@ -949,3 +949,65 @@
 
 	return 0;
 }
+
+/**
+ * control_get_state:
+ *
+ * @data: not used,
+ * @message: D-Bus connection and message received,
+ * @state: output string returned to client.
+ *
+ * Convert internal state to JSON string.
+ *
+ * Returns: zero on success, negative value on raised error.
+ **/
+int
+control_get_state (void           *data,
+		   NihDBusMessage  *message,
+		   char           **state)
+{
+	Session  *session;
+	uid_t     uid;
+	size_t    len;
+
+	nih_assert (message);
+	nih_assert (state);
+
+	uid = getuid ();
+
+	/* Get the relevant session */
+	session = session_from_dbus (NULL, message);
+
+	/* We don't want chroot sessions snooping outside their domain.
+	 *
+	 * Ideally, we'd allow them to query their own session, but the
+	 * current implementation doesn't lend itself to that.
+	 */
+	if (session && session->chroot) {
+		nih_warn (_("Ignoring state query from chroot session"));
+		return 0;
+	}
+
+	/* Disallow users from obtaining state details, unless they
+	 * happen to own this process (which they may do in the test
+	 * scenario and when running Upstart as a non-privileged user).
+	 */
+	if (session && session->user != uid) {
+		nih_dbus_error_raise_printf (
+			DBUS_INTERFACE_UPSTART ".Error.PermissionDenied",
+			_("You do not have permission to request state"));
+		return -1;
+	}
+
+	if (state_to_string (state, &len) < 0)
+		goto error;
+
+	nih_ref (*state, message);
+
+	return 0;
+
+error:
+	nih_dbus_error_raise_printf (DBUS_ERROR_NO_MEMORY,
+			_("Out of Memory"));
+	return -1;
+}
 
=== modified file 'init/control.h'
--- init/control.h	2012-09-11 14:59:40 +0000
+++ init/control.h	2012-12-06 09:42:23 +0000
@@ -105,6 +105,11 @@
 int control_bus_release_name (void)
 	__attribute__ ((warn_unused_result));
 
+int control_get_state (void           *data,
+		   NihDBusMessage  *message,
+		   char           **state)
+	__attribute__ ((warn_unused_result));
+
 NIH_END_EXTERN
 
 #endif /* INIT_CONTROL_H */
 
=== modified file 'init/job_class.c'
--- init/job_class.c	2012-11-14 14:47:19 +0000
+++ init/job_class.c	2012-12-06 09:42:23 +0000
@@ -271,9 +271,7 @@
 
 	/* If we found an entry, ensure we only consider the appropriate session */
 	while (registered && registered->session != class->session)
-	{
 		registered = (JobClass *)nih_hash_search (job_classes, class->name, &registered->entry);
-	}
 
 	if (registered != best) {
 		if (registered)
@@ -314,9 +312,7 @@
 
 	/* If we found an entry, ensure we only consider the appropriate session */
 	while (registered && registered->session != class->session)
-	{
 		registered = (JobClass *)nih_hash_search (job_classes, class->name, &registered->entry);
-	}
 
 	if (registered == class) {
 		if (class != best) {
@@ -364,7 +360,7 @@
  * @class: new class to select.
  *
  * Adds @class to the hash table iff no existing entry of the
- * same name exists.
+ * same name exists for the same session.
  **/
 void
 job_class_add_safe (JobClass *class)
@@ -376,7 +372,11 @@
 
 	control_init ();
 
-	existing = (JobClass *)nih_hash_search (job_classes, class->name, NULL);
+	/* Ensure no existing class exists for the same session */
+	do {
+		existing = (JobClass *)nih_hash_search (job_classes,
+				class->name, existing ? &existing->entry : NULL);
+	} while (existing && existing->session != class->session);
 
 	nih_assert (! existing);
 
@@ -1592,6 +1592,15 @@
 	json = json_object_new_object ();
 	if (! json)
 		return NULL;
+	
+	/* XXX: user and chroot jobs are not currently supported
+	 * due to ConfSources not currently being serialised.
+	 */
+	if (class->session) {
+		nih_info ("WARNING: serialisation of user jobs and "
+			"chroot sessions not currently supported");
+		goto error;
+	}
 
 	session_index = session_get_index (class->session);
 	if (session_index < 0)
@@ -1797,6 +1806,7 @@
 {
 	json_object    *json_normalexit;
 	JobClass       *class = NULL;
+	Session        *session;
 	int             session_index = -1;
 	int             ret;
 	nih_local char *name = NULL;
@@ -1814,21 +1824,24 @@
 	if (session_index < 0)
 		goto error;
 
+	session = session_from_index (session_index);
+
+	/* XXX: user and chroot jobs are not currently supported
+	 * due to ConfSources not currently being serialised.
+	 */
+	if (session) {
+		nih_info ("WARNING: deserialisation of user jobs and "
+			"chroot sessions not currently supported");
+		goto error;
+	}
+
 	if (! state_get_json_string_var_strict (json, "name", NULL, name))
 		goto error;
 
-	class = NIH_MUST (job_class_new (NULL, name,
-	                                 session_from_index (session_index)));
+	class = job_class_new (NULL, name, session);
 	if (! class)
 		goto error;
 
-	if (class->session != NULL) {
-		nih_warn ("XXX: WARNING (%s:%d): deserialisation of "
-				"user jobs and chroot sessions not currently supported",
-				__func__, __LINE__);
-		goto error;
-	}
-
 	/* job_class_new() sets path */
 	if (! state_get_json_string_var_strict (json, "path", NULL, path))
 		goto error;
@@ -2002,7 +2015,9 @@
 	return class;
 
 error:
-	nih_free (class);
+	if (class)
+		nih_free (class);
+
 	return NULL;
 }
 
@@ -2043,19 +2058,12 @@
 			goto error;
 
 		class = job_class_deserialise (json_class);
+
+		/* For parity with the serialisation code, don't treat
+		 * errors as fatal for the entire deserialisation.
+		 */
 		if (! class)
-			goto error;
-
-		/* FIXME:
-		 *
-		 * If user sessions exist (ie 'initctl --session list'
-		 * has been run), we get this failure:
-		 *
-		 *             serialised path='/com/ubuntu/Upstart/jobs/1000/bang'
-		 * path set by job_class_new()='/com/ubuntu/Upstart/jobs/_/1000/bang'
-		 *
-		 */
-
+			continue;
 	}
 
 	return 0;
 
=== modified file 'init/session.h'
--- init/session.h	2012-06-29 16:19:33 +0000
+++ init/session.h	2012-12-06 09:42:23 +0000
@@ -39,7 +39,8 @@
  * with a chroot path)).
  *
  * This structure is used to identify collections of jobs
- * that share either a common @chroot and/or common @user.
+ * that share either a common @chroot and/or common @user. Note that
+ * @conf_path is unique across all sessions.
  *
  * Summary of Session values for different environments:
  *
 
=== modified file 'init/state.c'
--- init/state.c	2012-11-23 11:36:47 +0000
+++ init/state.c	2012-12-06 09:42:23 +0000
@@ -2,7 +2,7 @@
  *
  * state.c - serialisation and deserialisation support.
  *
- * Copyright © 2012 Canonical Ltd.
+ * Copyright  2012 Canonical Ltd.
  * Author: James Hunt <[e-mail]>.
  *
  * This program is free software; you can redistribute it and/or modify
@@ -48,7 +48,8 @@
 json_object *json_events = NULL;
 json_object *json_classes = NULL;
 
-extern int use_session_bus;
+extern int   use_session_bus;
+extern char *log_dir;
 
 /**
  * args_copy:
@@ -68,22 +69,17 @@
 int restart = FALSE;
 
 /* Prototypes for static functions */
-static json_object *
-state_serialise_blocked (const Blocked *blocked)
-	__attribute__ ((malloc, warn_unused_result));
-
-static Blocked *
-state_deserialise_blocked (void *parent, json_object *json, NihList *list)
-	__attribute__ ((malloc, warn_unused_result));
-
 static JobClass *
 state_index_to_job_class (int job_class_index)
 	__attribute__ ((warn_unused_result));
 
 static Job *
-state_get_job (const char *job_class, const char *job_name)
+state_get_job (const Session *session, const char *job_class,
+	       const char *job_name)
 	__attribute__ ((warn_unused_result));
 
+static void state_write_file (NihIoBuffer *buffer);
+
 /**
  * state_read:
  *
@@ -246,10 +242,59 @@
 	return 0;
 
 error:
+	/* Failed to reconstruct internal state so attempt to write
+	 * the JSON state data to a file to allow for manual post
+	 * re-exec analysis.
+	 */
+	if (buffer->len && log_dir)
+		state_write_file (buffer);
+
 	return -1;
 }
 
 /**
+ * state_write_file:
+ *
+ * @buffer: NihIoBuffer containing JSON data.
+ *
+ * Write JSON data contained in @buffer to STATE_FILE below log_dir.
+ *
+ * Failures are ignored since this is designed to be called in an error
+ * scenario anyway.
+ **/
+void
+state_write_file (NihIoBuffer *buffer)
+{
+	int              fd;
+	ssize_t          bytes;
+	nih_local char  *state_file = NULL;
+
+	nih_assert (buffer);
+
+	state_file = nih_sprintf (NULL, "%s/%s", log_dir, STATE_FILE);
+	if (! state_file)
+		return;
+
+	/* Note the very restrictive permissions */
+	fd = open (state_file, (O_CREAT|O_WRONLY|O_TRUNC), S_IRUSR);
+	if (fd < 0)
+		return;
+
+	while (TRUE) {
+		bytes = write (fd, buffer->buf, buffer->len);
+
+		if (! bytes)
+			break;
+		else if (bytes > 0)
+			nih_io_buffer_shrink (buffer, (size_t)bytes);
+		else if (bytes < 0 && errno != EINTR)
+			break;
+	}
+
+	close (fd);
+}
+
+/**
  * state_write_objects:
  *
  * @fd: file descriptor to write serialisation data on,
@@ -1139,6 +1184,12 @@
 		if (! class)
 			goto error;
 
+		/* XXX: user and chroot jobs are not currently supported
+		 * due to ConfSources not currently being serialised.
+		 */
+		if (class->session)
+			continue;
+
 		if (! state_get_json_var_full (json_class, "jobs", array, json_jobs))
 			goto error;
 
@@ -1164,7 +1215,7 @@
 				goto error;
 
 			/* lookup job */
-			job = state_get_job (class->name, job_name);
+			job = state_get_job (class->session, class->name, job_name);
 			if (! job)
 				goto error;
 
@@ -1200,11 +1251,12 @@
  *
  * Returns: JSON-serialised Blocked object, or NULL on error.
  **/
-static json_object *
+json_object *
 state_serialise_blocked (const Blocked *blocked)
 {
 	json_object  *json;
 	json_object  *json_blocked_data;
+	int           session_index;
 
 	nih_assert (blocked);
 
@@ -1228,6 +1280,18 @@
 						blocked->job->class->name))
 				goto error;
 
+			session_index = session_get_index (blocked->job->class->session);
+			if (session_index < 0)
+				goto error;
+
+			/* Encode parent classes session index to aid in
+			 * finding the correct job on deserialisation.
+			 */
+			if (! state_set_json_int_var (json_blocked_data,
+						"session",
+						session_index))
+				goto error;
+
 			if (! state_set_json_string_var (json_blocked_data,
 						"name",
 						blocked->job->name))
@@ -1389,7 +1453,7 @@
  *
  * Returns: new Blocked object, or NULL on error.
  **/
-static Blocked *
+Blocked *
 state_deserialise_blocked (void *parent, json_object *json,
 		NihList *list)
 {
@@ -1397,7 +1461,7 @@
 	Blocked         *blocked = NULL;
 	nih_local char  *blocked_type_str = NULL;
 	BlockedType      blocked_type;
-	int             ret;
+	int              ret;
 
 	nih_assert (parent);
 	nih_assert (json);
@@ -1421,6 +1485,8 @@
 			nih_local char  *job_name = NULL;
 			nih_local char  *job_class_name = NULL;
 			Job             *job;
+			Session         *session;
+			int              session_index;
 
 			if (! state_get_json_string_var_strict (json_blocked_data,
 						"name", NULL, job_name))
@@ -1430,7 +1496,19 @@
 						"class", NULL, job_class_name))
 				goto error;
 
-			job = state_get_job (job_class_name, job_name);
+			/* On error, assume NULL session since the likelihood
+			 * is we're upgrading from Upstart 1.6 that did not set
+			 * the 'session' JSON object.
+			 */
+			if (! state_get_json_int_var (json_blocked_data, "session", session_index))
+				session_index = 0;
+
+			if (session_index < 0)
+				goto error;
+
+			session = session_from_index (session_index);
+
+			job = state_get_job (session, job_class_name, job_name);
 			if (! job)
 				goto error;
 
@@ -1583,8 +1661,14 @@
 		if (! json_blocked)
 			goto error;
 
+
+		/* Don't error in this scenario to allow for possibility
+		 * that version of Upstart that performed the
+		 * serialisation did not correctly handle user and
+		 * chroot jobs.
+		 */
 		if (! state_deserialise_blocked (parent, json_blocked, list))
-			goto error;
+			;
 	}
 
 	return 0;
@@ -1625,6 +1709,7 @@
 /**
  * state_get_job:
  *
+ * @session: session of job class,
  * @job_class: name of job class,
  * @job_name: name of job instance.
  *
@@ -1635,15 +1720,21 @@
  * job not found.
  **/
 static Job *
-state_get_job (const char *job_class, const char *job_name)
+state_get_job (const Session *session,
+	       const char *job_class,
+	       const char *job_name)
 {
-	JobClass  *class;
+	JobClass  *class = NULL;
 	Job       *job;
 
 	nih_assert (job_class);
 	nih_assert (job_classes);
 
-	class = (JobClass *)nih_hash_lookup (job_classes, job_class);
+	do {
+		class = (JobClass *)nih_hash_search (job_classes,
+				job_class, class ? &class->entry : NULL);
+	} while (class && class->session != session);
+
 	if (! class)
 		goto error;
 
 
=== modified file 'init/state.h'
--- init/state.h	2012-11-14 14:47:19 +0000
+++ init/state.h	2012-12-06 09:42:23 +0000
@@ -367,6 +367,18 @@
 #define STATE_WAIT_SECS_ENV "UPSTART_STATE_WAIT_SECS"
 
 /**
+ * STATE_FILE:
+ *
+ * Name of file that is written below the job log directory if the
+ * newly re-exec'ed init instance failed to understand the JSON sent to
+ * it by the old instance.
+ *
+ * This could happen for example if the old instance generated invalid
+ * JSON, or JSON in an unexected format.
+ **/
+#define STATE_FILE "upstart.state"
+
+/**
  * state_get_timeout:
  *
  * @var: name of long integer var to set to timeout value.
 
=== added directory 'init/tests/data'
=== added file 'init/tests/data/upstart-1.6.json'
--- init/tests/data/upstart-1.6.json	1970-01-01 00:00:00 +0000
+++ init/tests/data/upstart-1.6.json	2012-12-06 09:42:23 +0000
@@ -0,0 +1,1 @@
+{ "sessions": [ ], "events": [ { "session": 0, "name": "Christmas", "fd": -1, "progress": "EVENT_PENDING", "failed": 0, "blockers": 0, "blocking": [ { "data": { "class": "bar", "name": "" }, "type": "BLOCKED_JOB" } ] } ], "job_classes": [ { "session": 0, "name": "bar", "path": "\/com\/ubuntu\/Upstart\/jobs\/bar", "instance": "", "jobs": [ { "name": "", "path": "\/com\/ubuntu\/Upstart\/jobs\/bar\/_", "goal": "JOB_STOP", "state": "JOB_WAITING", "env": [ ], "start_env": [ ], "stop_env": [ ], "fds": [ ], "pid": [ 0, 0, 0, 0, 0 ], "blocker": 0, "kill_process": "PROCESS_INVALID", "failed": 0, "failed_process": "PROCESS_INVALID", "exit_status": 0, "respawn_time": 0, "respawn_count": 0, "trace_forks": 0, "trace_state": "TRACE_NONE", "log": [ { "path": null }, { "path": null }, { "path": null }, { "path": null }, { "path": null } ] } ], "description": null, "author": null, "version": null, "env": [ ], "export": [ ], "emits": [ ], "process": [ { "script": 0, "command": null }, { "script": 0, "command": null }, { "script": 0, "command": null }, { "script": 0, "command": null }, { "script": 0, "command": null } ], "expect": "EXPECT_NONE", "task": 0, "kill_timeout": 5, "kill_signal": 15, "respawn": 0, "respawn_limit": 10, "respawn_interval": 5, "normalexit": [ ], "console": "CONSOLE_LOG", "umask": 18, "nice": 0, "oom_score_adj": 0, "limits": [ { "rlim_cur": 0, "rlim_max": 0 }, { "rlim_cur": 0, "rlim_max": 0 }, { "rlim_cur": 0, "rlim_max": 0 }, { "rlim_cur": 0, "rlim_max": 0 }, { "rlim_cur": 0, "rlim_max": 0 }, { "rlim_cur": 0, "rlim_max": 0 }, { "rlim_cur": 0, "rlim_max": 0 }, { "rlim_cur": 0, "rlim_max": 0 }, { "rlim_cur": 0, "rlim_max": 0 }, { "rlim_cur": 0, "rlim_max": 0 }, { "rlim_cur": 0, "rlim_max": 0 }, { "rlim_cur": 0, "rlim_max": 0 }, { "rlim_cur": 0, "rlim_max": 0 }, { "rlim_cur": 0, "rlim_max": 0 }, { "rlim_cur": 0, "rlim_max": 0 }, { "rlim_cur": 0, "rlim_max": 0 } ], "chroot": null, "chdir": null, "setuid": null, "setgid": null, "deleted": 0, "debug": 0, "usage": null } ] }
 
=== modified file 'init/tests/test_job_process.c'
=== modified file 'init/tests/test_state.c'
--- init/tests/test_state.c	2012-11-14 14:47:19 +0000
+++ init/tests/test_state.c	2012-12-06 09:42:23 +0000
@@ -50,11 +50,27 @@
 #include "control.h"
 #include "test_util.h"
 
+#ifndef TEST_DATA_DIR
+#error ERROR: TEST_DATA_DIR not defined
+#endif
+
+/* These functions are 'protected'.
+ *
+ * The test code needs access, but they cannot be public due to
+ * header-file complications.
+ */
+json_object *
+state_serialise_blocked (const Blocked *blocked)
+	__attribute__ ((malloc, warn_unused_result));
+
+Blocked *
+state_deserialise_blocked (void *parent, json_object *json, NihList *list)
+	__attribute__ ((malloc, warn_unused_result));
 
 /**
  * AlreadySeen:
  *
- * Used to allow objects that directly or indirectly reference on
+ * Used to allow objects that directly or indirectly reference
  * another to be inspected and compared without causing infinite
  * recursion.
  *
@@ -127,6 +143,43 @@
 int blocked_diff (const Blocked *a, const Blocked *b, AlreadySeen seen)
 	__attribute__ ((warn_unused_result));
 
+void test_upstart1_6_upgrade (const char *conf_file, const char *path);
+
+/**
+ * TestDataFile:
+ *
+ * @conf_file: Name of ConfFile that must be created prior to
+ *  deserialising JSON data in @filename.
+ * @filename: basename of data file,
+ * @func: function to run to test @filename.
+ *
+ * Representation of a JSON data file used for ensuring that the current
+ * version of Upstart is able to deserialise all previous JSON data file
+ * format versions.
+ *
+ * @conf_file is required since we do not currently serialise ConfFile
+ * and ConfSource objects so these entities must be created immediately
+ * prior to attempting deserialisation.
+ *
+ * @func returns nothing so is expected to assert on any error.
+ **/
+typedef struct test_data_file {
+	char		*conf_file;
+	char		*filename;
+		  void (*func) (const char *conf_file, const char *path);
+} TestDataFile;
+
+/**
+ * test_data_files:
+ *
+ * Array of data files to test.
+ **/
+TestDataFile test_data_files[] = {
+	{ "bar", "upstart-1.6.json", test_upstart1_6_upgrade },
+
+	{ NULL, NULL, NULL }
+};
+
 /* Data with some embedded nulls */
 const    char test_data[] = {
 	'h', 'e', 'l', 'l', 'o', 0x0, 0x0, 0x0, ' ', 'w', 'o', 'r', 'l', 'd', '\n', '\r', '\0'
@@ -139,7 +192,7 @@
 int64_t  values64[] = {INT64_MIN, -1, 0, 1, INT64_MAX};
 const Process test_procs[] = {
 	{ 0, "echo hello" },
-	{ 1, "echo hello" },
+	{ 1, "echo hello" }
 };
 rlim_t rlimit_values[] = { 0, 1, 2, 3, 7, RLIM_INFINITY };
 
@@ -993,17 +1046,23 @@
 void
 test_blocking (void)
 {
-	nih_local char        *json_string = NULL;
-	ConfSource            *source = NULL;
-	ConfFile              *file;
-	JobClass              *class;
-	JobClass              *new_class;
-	Job                   *job;
-	Job                   *new_job;
-	Event                 *event;
-	Event                 *new_event;
-	Blocked               *blocked;
-	size_t                 len;
+	nih_local char          *json_string = NULL;
+	nih_local char          *parent_str = NULL;
+	ConfSource              *source = NULL;
+	ConfFile                *file;
+	JobClass                *class;
+	JobClass                *new_class;
+	Job                     *job;
+	Job                     *new_job;
+	Event                   *event;
+	Event                   *new_event;
+	Blocked                 *blocked;
+	Blocked                 *new_blocked;
+	NihList                  blocked_list;
+	size_t                   len;
+	json_object             *json_blocked;
+	Session                 *session;
+	Session                 *new_session;
 
 	conf_init ();
 	session_init ();
@@ -1014,6 +1073,278 @@
 	TEST_GROUP ("Blocked serialisation and deserialisation");
 
 	/*******************************/
+	TEST_FEATURE ("BLOCKED_JOB serialisation and deserialisation");
+
+	nih_list_init (&blocked_list);
+	TEST_LIST_EMPTY (&blocked_list);
+
+	source = conf_source_new (NULL, "/tmp/foo", CONF_JOB_DIR);
+	TEST_NE_P (source, NULL);
+
+	file = conf_file_new (source, "/tmp/foo/bar");
+	TEST_NE_P (file, NULL);
+	class = file->job = job_class_new (file, "bar", NULL);
+
+	TEST_NE_P (class, NULL);
+	TEST_HASH_EMPTY (job_classes);
+	TEST_TRUE (job_class_consider (class));
+	TEST_HASH_NOT_EMPTY (job_classes);
+
+	job = job_new (class, "");
+	TEST_NE_P (job, NULL);
+
+	parent_str = nih_strdup (NULL, "parent");
+	TEST_NE_P (parent_str, NULL);
+
+	blocked = blocked_new (NULL, BLOCKED_JOB, job);
+	TEST_NE_P (blocked, NULL);
+
+	json_blocked = state_serialise_blocked (blocked);
+	TEST_NE_P (json_blocked, NULL);
+
+	new_blocked = state_deserialise_blocked (parent_str,
+			json_blocked, &blocked_list);
+	TEST_NE_P (new_blocked, NULL);
+	TEST_LIST_NOT_EMPTY (&blocked_list);
+
+	assert0 (blocked_diff (blocked, new_blocked, ALREADY_SEEN_SET));
+
+	json_object_put (json_blocked);
+	nih_free (source);
+
+	/*******************************/
+	TEST_FEATURE ("BLOCKED_EVENT serialisation and deserialisation");
+
+	event = event_new (NULL, "event", NULL);
+	TEST_NE_P (event, NULL);
+
+	nih_list_init (&blocked_list);
+
+	source = conf_source_new (NULL, "/tmp/foo", CONF_JOB_DIR);
+	TEST_NE_P (source, NULL);
+
+	file = conf_file_new (source, "/tmp/foo/bar");
+	TEST_NE_P (file, NULL);
+	class = file->job = job_class_new (file, "bar", NULL);
+
+	TEST_NE_P (class, NULL);
+	TEST_HASH_EMPTY (job_classes);
+	TEST_TRUE (job_class_consider (class));
+	TEST_HASH_NOT_EMPTY (job_classes);
+
+	job = job_new (class, "");
+	TEST_NE_P (job, NULL);
+
+	TEST_LIST_EMPTY (&job->blocking);
+
+	blocked = blocked_new (NULL, BLOCKED_EVENT, event);
+	TEST_NE_P (blocked, NULL);
+
+	nih_list_add (&job->blocking, &blocked->entry);
+	TEST_LIST_NOT_EMPTY (&job->blocking);
+
+	event->blockers = 1;
+
+	parent_str = nih_strdup (NULL, "parent");
+	TEST_NE_P (parent_str, NULL);
+
+	json_blocked = state_serialise_blocked (blocked);
+	TEST_NE_P (json_blocked, NULL);
+
+	TEST_LIST_EMPTY (&blocked_list);
+	new_blocked = state_deserialise_blocked (parent_str,
+			json_blocked, &blocked_list);
+	TEST_NE_P (new_blocked, NULL);
+	TEST_LIST_NOT_EMPTY (&blocked_list);
+
+	assert0 (blocked_diff (blocked, new_blocked, ALREADY_SEEN_SET));
+
+	json_object_put (json_blocked);
+	nih_free (source);
+	nih_free (event);
+
+	/*******************************/
+	/* Test Upstart 1.6+ behaviour
+	 *
+	 * The data serialisation format for this version now includes
+	 * a "session" entry in the JSON for the blocked job.
+	 *
+	 * Note that this test is NOT testing whether a JobClass with an
+	 * associated Upstart session is handled correctly, it is merely
+	 * testing that a JobClass with the NULL session is handled
+	 * correctly!
+	 */
+	TEST_FEATURE ("BLOCKED_JOB with JSON session object");
+
+	TEST_LIST_EMPTY (sessions);
+	TEST_LIST_EMPTY (events);
+	TEST_LIST_EMPTY (conf_sources);
+	TEST_HASH_EMPTY (job_classes);
+
+	event = event_new (NULL, "Christmas", NULL);
+	TEST_NE_P (event, NULL);
+	TEST_LIST_EMPTY (&event->blocking);
+
+	source = conf_source_new (NULL, "/tmp/foo", CONF_JOB_DIR);
+	TEST_NE_P (source, NULL);
+
+	file = conf_file_new (source, "/tmp/foo/bar");
+	TEST_NE_P (file, NULL);
+	/* Create class with NULL session */
+	class = file->job = job_class_new (NULL, "bar", NULL);
+
+	TEST_NE_P (class, NULL);
+	TEST_HASH_EMPTY (job_classes);
+	TEST_TRUE (job_class_consider (class));
+	TEST_HASH_NOT_EMPTY (job_classes);
+
+	job = job_new (class, "");
+	TEST_NE_P (job, NULL);
+	TEST_HASH_NOT_EMPTY (class->instances);
+
+	blocked = blocked_new (event, BLOCKED_JOB, job);
+	TEST_NE_P (blocked, NULL);
+
+	nih_list_add (&event->blocking, &blocked->entry);
+	job->blocker = event;
+
+	TEST_LIST_NOT_EMPTY (&event->blocking);
+
+	assert0 (state_to_string (&json_string, &len));
+	TEST_GT (len, 0);
+
+	/* XXX: We don't remove the source as these are not
+	 * recreated on re-exec, so we'll re-use the existing one.
+	 */
+	nih_list_remove (&class->entry);
+	nih_list_remove (&event->entry);
+
+	TEST_LIST_EMPTY (events);
+	TEST_LIST_NOT_EMPTY (conf_sources);
+	TEST_HASH_EMPTY (job_classes);
+
+	assert0 (state_from_string (json_string));
+
+	TEST_LIST_NOT_EMPTY (conf_sources);
+	TEST_LIST_NOT_EMPTY (events);
+	TEST_HASH_NOT_EMPTY (job_classes);
+	TEST_LIST_EMPTY (sessions);
+
+	new_class = (JobClass *)nih_hash_lookup (job_classes, "bar");
+	TEST_NE_P (new_class, NULL);
+	nih_list_remove (&new_class->entry);
+
+	/* Upstart 1.6 can only deserialise the NULL session */
+	TEST_EQ_P (class->session, NULL);
+
+	new_event = (Event *)nih_list_remove (events->next);
+	TEST_LIST_EMPTY (events);
+	TEST_LIST_NOT_EMPTY (&new_event->blocking);
+	assert0 (event_diff (event, new_event, ALREADY_SEEN_SET));
+
+	nih_free (event);
+	nih_free (new_event);
+	nih_free (source);
+	nih_free (new_class);
+
+	TEST_HASH_EMPTY (job_classes);
+
+	TEST_LIST_EMPTY (sessions);
+	TEST_LIST_EMPTY (events);
+	TEST_LIST_EMPTY (conf_sources);
+	TEST_HASH_EMPTY (job_classes);
+
+	/*******************************/
+	/* We don't currently handle user+chroot jobs, so let's assert
+	 * that behaviour.
+	 */
+	TEST_FEATURE ("ensure BLOCKED_JOB with non-NULL session is ignored");
+
+	TEST_LIST_EMPTY (sessions);
+	TEST_LIST_EMPTY (events);
+	TEST_LIST_EMPTY (conf_sources);
+	TEST_HASH_EMPTY (job_classes);
+
+	session = session_new (NULL, "/my/session", getuid ());
+	TEST_NE_P (session, NULL);
+	session->conf_path = NIH_MUST (nih_strdup (session, "/lives/here"));
+	TEST_LIST_NOT_EMPTY (sessions);
+
+	/* We simulate a user job being blocked by a system event, hence
+	 * the session is not associated with the event.
+	 */
+	event = event_new (NULL, "Christmas", NULL);
+	TEST_NE_P (event, NULL);
+	TEST_LIST_EMPTY (&event->blocking);
+
+	source = conf_source_new (NULL, "/tmp/foo", CONF_JOB_DIR);
+	source->session = session;
+	TEST_NE_P (source, NULL);
+
+	file = conf_file_new (source, "/tmp/foo/bar");
+	TEST_NE_P (file, NULL);
+
+	/* Create class with non-NULL session, simulating a user job */
+	class = file->job = job_class_new (NULL, "bar", session);
+	TEST_NE_P (class, NULL);
+
+	TEST_HASH_EMPTY (job_classes);
+	TEST_TRUE (job_class_consider (class));
+	TEST_HASH_NOT_EMPTY (job_classes);
+
+	job = job_new (class, "");
+	TEST_NE_P (job, NULL);
+	TEST_HASH_NOT_EMPTY (class->instances);
+
+	blocked = blocked_new (event, BLOCKED_JOB, job);
+	TEST_NE_P (blocked, NULL);
+
+	nih_list_add (&event->blocking, &blocked->entry);
+	job->blocker = event;
+
+	TEST_LIST_NOT_EMPTY (&event->blocking);
+
+	assert0 (state_to_string (&json_string, &len));
+	TEST_GT (len, 0);
+
+	/* XXX: We don't remove the source as these are not
+	 * recreated on re-exec, so we'll re-use the existing one.
+	 */
+	nih_list_remove (&class->entry);
+	nih_free (event);
+	nih_list_remove (&session->entry);
+
+	TEST_LIST_EMPTY (events);
+	TEST_LIST_NOT_EMPTY (conf_sources);
+	TEST_HASH_EMPTY (job_classes);
+
+	assert0 (state_from_string (json_string));
+
+	TEST_LIST_NOT_EMPTY (conf_sources);
+	TEST_LIST_NOT_EMPTY (events);
+
+	/* We don't expect any job_classes since the serialised one
+	 * related to a user session.
+	 */
+	TEST_HASH_EMPTY (job_classes);
+
+	/* However, the session itself will exist */
+	TEST_LIST_NOT_EMPTY (sessions);
+
+	new_session = (Session *)nih_list_remove (sessions->next);
+
+	nih_free (session);
+	nih_free (new_session);
+	event = (Event *)nih_list_remove (events->next);
+	nih_free (event);
+	nih_free (source);
+
+	TEST_LIST_EMPTY (sessions);
+	TEST_LIST_EMPTY (events);
+	TEST_LIST_EMPTY (conf_sources);
+	TEST_HASH_EMPTY (job_classes);
+
+	/*******************************/
 	TEST_FEATURE ("event blocking a job");
 
 	TEST_LIST_EMPTY (sessions);
@@ -2608,6 +2939,141 @@
 	/*******************************/
 }
 
+/**
+ * test_upgrade:
+ * 
+ * Run tests that simulate an upgrade by attempting to deserialise an
+ * older version of the JSON data format than is currently used.
+ **/
+void
+test_upgrade (void)
+{
+	TestDataFile  *datafile;
+
+	TEST_GROUP ("upgrade tests");
+
+	for (datafile = test_data_files; datafile && datafile->filename; datafile++) {
+		nih_local char  *path = NULL;
+		nih_local char  *name = NULL;
+
+		nih_assert (datafile->func != NULL);
+
+		name = NIH_MUST (nih_sprintf (NULL, "with data file '%s'",
+					datafile->filename));
+		TEST_FEATURE (name);
+
+		path = NIH_MUST (nih_sprintf (NULL, "%s/%s",
+					TEST_DATA_DIR, datafile->filename));
+
+		datafile->func (datafile->conf_file, path);
+	}
+}
+
+/**
+ * test_upstart1_6_upgrade:
+ *
+ * @conf_file: name of ConfFile to create prior to running test,
+ * @path: full path to JSON data file to deserialise.
+ *
+ * Test for original Upstart 1.6 serialisation data format containing
+ * a blocked object that does not contain a 'session' element.
+ *
+ * Note that this test is NOT testing whether a JobClass with an
+ * associated Upstart session is handled correctly, it is merely
+ * testing that a JobClass with the NULL session encoded in the JSON
+ * is handled correctly.
+ **/
+void
+test_upstart1_6_upgrade (const char *conf_file, const char *path)
+{
+	nih_local char  *json_string = NULL;
+	Event           *event;
+	ConfSource      *source;
+	ConfFile        *file;
+	nih_local char  *conf_file_path = NULL;
+	struct stat      statbuf;
+	size_t           len;
+
+	nih_assert (conf_file);
+	nih_assert (path);
+
+	conf_init ();
+	session_init ();
+	event_init ();
+	control_init ();
+	job_class_init ();
+
+	TEST_LIST_EMPTY (sessions);
+	TEST_LIST_EMPTY (events);
+	TEST_LIST_EMPTY (conf_sources);
+	TEST_HASH_EMPTY (job_classes);
+
+	/* Check data file exists */
+	TEST_EQ (stat (path, &statbuf), 0);
+
+	json_string = nih_file_read (NULL, path, &len);
+	TEST_NE_P (json_string, NULL);
+
+	/* Create the ConfSource and ConfFile objects to simulate
+	 * Upstart reading /etc/init on startup. Required since we
+	 * don't currently serialise these objects.
+	 */
+	source = conf_source_new (NULL, "/tmp/foo", CONF_JOB_DIR);
+	TEST_NE_P (source, NULL);
+
+	conf_file_path = NIH_MUST (nih_sprintf (NULL, "%s/%s",
+				"/tmp/foo", conf_file));
+
+	file = conf_file_new (source, conf_file_path);
+	TEST_NE_P (file, NULL);
+
+	/* Recreate state from JSON data file */
+	assert0 (state_from_string (json_string));
+
+	TEST_LIST_NOT_EMPTY (conf_sources);
+	TEST_LIST_NOT_EMPTY (events);
+	TEST_HASH_NOT_EMPTY (job_classes);
+	TEST_LIST_EMPTY (sessions);
+
+	event = (Event *)nih_list_remove (events->next);
+	TEST_NE_P (event, NULL);
+	TEST_EQ_STR (event->name, "Christmas");
+
+	NIH_HASH_FOREACH (job_classes, iter) {
+		JobClass *class = (JobClass *)iter;
+
+		TEST_EQ_STR (class->name, "bar");
+		TEST_EQ_STR (class->path, "/com/ubuntu/Upstart/jobs/bar");
+		TEST_HASH_NOT_EMPTY (class->instances);
+
+		NIH_HASH_FOREACH (class->instances, iter2) {
+			Blocked        *blocked;
+			Job            *job = (Job *)iter2;
+			nih_local char *instance_path = NULL;
+
+			/* instance name */
+			TEST_EQ_STR (job->name, "");
+
+			instance_path = NIH_MUST (nih_sprintf (NULL, "%s/_", class->path));
+			TEST_EQ_STR (job->path, instance_path);
+
+			/* job is blocked by the event */
+			TEST_EQ (job->blocker, event);
+
+			/* First entry in list should be a Blocked
+			 * object pointing to the job.
+			 */
+			TEST_LIST_NOT_EMPTY (&event->blocking);
+			blocked = (Blocked *)(&event->blocking)->next;
+			TEST_EQ (blocked->type, BLOCKED_JOB);
+			TEST_EQ (blocked->job, job);
+		}
+	}
+
+	nih_free (event);
+	nih_free (conf_sources);
+}
+
 int
 main (int   argc,
       char *argv[])
@@ -2632,6 +3098,7 @@
 	test_log_serialise ();
 	test_job_serialise ();
 	test_job_class_serialise ();
+	test_upgrade ();
 
 	return 0;
 }