[Merge] lp:~jamesodhunt/upstart/allow-multiple-cmdline-confdirs into lp:upstart

[Dmitrijs Ledkovs]

On 2 April 2013 11:31, James Hunt <james.hunt at canonical.com> wrote:
>
> === modified file 'init/main.c'
> --- init/main.c 2013-03-01 15:13:54 +0000
> +++ init/main.c 2013-04-02 10:30:30 +0000
> @@ -88,6 +88,7 @@
>  static void handle_confdir      (void);
>  static void handle_logdir       (void);
>  static int  console_type_setter (NihOption *option, const char *arg);
> +static int  conf_dir_setter     (NihOption *option, const char *arg);
>
>
>  /**
> @@ -99,12 +100,11 @@
>  static int state_fd = -1;
>
>  /**
> - * conf_dir:
> - *
> - * Full path to job configuration file directory.
> - *
> + * conf_dirs:
> + *
> + * Array of full paths to job configuration file directories.
>   **/
> -static char *conf_dir = NULL;
> +static char **conf_dirs = NULL;
>
>  /**
>   * initial_event:
> @@ -136,7 +136,7 @@
>   **/
>  static NihOption options[] = {
>         { 0, "confdir", N_("specify alternative directory to load configuration files from"),
> -               NULL, "DIR", &conf_dir, NULL },
> +               NULL, "DIR", NULL, conf_dir_setter },
>
>         { 0, "default-console", N_("default value for console stanza"),
>                 NULL, "VALUE", NULL, console_type_setter },
> @@ -185,9 +185,10 @@
>        char *argv[])
>  {
>         char **args = NULL;
> -       char **dirs = NULL;
>         int    ret;
>
> +       conf_dirs = NIH_MUST (nih_str_array_new (NULL));
> +
>         args_copy = NIH_MUST (nih_str_array_copy (NULL, NULL, argv));
>
>         nih_main_init (args_copy[0]);
> @@ -532,19 +533,38 @@
>         }
>
>         /* Read configuration */
> -       if (! user_mode)
> +       if (! user_mode) {
> +               char   *conf_dir;
> +               int     len = 0;
> +
> +               nih_assert (conf_dirs[0]);
> +
> +               /* Count entries */
> +               for (char **d = conf_dirs; d && *d; d++, len++)
> +                       ;
> +
> +               nih_assert (len);
> +
> +               /* Use last value specified */
> +               conf_dir = conf_dirs[len-1];
> +
>                 NIH_MUST (conf_source_new (NULL, CONFFILE, CONF_FILE));
>
> -       if (conf_dir)
> +               nih_debug ("Using configuration directory %s", conf_dir);
>                 NIH_MUST (conf_source_new (NULL, conf_dir, CONF_JOB_DIR));
> +       } else {
> +               nih_local char **dirs = NULL;
>
> -       if (user_mode) {
>                 dirs = NIH_MUST (get_user_upstart_dirs ());
> -               for (char **d = dirs; d && *d; d++)
> +
> +               for (char **d = conf_dirs[0] ? conf_dirs : dirs; d && *d; d++) {
> +                       nih_debug ("Using configuration directory %s", *d);
>                         NIH_MUST (conf_source_new (NULL, *d, CONF_JOB_DIR));
> -               nih_free (dirs);
> +               }
>         }
>
> +       nih_free (conf_dirs);
> +

Why not allow "system" init to have mulple conf_dirs as well?
Or was it intentional to preserve previous behaviour (last one wins)?
I'm not sure it's worth preserving the behaviour, and I think it is
useful to support multiple confdirs even for system init. It has a
usecase in the phablet / cloud scenarios where rootfs can be read-only
and the rest is overlaid in different directories.

E.g. Something like this:

 535        /* Read configuration */
 536        nih_local char **dirs = NULL;
 537        nih_assert (conf_dirs[0]);
 538
 539        if (user_mode)
 540                dirs = NIH_MUST (get_user_upstart_dirs ());
 541        else
 542                NIH_MUST (conf_source_new (NULL, CONFFILE, CONF_FILE));
 543
 544        for (char **d = conf_dirs[0] ? conf_dirs : dirs; d && *d; d++) {
 545                nih_debug ("Using configuration directory %s", *d);
 546                NIH_MUST (conf_source_new (NULL, *d, CONF_JOB_DIR));
 547        }
 548
 549        nih_free (conf_dirs);


> === modified file 'init/man/init.8'
> --- init/man/init.8     2013-02-25 09:42:11 +0000
> +++ init/man/init.8     2013-04-02 10:30:30 +0000
> @@ -65,10 +65,13 @@
>  .\"
>  .TP
>  .B \-\-confdir \fIdirectory\fP
> -Read job configuration files from a directory other than
> -\fI/etc/init\fP.
> -For user session mode, read job configuration files from this
> -directory at highest priority.
> +Read job configuration files from a directory other than the default
> +(\fI/etc/init\fP for process ID 1).
> +
> +When running as process ID 1, the last directory specified will be used.
> +
> +In user session mode, multiple directories will be honoured and job
> +configuration files loaded from the directories in the order specified.

And well then this also becomes more consistent.

-- 
https://code.launchpad.net/~jamesodhunt/upstart/allow-multiple-cmdline-confdirs/+merge/156512
Your team Upstart Reviewers is requested to review the proposed merge of lp:~jamesodhunt/upstart/allow-multiple-cmdline-confdirs into lp:upstart.