| === erkules_ is now known as erkules | ||
| jodh | xnox: hi - could you take a look at https://code.launchpad.net/~jamesodhunt/upstart/bug-1199778/+merge/174138 when you get a moment? | 12:51 |
|---|---|---|
| xnox | jodh: reviewed. Slightly confused why we need those three asserts =) looks like a timebomb waiting to explode. | 13:25 |
| jodh | xnox: thanks. I disagree about the timebomb bit though :) | 14:30 |
| xnox | jodh: so what are we asserting? | 14:31 |
| jodh | xnox: MP updated | 14:31 |
| xnox | looking | 14:31 |
| xnox | jodh: so you are saying it is only valid to deserialise to a completely blank state? why would that be necessary? | 14:33 |
| xnox | jodh: i can write a unit test that does 3 deserialise_all in a row and check that objects are correctly replaced? | 14:34 |
| jodh | xnox: yes, it's only valid to deserialise from a blank state. | 14:36 |
| 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:40 |
| xnox | my understanding was that stateful re-exec should give up, under failure conditions. | 14:41 |
| xnox | but not assert. | 14:41 |
| 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:42 |
| 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:44 |
| 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:50 |
| 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:51 |
| xnox | at the moment all of our files are without sessions. | 14:52 |
| jodh | xnox: agreed - we should add such a test. | 14:55 |
| 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:12 |
| 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:13 |
| 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:14 |
| jodh | xnox: well, is it valid json? what does "json_pp < file.json" do? | 16:16 |
| xnox | json_pp spins at 100%..... | 16:18 |
| 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:19 |
| jodh | xnox: I think gdb may be required to establish where the failure is being indicated :) | 16:26 |
| 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:27 |
| 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:28 |
| 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:30 |
| xnox | jodh: is it expected for chroot deserialisation to fail resolving dependencies...? | 16:31 |
| jodh | xnox: slightly bizarrely, chroot sessions are not deserialised - they are detected, then ignored with a warning (see my comments on bug 1200264). | 16:33 |
| xnox | jodh: ok. Hm..... yet something later probs for them..... like the full deserialisation code? | 16:34 |
| xnox | i'll debug it. | 16:35 |
| xnox | jodh: have fun wherever you are off to =) | 16:35 |
| 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......... | 22:27 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!