alias stanza wip patch

[Marc - A. Dahlhaus]

Hi James,

Am Dienstag, den 21.06.2011, 16:05 +0100 schrieb James Hunt:
> Hi Marc,
> 
> On 16/06/11 23:19, Marc - A. Dahlhaus wrote:
> > Am 16.06.2011 20:53, schrieb James Hunt:
> >> -----BEGIN PGP SIGNED MESSAGE-----
> >> Hash: SHA1
> >>
> >> Hi Mark,
> Oops! Sorry :-)
> 
> >> * init/job.c: job_emit_event(): Maybe "blocked_alias" would be easier to read than "blockedalias".
> > 
> > I changed it.
> Great.
> 
> >> - - What if multiple .conf files *do* contain the same alias name? Even though this isn't legit, we
> >> need to ensure we handle the scenario.
> >> - - What if a job defines an alias for an existing job name?
> > 
> > First active job or alias should win
> Well, are we even into active jobs before we need to handle this resolution problem?
> 
> If we had:
> 
> a-display-manager.conf:
> 
> 	alias display-manager
> 
> gdm.conf:
> 
> 	alias display-manager
> 
> Upstart will read both at boot time of course. I was imagining that we would build the job->alias
> map initially which here would cause a dilemma. Since this is a stanza, the current behaviour of
> Upstart is to honour the *last* use of a stanza so we could be construed as changing the behaviour
> by honouring the first mention of a stanza.

If we have multiple alias stanzas inside of a single job config, than
yes, this would be right but we talk about different jobs that are
providing the same stanza, no overwrite of the alias value can happen as
the live in different job-name-spaces...

> Even if we delay resolving an alias (and thus adding it into a map) until a "job" which doesn't
> correspond directly to a .conf file (ie an alias) needs to be started, we still have the same
> problem in that we would have to trawl through the "job_classes" hash and somehow determine the
> "correct" job to use for the specified alias.
> 
> Further, if we only store a ref to the single (first?) job that sets an alias, what happens here?:
> 
>   $ rm a-display-manager.conf
> 
> I think the answer is that we'd need to reload the *entire* /etc/init/ directory looking for a new
> job that resolves the display-manager alias. This isn't ideal. So, we'd need to atleast record
> job->alias for every job that specifies "alias" to avoid rescanning the entire directory.

We simply remove the JobClass of the removed config form the alias list
inside of the known_job and if  ->job and ->alias are empty after that
we remove the entire entry from known_jobs in that case...

> 
>  and a job always should take
> > precedence before aliases.
> That makes sense however it does mean we always have to do 2 lookups for every job: 1 in the alias
> map and one in job_classes. Maybe we simply disallow an alias name being the same as an existing job
> and complain bitterly if we find this scenario?

No, we just use the first active JobClass. If ->job is set, we use it,
no lookup for content in ->alias is done.

> > 
> > I also thought about the handling in initctl, how about adding jobs and
> > aliases to a hash list, and use the list as a source of known jobs for
> > initctl.
> I'm not sure I understand this 100%. Are you proposing we add this hash to initctl itself? I think
> the alias handling can only occur in init itself since initctl is (mostly) just a D-Bus client that
> gets init to do the heavy lifting and we would want to allow any D-Bus client access to alias info
> in principle I think.

The hashlist stays in init itself of course.
I thought about the info init sends down to initctl for the list and
status commands.

We want to remove the reloading in initctl itself and replace this with
a reload stanza, right?
This will prepare init for that...

> > 
> > typedef struct known_job {
> >     NihList   entry;
> >     char     *name;    /* job or alias name */
> >     Session  *session;
> >     JobClass *job;     /* pointer to JobClass object of real job */
> >     NihList   aliases; /* NihListEntry pointers to JobClass objects */
> > } NamedJob;
> > 
> > NihHash *known_jobs;
> > 
> > On any alteration of a Named Task (no matter if its a real job or an
> > alias) with initctl we could look up the name in the list of known jobs
> > and when we find one we use the first non manual JobClass we can find
> > for the interaction. We first check if *known_jobs->job is set and not
> > disabled by manual and use it or otherwise do the same checks for every
> > list entry in aliases.
> > 
> > That way we have no complicated error handling involved. We add a job to
> > its hash entry and is takes precedence even if aliases for the job were
> > parsed earlier. We can use the hash list for any initctl interaction.
> > 
> > I haven't read over the code of initctl and i did not look if a hash
> > list for job already exists in upstart that could be extended to handle
> > aliases so all of this should be taken with a grain of salt.
> In init itself, we have "job_classes", but this currently disallows multiple jobs with the same name
> (in the same session).

We don't have to change that. As the ->job member above is exactly that.
We have an array behind ->aliases that would be filled with jobs that
provide a given alias. We just need to extend the lookup-logic for that
list that is used by the signals initctl sends to check which job should
handle the signal.

> > 
> > I don't think that we need to disable the sending of alias events for
> > jobs that are active and are not the first provider for a given alias.
> > All events depending on the alias are run by the first provider of the
> > alias so all should be well even if we encounter an event more than one
> > time.
> Yes, I see your point here although we would need to document this behaviour carefully to avoid
> users having surprises :)

Of course.

Thanks,

Marc