[12:51] <jodh> xnox: hi - could you take a look at https://code.launchpad.net/~jamesodhunt/upstart/bug-1199778/+merge/174138 when you get a moment?
[13:25] <xnox> jodh: reviewed. Slightly confused why we need those three asserts =) looks like a timebomb waiting to explode.
[14:30] <jodh> xnox: thanks. I disagree about the timebomb bit though :)
[14:31] <xnox> jodh: so what are we asserting?
[14:31] <jodh> xnox: MP updated
[14:31] <xnox> looking
[14:33] <xnox> jodh: so you are saying it is only valid to deserialise to a completely blank state? why would that be necessary?
[14:34] <xnox> jodh: i can write a unit test that does 3 deserialise_all in a row and check that objects are correctly replaced?
[14:36] <jodh> xnox: yes, it's only valid to deserialise from a blank state.
[14:40] <xnox> jodh: and if the state is not blank, state_from_string should return error code less than 0, error condition should run - write out state_write_file, and dumn re-exec should have been run.
[14:40] <xnox> s/dumn/dump/
[14:41] <xnox> my understanding was that stateful re-exec should give up, under failure conditions.
[14:41] <xnox> but not assert.
[14:42] <jodh> xnox: I think it's probably best to raise a bug with these ideas so we can work on refining this as a separate project.
[14:42] <xnox> ok.
[14:44] <jodh> xnox: in terms of the bug at hand though, can you confirm the fix works based on my instructions on how to recreate?
[14:50] <xnox> jodh: it should. But there are a couple things I want to try though first. I'm on my laptop in bluefin, will be back at my desktop in the evening.
[14:51] <xnox> jodh: i think this bug can be caught by testing deserialisation with a dump from a system where sessions jobs are running.
[14:51] <xnox> thus it should be added as a unit test.
[14:52] <xnox> at the moment all of our files are without sessions.
[14:55] <jodh> xnox: agreed - we should add such a test.
[16:12] <xnox> jodh: so, it still fails for me.
[16:12] <xnox> not sure if there is something wrong with my test.
[16:12] <xnox> see: https://code.launchpad.net/~xnox/upstart/bug-1199778/+merge/174235
[16:13] <xnox> or just branch lp:~xnox/upstart/bug-1199778 and build/run the test_conf test.
[16:13] <xnox> this branch has your fix merged.
[16:14] <jodh> xnox: is it crashing in the same place? assert/backtrace?
[16:14] <xnox> ...with unflushed data
[16:14] <xnox> (null): No ConfSources present in state data
[16:14] <xnox> (null): No ConfSources present in state data
[16:14] <xnox> (null): No ConfSources present in state data
[16:14] <xnox> (null): No ConfSources present in state data
[16:14] <xnox> (null): Failed to resolve deserialisation dependencies
[16:14] <xnox> test_state: tests/test_state.c:3371: test_session_upgrade: Assertion `(state_from_string (json_string)) == 0' failed.
[16:14] <xnox> jodh: I wonder if the pastebin that I fetched from the VM was incomplete serialisation (well chopped off due to size)
[16:16] <jodh> xnox: well, is it valid json? what does "json_pp < file.json" do?
[16:18] <xnox> json_pp spins at 100%.....
[16:19] <xnox> produced formatted output, let me rerun again to see the exit status
[16:19] <xnox> exited with 0, so it's all of it.
[16:26] <jodh> xnox: I think gdb may be required to establish where the failure is being indicated :)
[16:27] <xnox> it's weird.
[16:27] <xnox> so running without the silly automake runner.
[16:27] <xnox> Testing upgrade tests
[16:27] <xnox> ...with data file 'upstart-1.6.json'
[16:27] <xnox> (null): No ConfSources present in state data
[16:27] <xnox> ...with data file 'upstart-1.8.json'
[16:27] <xnox> (null): No ConfSources present in state data
[16:27] <xnox> ...with data file 'upstart-pre-security.json'
[16:27] <xnox> (null): No ConfSources present in state data
[16:27] <xnox> ...with data file 'upstart-1.8+apparmor.json'
[16:27] <xnox> (null): No ConfSources present in state data
[16:27] <xnox> ...with data file 'upstart-1.8+full_serialisation-apparmor.json'
[16:27] <xnox> ...with data file 'upstart-1.8+full_serialisation+apparmor.json'
[16:28] <xnox> ...with data file 'upstart-session.json'
[16:28] <xnox> (null): Failed to resolve deserialisation dependencies
[16:28] <xnox> test_state: tests/test_state.c:3371: test_session_upgrade: Assertion `(state_from_string (json_string)) == 0' failed.
[16:28] <xnox> Aborted (core dumped)
[16:28] <xnox> So all of them have no confsources present.... which is well totally not true.
[16:30] <xnox> jodh: going back to before your fix also has "No ConfSources present in state data" messages printed....
[16:30] <xnox> which I guess is correct, since only full_serialisation introduced thsoe.
[16:30] <jodh> xnox: correct.
[16:31] <xnox> jodh: is it expected for chroot deserialisation to fail resolving dependencies...?
[16:33] <jodh> xnox: slightly bizarrely, chroot sessions are not deserialised - they are detected, then ignored with a warning (see my comments on bug 1200264).
[16:34] <xnox> jodh: ok. Hm..... yet something later probs for them..... like the full deserialisation code?
[16:35] <xnox> i'll debug it.
[16:35] <xnox> jodh: have fun wherever you are off to =)
[22:27] <xnox> I guess 1-to-1 mappings don't work, if you decide to skip a couple of items from the first set, and rely on positional mapping between the two sets.........