James Hunt has proposed merging lp:~jamesodhunt/upstart/bug-1079715 into
lp:upstart.
Requested reviews:
Upstart Reviewers (upstart-reviewers)
For more details, see:
https://code.launchpad.net/~jamesodhunt/upstart/bug-1079715/+merge/135388
* init/job_class.c:
- job_class_consider(): Removed redundant braces.
- job_class_reconsider(): Removed redundant braces.
- job_class_add_safe(): Consider session before asserting
(LP: #1079715).
- job_class_serialise():
- Explicitly disallow user and chroot sessions
from being serialised since this scenario is not supported
(due to our not serialising ConfSource objects yet).
- job_class_deserialise():
- Check session as early as possible.
- Assert that we do not have user and chroot sessions to deal with.
- Fix potential invalid free if error occurs before JobClass
is created.
* init/session.h: Comment.
* init/state.c:
- state_deserialise_resolve_deps(): Specify new session parameter to
state_get_job().
- state_serialise_blocked(): Encode session index for BLOCKED_JOB.
- state_deserialise_blocked(): Extract session from index index for
BLOCKED_JOB to pass to state_get_job().
- state_get_job(): Add @session parameter to allow exact job match.
--
https://code.launchpad.net/~jamesodhunt/upstart/bug-1079715/+merge/135388
Your team Upstart Reviewers is requested to review the proposed merge of
lp:~jamesodhunt/upstart/bug-1079715 into lp:upstart.
=== modified file 'init/job_class.c'
--- init/job_class.c 2012-11-14 14:47:19 +0000
+++ init/job_class.c 2012-11-21 12:12:26 +0000
@@ -271,9 +271,7 @@
/* If we found an entry, ensure we only consider the appropriate session */
while (registered && registered->session != class->session)
- {
registered = (JobClass *)nih_hash_search (job_classes, class->name, ®istered->entry);
- }
if (registered != best) {
if (registered)
@@ -314,9 +312,7 @@
/* If we found an entry, ensure we only consider the appropriate session */
while (registered && registered->session != class->session)
- {
registered = (JobClass *)nih_hash_search (job_classes, class->name, ®istered->entry);
- }
if (registered == class) {
if (class != best) {
@@ -364,7 +360,7 @@
* @class: new class to select.
*
* Adds @class to the hash table iff no existing entry of the
- * same name exists.
+ * same name exists for the same session.
**/
void
job_class_add_safe (JobClass *class)
@@ -378,6 +374,10 @@
existing = (JobClass *)nih_hash_search (job_classes, class->name, NULL);
+ /* Ensure no existing class exists for the same session */
+ while (existing && existing->session != class->session)
+ existing = (JobClass *)nih_hash_search (job_classes, class->name, &existing->entry);
+
nih_assert (! existing);
job_class_add (class);
@@ -1592,6 +1592,15 @@
json = json_object_new_object ();
if (! json)
return NULL;
+
+ /* XXX: user and chroot jobs are not currently supported
+ * due to ConfSources not currently being serialised.
+ */
+ if (class->session) {
+ nih_info ("WARNING: serialisation of user jobs and "
+ "chroot sessions not currently supported");
+ goto error;
+ }
session_index = session_get_index (class->session);
if (session_index < 0)
@@ -1797,6 +1806,7 @@
{
json_object *json_normalexit;
JobClass *class = NULL;
+ Session *session;
int session_index = -1;
int ret;
nih_local char *name = NULL;
@@ -1806,29 +1816,28 @@
nih_assert (job_classes);
if (! state_check_json_type (json, object))
- goto error;
+ goto out;
if (! state_get_json_int_var (json, "session", session_index))
- goto error;
+ goto out;
if (session_index < 0)
- goto error;
+ goto out;
+
+ session = session_from_index (session_index);
+
+ /* XXX: user and chroot jobs are not currently supported
+ * due to ConfSources not currently being serialised.
+ */
+ nih_assert (session == NULL);
if (! state_get_json_string_var_strict (json, "name", NULL, name))
- goto error;
+ goto out;
- class = NIH_MUST (job_class_new (NULL, name,
- session_from_index (session_index)));
+ class = NIH_MUST (job_class_new (NULL, name, session));
if (! class)
goto error;
- if (class->session != NULL) {
- nih_warn ("XXX: WARNING (%s:%d): deserialisation of "
- "user jobs and chroot sessions not currently supported",
- __func__, __LINE__);
- goto error;
- }
-
/* job_class_new() sets path */
if (! state_get_json_string_var_strict (json, "path", NULL, path))
goto error;
@@ -2003,6 +2012,7 @@
error:
nih_free (class);
+out:
return NULL;
}
=== modified file 'init/session.h'
--- init/session.h 2012-06-29 16:19:33 +0000
+++ init/session.h 2012-11-21 12:12:26 +0000
@@ -39,7 +39,8 @@
* with a chroot path)).
*
* This structure is used to identify collections of jobs
- * that share either a common @chroot and/or common @user.
+ * that share either a common @chroot and/or common @user. Note that
+ * @conf_path is unique across all sessions.
*
* Summary of Session values for different environments:
*
=== modified file 'init/state.c'
--- init/state.c 2012-11-07 11:56:33 +0000
+++ init/state.c 2012-11-21 12:12:26 +0000
@@ -81,7 +81,8 @@
__attribute__ ((warn_unused_result));
static Job *
-state_get_job (const char *job_class, const char *job_name)
+state_get_job (const Session *session, const char *job_class,
+ const char *job_name)
__attribute__ ((warn_unused_result));
/**
@@ -1164,7 +1165,7 @@
goto error;
/* lookup job */
- job = state_get_job (class->name, job_name);
+ job = state_get_job (class->session, class->name, job_name);
if (! job)
goto error;
@@ -1205,6 +1206,7 @@
{
json_object *json;
json_object *json_blocked_data;
+ int session_index;
nih_assert (blocked);
@@ -1220,6 +1222,8 @@
switch (blocked->type) {
case BLOCKED_JOB:
{
+ Session *session;
+
/* Need to encode JobClass name and Job name to make
* it unique.
*/
@@ -1228,6 +1232,20 @@
blocked->job->class->name))
goto error;
+ session = blocked->job->class->session;
+
+ session_index = session_get_index (session);
+ if (session_index < 0)
+ goto error;
+
+ /* Encode parent classes session index to aid in
+ * finding the correct job on deserialisation.
+ */
+ if (! state_set_json_int_var (json_blocked_data,
+ "session",
+ session_index))
+ goto error;
+
if (! state_set_json_string_var (json_blocked_data,
"name",
blocked->job->name))
@@ -1397,7 +1415,7 @@
Blocked *blocked = NULL;
nih_local char *blocked_type_str = NULL;
BlockedType blocked_type;
- int ret;
+ int ret;
nih_assert (parent);
nih_assert (json);
@@ -1421,6 +1439,8 @@
nih_local char *job_name = NULL;
nih_local char *job_class_name = NULL;
Job *job;
+ Session *session;
+ int session_index;
if (! state_get_json_string_var_strict (json_blocked_data,
"name", NULL, job_name))
@@ -1430,7 +1450,15 @@
"class", NULL, job_class_name))
goto error;
- job = state_get_job (job_class_name, job_name);
+ if (! state_get_json_int_var (json_blocked_data, "session", session_index))
+ goto error;
+
+ if (session_index < 0)
+ goto error;
+
+ session = session_from_index (session_index);
+
+ job = state_get_job (session, job_class_name, job_name);
if (! job)
goto error;
@@ -1625,6 +1653,7 @@
/**
* state_get_job:
*
+ * @session: session of job class,
* @job_class: name of job class,
* @job_name: name of job instance.
*
@@ -1635,7 +1664,9 @@
* job not found.
**/
static Job *
-state_get_job (const char *job_class, const char *job_name)
+state_get_job (const Session *session,
+ const char *job_class,
+ const char *job_name)
{
JobClass *class;
Job *job;
@@ -1643,7 +1674,13 @@
nih_assert (job_class);
nih_assert (job_classes);
- class = (JobClass *)nih_hash_lookup (job_classes, job_class);
+ class = (JobClass *)nih_hash_search (job_classes, job_class, NULL);
+ if (! class)
+ goto error;
+
+ while (class && class->session != session)
+ class = (JobClass *)nih_hash_search (job_classes, job_class, &class->entry);
+
if (! class)
goto error;
--
upstart-devel mailing list
[email protected]
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/upstart-devel
