| === SpamapS_ is now known as SpamapS | ||
| xnox | jodh: https://www.gnu.org/software/automake/manual/html_node/Parallel-Test-Harness.html#Parallel-Test-Harness | 11:05 |
|---|---|---|
| jodh | ack. | 11:06 |
| jodh | xnox: I've just updated lp:~jamesodhunt/upstart/serialise-remaining-objects with the new tests we discussed yesterday. Can you review when you get a chance so we can discuss the backporting after lunch? | 11:06 |
| xnox | jodh: right. I was starting to rereview your changes from 18h ago. will pull again. | 11:07 |
| jodh | xnox: thanks | 11:07 |
| xnox | jodh: hmm..... shouldn't there also be: upstart-1.8 +full_serial +apparmor? since as soon as we merge this branch that's what the current format is going to become | 11:29 |
| jodh | xnox: yep - on it... | 12:16 |
| jodh | ok - branch updated. | 12:25 |
| xnox | jodh: looks good. I guess none of the jobs use apparmor_switch yet?! all dumps just have it as "null" and thus, well it's just forwards compatible stanza then =))))) | 12:50 |
| jodh | xnox: correct. I guess I could add in some apparmor_switch values for completeness... | 12:53 |
| xnox | jodh: do you just run your store-dump patch to generate these dumps? are those merged into lp:upstart already | 12:54 |
| xnox | jodh: to be honest no need, as there are other tests which correctly test strings in that field. | 12:54 |
| xnox | and i'm sure that we are able to serialise/deserialise strings =) | 12:55 |
| * xnox prefers real dumps vs artificially constructed dumps. | 12:55 | |
| jodh | xnox: that was my view :) | 12:55 |
| xnox | ah, write_state_file is in this branch. | 13:00 |
| xnox | jodh: how do you distinguish between: confsources serialised and fail to deserialise vs not-present. | 13:02 |
| xnox | and is it possible to distinguish. | 13:03 |
| jodh | xnox: I think we'd need the STATE_VERSION bits to distinguish between those 2 scenarios reliably. | 13:08 |
| xnox | jodh: hmm... i would have thought conf_source_deserialise would for example return -1 if 'conf_sources' key is not found in json, and -2 if 'conf_sources' key was found but failed to deserialise in any other way. | 13:12 |
| xnox | cd ../ | 13:13 |
| jodh | xnox: there is no point distinguishing in fact since we don't know the serialisation format version of the init which is feeding us data. Hence, we error on any unexpected scenario and let the caller decide what to do. And since we only latterly added serialisation of ConfSources but don't have a version number, the best we can do is display a warning. | 13:16 |
| === hatchetation_ is now known as hatchetation | ||
| xnox | jodh: that's not what I was after. if we do feature detection, and 'conf_sources' key is present in the serialisation format, we expect conf_source_deserialise_all to succeed. Yet, if for example, somewhere inside conf_sources the stream got corrupted (single bit flip in ram), the call to conf_source_deserialise_all will fail, yet state_from_string will return success. | 13:41 |
| xnox | which is imho wrong. | 13:41 |
| xnox | this is for the case of latest upstart doing stateful rexec to latest upstart. | 13:41 |
| === soren_ is now known as soren | ||
| === JanC is now known as Guest69968 | ||
| xnox | jodh: something like this? http://paste.ubuntu.com/5735811/ | 13:51 |
| === crank_ is now known as crank | ||
| jodh | xnox: I guess we could do that, but we'd need to do a bit of rework to make the code consistent in that respect. | 14:34 |
| === Guest69968 is now known as JanC | ||
| xnox | jodh: so is above unreasonable? | 15:55 |
| jodh | I guess it makes sense to add that check. We still can't detect of course that the ConfSources are missing when we actually expect them to be present, but I can add the extra check. | 15:56 |
| xnox | jodh: sure, and that's kind of a limitation of feature-based-testing. i wonder if we should be checksumming the serialisation =) | 15:57 |
| slangasek | xnox: if they're absent and supposed to be present, that's not going to be a bit-flip error :) | 16:02 |
| slangasek | jodh, xnox: reading scrollback, I agree that we want to distinguish between "this new bit is absent" and "we failed to deserialize this bit". The -2 == warning isn't very idiomatic, however. What about just returning 0 for this case? Or, having the caller check for conf_sources and skip the call to conf_source_deserialise_all() if absent? | 16:03 |
| jodh | slangasek/xnox: yeah, I vote for the latter as there is already precendent for that. | 16:04 |
| xnox | slangasek: yeah "-2" is a bit out of the blue. I prefer "having the caller check for conf_sources and skip the call to conf_source_deserialise_all() if absent?" over the '-2' hack. | 16:10 |
| slangasek | ok | 16:10 |
| jodh | xnox: it's a shame as it imbues the caller with knowledge of the format of the json, but as you would say "meh" :) | 16:10 |
| slangasek | then we all agree :) | 16:10 |
| * jodh is on it... | 16:11 | |
| jodh | slangasek/xnox: branch updated. | 16:19 |
| xnox | jodh: looks good to me. | 16:57 |
| xnox | jodh: are you planning to make a release, or simply take a single cherrypick of this both into raring & saucy? | 16:58 |
| jodh | xnox: well, I had planned on making a release once all the o/s branches had been merged. However, as I'm away for a while, maybe a cherry-pick would be the most expeditious option? | 18:02 |
| slangasek | xnox: so given that jodh is effectively on [vac] already, I think we need to go the cherry pick route | 18:09 |
| slangasek | bonus: that means we get to test the cherry-pick in saucy /before/ SRU to raring | 18:09 |
| === marrusl is now known as marrusl_really | ||
| === crank_ is now known as crank | ||
| xnox | ack. | 22:03 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!