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