[systemd-devel] [PATCH] fstab-generator: add x-systemd.{after, requires-mounts-for}=

[Lennart Poettering]

On Tue, 12.05.15 18:04, Karel Zak (kzak at redhat.com) wrote:

> Currently we have no way how to specify dependencies between fstab
> entries (or another units) in the /etc/fstab. It means that users are
> forced to bypass fstab and write .mount units manually.
> 
> Years ago Lennart suggested to add
> 
> x-systemd.after=<PATH>
> 
>  - to specify dependence an another mount (PATH is translated to <PATH>.mount)
> 
> x-systemd.after=<UNIT>
> 
>  - to specify dependence on arbitrary UNIT
> 
> The x-systemd.after= is implemented by After= and Requires=.

Hmm, I am pretty sure x-systemd.after= should only create After=
dependencies, not Requires=. I'd be open though to add
x-systemd.requires= which adds both After= and Requires=.

(So far we never augmented ordering deps by requirement deps, but
sometimes we do augment requirement deps with ordering deps, for
example in many cases for DefaultDependencies=yes logic. Hence we
should  follow this here, too.)

Hence, please rename this option to "x-systemd.requires=". Otherwise I
like the concept.

> x-systemd.requires-mounts-for=<PATH ...>
> 
>  - to specify dependence on another paths, implemented by
>    RequiresMountsFor=.


Sounds good. 

>        <varlistentry>
> +        <term><option>x-systemd.after=</option></term>
> +
> +        <listitem><para>Configures <varname>After=</varname> and
> <varname>Requires=</varname> dependence

s/dependence/dependency

> +
> +        <listitem><para>Configures <varname>RequiresMountsFor=</varname> dependence between the mount and
> +        another mount. The argument is a space-separated list of absolute paths. Note
> +        that <filename>/etc/fstab</filename> format requires to escape space as \040.
> +        See <varname>RequiresMountsFor=</varname> in
> +        <citerefentry><refentrytitle>systemd.unit</refentrytitle><manvolnum>5</manvolnum></citerefentry>
> +        for details.</para></listitem>

Hmm, I think it would be better to allow specifiying the option
multiple times rather than using spaces to seperate the values...

> +        r = fstab_filter_options(opts, "x-systemd.after\0", NULL, &arg, NULL);
> +        if (r < 0)
> +                return log_warning_errno(r, "Failed to parse options: %m");
> +        if (r == 0)
> +                return 0;
> +
> +        if (*arg == '/') {
> +                r = unit_name_from_path(arg, ".mount", &unit);

I figure this should simply use unit_name_mangle_with_suffix(), which
will do the right thing for device nodes, normal paths and unit
names. No need to manually mangle this...

> +        if (!automount && opts) {
> +                 write_requires_after(f, opts);
> +                 write_requires_mounts_for(f, opts);
> +        }
> +

This should probably propagate errors.

>          if (passno != 0) {
>                  r = generator_write_fsck_deps(f, arg_dest, what, where, fstype);
>                  if (r < 0)
> @@ -315,6 +359,11 @@ static int add_mount(
>                                  "Before=%s\n",
>                                  post);
>  
> +                if (opts) {
> +                        write_requires_after(f, opts);
> +                        write_requires_mounts_for(f, opts);
> +                }

Same here.

Lennart

-- 
Lennart Poettering, Red Hat