[systemd-devel] [PATCH] cryptsetup-generator: auto add deps for device as password

[Dave Reisner]

On Sat, Feb 08, 2014 at 07:31:28PM +0100, Zbigniew Jędrzejewski-Szmek wrote:
> On Sat, Feb 08, 2014 at 01:12:14PM -0500, Dave Reisner wrote:
> > If the password is a device file, we can add Requires/After dependencies
> > on the device rather than requiring the user to do so.
> > ---
> > This is based on a "bug" filed to Arch:
> > 
> > https://bugs.archlinux.org/task/38842
> > 
> > Assuming I'm correct about the race condition, this should be an easy way
> > of closing it without user involvement.
> > 
> >  src/cryptsetup/cryptsetup-generator.c | 25 ++++++++++++++++++++-----
> >  1 file changed, 20 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c
> > index 9c98f0b..4542757 100644
> > --- a/src/cryptsetup/cryptsetup-generator.c
> > +++ b/src/cryptsetup/cryptsetup-generator.c
> > @@ -130,11 +130,26 @@ static int create_disk(
> >                      streq(password, "/dev/random") ||
> >                      streq(password, "/dev/hw_random"))
> >                          fputs("After=systemd-random-seed.service\n", f);
> > -                else if (!streq(password, "-") &&
> > -                         !streq(password, "none"))
> > -                        fprintf(f,
> > -                                "RequiresMountsFor=%s\n",
> > -                                password);
> > +                else {
> > +                        _cleanup_free_ char *uu = fstab_node_to_udev_node(password);
> > +                        if (uu == NULL)
> > +                                return log_oom();
> > +
> > +                        if (is_device_path(uu)) {
> > +                                _cleanup_free_ char *dd = unit_name_from_path(uu, ".device");
> > +                                if (dd == NULL)
> > +                                        return log_oom();
> > +
> > +                                fprintf(f,
> > +                                                "After=%s\n"
> > +                                                "Requires=%s\n",
> > +                                                dd, dd);
> > +                        } else if (!streq(password, "-") &&
> > +                                         !streq(password, "none"))
> > +                                        fprintf(f,
> > +                                                "RequiresMountsFor=%s\n",
> > +                                                password);
> > +                }
> Patch looks fine, but I don't really understand why you invert the order of conditions.
> Something like this feels more natural:

Yeah, not sure what I was thinking here. Thanks for the suggestion --
will push with this change.

>                 else if (!streq(password, "-") &&
>                          !streq(password, "none")) {
> 
>                         _cleanup_free_ char *uu = fstab_node_to_udev_node(password);
>                         if (uu == NULL)
>                                 return log_oom();
> 
>                         if (is_device_path(uu)) {
>                                 _cleanup_free_ char *dd = unit_name_from_path(uu, ".device");
>                                 if (dd == NULL)
>                                         return log_oom();
> 
>                                 fprintf(f, "After=%1$s\nRequires=%1$s\n", dd);
>                         } else
> 				fprintf(f, "RequiresMountsFor=%s\n", password);
>                 }
> 
> Zbyszek