[Merge] lp:~jamesodhunt/upstart/serialise-remaining-objects into lp:upstart

[Steve Langasek]

--- init/event_operator.h       2013-02-27 11:46:04 +0000
+++ init/event_operator.h       2013-05-30 17:07:37 +0000
[...]
+event_operator_deserialise_all (void *parent, json_object *json)
+       __attribute__ ((warn_unused_result));
+
+NIH_END_EXTERN
+
 char *event_operator_collapse (EventOperator *condition)
-       __attribute__ ((warn_unused_result));
-
-NIH_END_EXTERN
+       __attribute__ ((warn_unused_result, unused));

This moves the event_operator_collapse() definition outside the extern "C" {} block, which seems like a mistake?

--- init/job_class.c    2013-05-15 13:21:54 +0000
+++ init/job_class.c    2013-05-30 17:07:37 +0000
@@ -1910,17 +2015,17 @@
 
                json_class = job_class_serialise (class);
 
-               /* No object returned means the class doesn't need to be
-                * serialised.  Even if this is a real failure, it's always
-                * better to serialise as much of the state as possible.
-                */
                if (! json_class)
-                       continue;
+                       goto error;
 
                json_object_array_add (json, json_class);

I think the reasoning in that comment still holds (that it's better to serialize as much of the state as possible).  But this is consistent with the behavior in the rest of the code, so I suppose that shouldn't be a blocker.

--- init/state.c        2013-02-28 16:40:36 +0000
+++ init/state.c        2013-05-30 17:07:37 +0000
[...]
+       /* Again, we cannot error here since older JSON state data did
+        * not encode ConfSource or ConfFile objects.
+        */
+       if (conf_source_deserialise_all (json) < 0)
+               nih_warn ("%s", _("No ConfSources present in state data"));

Wouldn't it make sense to have a check on the format version, and make this an error for new formats vs. a warning for the old format?

So given that there are version checks now in the deserializer, and there is another change to the serialization format landing upstream at the same time: how can we backport this change to raring in an SRU?  Should we consider the format with apparmor_switch to be version 4, and treat the confsource serialization without apparmor_switch to be version 3, then include tests for each (i.e., change 
test_upstart_pre_security_upgrade() to use v3)?

Finally, it appears that there is no test json for the *current* serialization format.  This should definitely be included as its own test, and added to the tree now - not be left until the next time someone changes the serialization format (or accidentally changes the deserializer!).

Other than that, this looks very good.  If you can fix up the above issues, I'll approve this.
-- 
https://code.launchpad.net/~jamesodhunt/upstart/serialise-remaining-objects/+merge/163151
Your team Upstart Reviewers is subscribed to branch lp:upstart.