This is excellent. I really like the way you've introduced the new security 
process type as it is extensible and allows any apparmor_parser failure output 
to be logged automatically ;-)

My only comment is that we could do with on more test to assert that we can 
handle reading an "entire" state file that does _not_ contain the new 
AppArmor-related serialisation data. Specifically, can you add an explicit test 
to init/tests/test_state.c:test_data_files that:

1) Reads in a prepared JSON file without the AppArmor content.
2) Asserts 'assert0 (state_from_string (json_string));'
3) Checks that JobClass->apparmor_switch has a reasonable value.
4) Checks that Job->pid[PROCESS_SECURITY] has a reasonable value.


Also, a reminder-to-self:

Once lp:~jamesodhunt/upstart/serialise-remaining-objects lands, we'll need to 
bump STATE_VERSION to take account of the new serialisation format 
(PROCESS_SECURITY and JobClass->apparmor_switch).

-- 
https://code.launchpad.net/~mdeslaur/upstart/apparmor-support/+merge/164169
Your team Upstart Reviewers is requested to review the proposed merge of 
lp:~mdeslaur/upstart/apparmor-support into lp:upstart.

-- 
upstart-devel mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/upstart-devel

Reply via email to