[systemd-devel] Patch: fix generator for cryptoloop

[Lennart Poettering]

On Wed, 14.03.12 14:51, Frederic Crozat (fcrozat at suse.com) wrote:

> -        char *p = NULL, *n = NULL, *d = NULL, *u = NULL, *from = NULL, *to = NULL, *e = NULL;
> +        char *p = NULL, *n = NULL, *d = NULL, *u = NULL, *from = NULL, *to = NULL, *e = NULL, *path_file = NULL;
>          int r;
>          FILE *f = NULL;
>          bool noauto, nofail;
> @@ -93,10 +93,50 @@ static int create_disk(
>                  goto fail;
>          }
>  
> -        if (!(d = unit_name_from_path(u, ".device"))) {
> -                r = -ENOMEM;
> -                log_error("Failed to allocate device name.");
> -                goto fail;
> +        if (!startswith(device,"/dev/")) {
> +
> +                if (!(d = unit_name_build_escape("cryptsetup", name, ".path"))) {
> +                        r = -ENOMEM;
> +                        log_error("Failed to allocate path name.");
> +                        goto fail;
> +                }

For new code we try to write

    abc = foobar()
    if (abc) {

rather than

    if ((abc = foobar())) {

Hmm, but I am not really sure we should be using a .path unit here,
since it does more than what we probably want here. i.e. this will wait
until the file exists, and as long as it doesn't not warn at all about
that. In most other cases where we have a dependency on a file we
implement that with normal dependencies, which means that if a file
doesn't show up you will eventually get an error, which is probably
desirable.

Maybe the solution here is to introduce something like NeedMounts= or so
in [Unit] which takes a list of paths, and is just a short way to create
After= and Requires= dependencies to all .mount units needed for those
paths. We currently have explicit code for this in
socket_add_mount_links(), path_add_mount_links() and so on. Maybe we
should simplify that by generilizing it, and make this a general feature
for units, instead of having per-unit code for this.

I think doing that could save us a lot of code, and make what we already
have simpler and shorter. And then, for the cryptsetup code we'd just
make use of NeedsMounts= and things would be pretty and delicious ;-)

I have now added this to the TODO list, and will implement this
eventually. But I am of course also very happy to take a patch for that,
especially if you need this timely. This should probably be four steps:

1) introduce the backend for NeedMounts=

2) port the old socket/path/automount/... code to this new logic

3) expose NeedMounts= actually in config files

4) oneline change to the cryptsetup generator to make of this feature

Lennart

-- 
Lennart Poettering - Red Hat, Inc.