[systemd-devel] [PATCH] core/cryptsetup: Add WantsMountFor option to enable fallback to password request for crypt mounts. Signed-off-by: Przemek Rudy <prudy1 at o2.pl>

[Lennart Poettering]

On Sun, 27.04.14 23:46, Przemek Rudy (prudy1 at o2.pl) wrote:

> 
> This patch is a proposal for a problem with not falling back to password request
> if the device with unlocking key for crypt volumes is not mounted for
> defined time.

Can you elaborate on the usecase? I mean, this would still result in in
90s timeout, right? Or what's your idea here?

> +int config_parse_unit_wants_mounts_for(
> +                const char *unit,
> +                const char *filename,
> +                unsigned line,
> +                const char *section,
> +                unsigned section_line,
> +                const char *lvalue,
> +                int ltype,
> +                const char *rvalue,
> +                void *data,
> +                void *userdata) {
> +}

Sounds like a call to unify with
config_parse_unit_requires_mounts_for(), and use the "ltype" param to
distuingish them.

> +        /* Adds in links to other units that use (want) this path or paths
> +         * further down in the hierarchy */
> +        s = manager_get_units_want_mounts_for(UNIT(m)->manager, m->where);
> +        SET_FOREACH(other, s, i) {
> +
> +                if (other->load_state != UNIT_LOADED)
> +                        continue;
> +
> +                if (other == UNIT(m))
> +                        continue;
> +
> +                r = unit_add_dependency(other, UNIT_AFTER, UNIT(m), true);
> +                if (r < 0)
> +                        return r;
> +
> +                if (UNIT(m)->fragment_path) {
> +                        /* If we have fragment configuration, then make this dependency required */
> +                        r = unit_add_dependency(other, UNIT_WANTS, UNIT(m), true);
> +                        if (r < 0)
> +                                return r;
> +                }
> +        }
> +

Something to unify with the "requires" codepath. For example, it should
be easy to turn most of the loop's body into a function of its own, that
could then  be reused...

> +static void unit_free_wants_mounts_for(Unit *u) {

Same here..

Please, let's not let turn this into a game of copy/paste...

> +        if (!strv_isempty(u->wants_mounts_for)) {
> +                fprintf(f,
> +                        "%s\tWantsMountsFor:", prefix);
> +
> +                STRV_FOREACH(j, u->wants_mounts_for)
> +                        fprintf(f, " %s", *j);
> +
> +                fputs("\n", f);
> +        }
> +

Same here...

> +        STRV_FOREACH(i, u->wants_mounts_for) {
> +                char prefix[strlen(*i) + 1];
> +
> +                PATH_FOREACH_PREFIX_MORE(prefix, *i) {
> +                        Unit *m;
> +
> +                        r = manager_get_unit_by_path(u->manager, prefix, ".mount", &m);
> +                        if (r < 0)
> +                                return r;
> +                        if (r == 0)
> +                                continue;
> +                        if (m == u)
> +                                continue;
> +
> +                        if (m->load_state != UNIT_LOADED)
> +                                continue;
> +
> +                        r = unit_add_dependency(u, UNIT_AFTER, m, true);
> +                        if (r < 0)
> +                                return r;
> +
> +                        if (m->fragment_path) {
> +                                r = unit_add_dependency(u, UNIT_WANTS, m, true);
> +                                if (r < 0)
> +                                        return r;
> +                        }
> +                }
> +        }
> +

Not funny anymore.

>          return 0;
>  }
>  
> @@ -3289,6 +3357,82 @@ int unit_require_mounts_for(Unit *u, const char *path) {
>          return 0;
>  }
>  
> +int unit_want_mounts_for(Unit *u, const char *path) {

Totally not funny anymore...

Lennart

-- 
Lennart Poettering, Red Hat