[systemd-devel] [PATCH] cryptsetup-generator: Add JobTimeoutSec=0 for all known crypt devices

[h***@redhat.com]

From: Harald Hoyer <***@redhat.com>

put JobTimeoutSec=0 in <device-unit>.d/JobTimeoutSec0.conf files

This helps with grabbing a cup of coffee while booting and not have the
crypto password dialog timeout and systemd in a failed state.

This is even needed for "timeout=0" in /etc/crypttab!
---
src/cryptsetup/cryptsetup-generator.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)

diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c
index 6b9bc55..5ca9529 100644
--- a/src/cryptsetup/cryptsetup-generator.c
+++ b/src/cryptsetup/cryptsetup-generator.c
@@ -213,6 +213,35 @@ static int create_disk(
return -errno;
}

+ if (!noauto && !nofail) {
+ free(p);
+ p = strjoin(arg_dest, "/dev-mapper-", e, ".device.d/50-JobTimeoutSec0.conf", NULL);
+ if (!p)
+ return log_oom();
+
+ mkdir_parents_label(p, 0755);
+
+ if (f)
+ fclose(f);
+ f = fopen(p, "wxe");
+ if (!f) {
+ log_error("Failed to create unit file %s: %m", p);
+ return -errno;
+ }
+
+ fprintf(f,
+ "# Automatically generated by systemd-cryptsetup-generator\n\n"
+ "[Unit]\n"
+ "JobTimeoutSec=0\n" /* the binary handles timeouts anyway */
+ );
+ fflush(f);
+
+ if (ferror(f)) {
+ log_error("Failed to write file %s: %m", p);
+ return -errno;
+ }
+ }
+
return 0;
}

[Lennart Poettering]

Post by h***@redhat.com
put JobTimeoutSec=0 in <device-unit>.d/JobTimeoutSec0.conf files
This helps with grabbing a cup of coffee while booting and not have the
crypto password dialog timeout and systemd in a failed state.
This is even needed for "timeout=0" in /etc/crypttab!

Well, this explanation is too short. You need to clarify that this only
has an effect on devices listed in fstab with their device mapper path
(i.e. rather than LABEL= or UUID=, which is by far more common). For the
generic case there is no way around setting x-systemd.device-timeout=0
in fstab, because we wouldn't know that the fs with that label or uuid
is hidden behind a certain encrypted device before we have decrypted
it.

This really needs to be clarified in both the commit msg and in the
comments of the code.

Post by h***@redhat.com
+ p = strjoin(arg_dest, "/dev-mapper-", e, ".device.d/50-JobTimeoutSec0.conf", NULL);

We currently don't really use CamelCase file names, would prefer to call
this 50-job-timeout-sec-0.conf or so.

Also, please use write_string_file(), this should shorten the code quite
a bit.

Lennart

[Harald Hoyer]

Post by Lennart Poettering

Post by h***@redhat.com
put JobTimeoutSec=0 in <device-unit>.d/JobTimeoutSec0.conf files
This helps with grabbing a cup of coffee while booting and not have the
crypto password dialog timeout and systemd in a failed state.
This is even needed for "timeout=0" in /etc/crypttab!

Well, this explanation is too short. You need to clarify that this only
has an effect on devices listed in fstab with their device mapper path
(i.e. rather than LABEL= or UUID=, which is by far more common). For the
generic case there is no way around setting x-systemd.device-timeout=0
in fstab, because we wouldn't know that the fs with that label or uuid
is hidden behind a certain encrypted device before we have decrypted
it.

This is not true. This also affects LVM on top of the cryptsetup devices, where
none of the /dev/mapper devices is and can be in /etc/fstab.

Post by Lennart Poettering
This really needs to be clarified in both the commit msg and in the
comments of the code.

Post by h***@redhat.com
+ p = strjoin(arg_dest, "/dev-mapper-", e, ".device.d/50-JobTimeoutSec0.conf", NULL);

We currently don't really use CamelCase file names, would prefer to call
this 50-job-timeout-sec-0.conf or so.

ok

Post by Lennart Poettering
Also, please use write_string_file(), this should shorten the code quite
a bit.

err.. I just used the same style like in the rest of the file.

Post by Lennart Poettering
Lennart

[h***@redhat.com]

From: Harald Hoyer <***@redhat.com>

The password query for a crypto device currently times out after 90s,
which is too short to grab a cup of coffee when a machine boots up.

The resulting decrypted device /dev/mapper/luks-<uuid> might not
be a mountpoint (but part of a LVM PV or raid array)
and therefore the timeout cannot be controlled by the settings
in /etc/fstab. For this reason this device should not carry its own timeout.

Also the encrypted device /dev/disk/by-*/* already has a timeout and
additionally the timeout for the password query is set in /etc/crypttab.

This patch disables the timeout of the resulting decrypted devices by creating
<device-unit>.d/50-job-timeout-sec-0.conf files with "JobTimeoutSec=0".
---
src/cryptsetup/cryptsetup-generator.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)

diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c
index 6b9bc55..33604f7 100644
--- a/src/cryptsetup/cryptsetup-generator.c
+++ b/src/cryptsetup/cryptsetup-generator.c
@@ -213,6 +213,35 @@ static int create_disk(
return -errno;
}

+ if (!noauto && !nofail) {
+ free(p);
+ p = strjoin(arg_dest, "/dev-mapper-", e, ".device.d/50-job-timeout-sec-0.conf", NULL);
+ if (!p)
+ return log_oom();
+
+ mkdir_parents_label(p, 0755);
+
+ if (f)
+ fclose(f);
+ f = fopen(p, "wxe");
+ if (!f) {
+ log_error("Failed to create unit file %s: %m", p);
+ return -errno;
+ }
+
+ fprintf(f,
+ "# Automatically generated by systemd-cryptsetup-generator\n\n"
+ "[Unit]\n"
+ "JobTimeoutSec=0\n" /* the binary handles timeouts anyway */
+ );
+ fflush(f);
+
+ if (ferror(f)) {
+ log_error("Failed to write file %s: %m", p);
+ return -errno;
+ }
+ }
+
return 0;
}

[Lennart Poettering]

Post by Harald Hoyer

Post by Lennart Poettering
Well, this explanation is too short. You need to clarify that this only
has an effect on devices listed in fstab with their device mapper path
(i.e. rather than LABEL= or UUID=, which is by far more common). For the
generic case there is no way around setting x-systemd.device-timeout=0
in fstab, because we wouldn't know that the fs with that label or uuid
is hidden behind a certain encrypted device before we have decrypted
it.

This is not true. This also affects LVM on top of the cryptsetup devices, where
none of the /dev/mapper devices is and can be in /etc/fstab.

Well, LVM does not care about proper dependencies. They still are so
naive to assume they are run at a point in time where all devices are
probed.

Whether or not .device units exist is pretty much ignored by LVM.

Post by Harald Hoyer

Post by Lennart Poettering
This really needs to be clarified in both the commit msg and in the
comments of the code.

Post by h***@redhat.com
+ p = strjoin(arg_dest, "/dev-mapper-", e, ".device.d/50-JobTimeoutSec0.conf", NULL);

We currently don't really use CamelCase file names, would prefer to call
this 50-job-timeout-sec-0.conf or so.

ok

Post by Lennart Poettering
Also, please use write_string_file(), this should shorten the code quite
a bit.

err.. I just used the same style like in the rest of the file.

Well, not an excuse to make the same mistake... ;-)

Lennart

[h***@redhat.com]

From: Harald Hoyer <***@redhat.com>

The password query for a crypto device currently times out after 90s,
which is too short to grab a cup of coffee when a machine boots up.

The resulting decrypted device /dev/mapper/luks-<uuid> might not
be a mountpoint (but part of a LVM PV or raid array)
and therefore the timeout cannot be controlled by the settings
in /etc/fstab. For this reason this device should not carry its own timeout.

Also the encrypted device /dev/disk/by-*/* already has a timeout and
additionally the timeout for the password query is set in /etc/crypttab.

This patch disables the timeout of the resulting decrypted devices by creating
<device-unit>.d/50-job-timeout-sec-0.conf files with "JobTimeoutSec=0".
---
src/cryptsetup/cryptsetup-generator.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c
index 6b9bc55..fd2080b 100644
--- a/src/cryptsetup/cryptsetup-generator.c
+++ b/src/cryptsetup/cryptsetup-generator.c
@@ -213,6 +213,23 @@ static int create_disk(
return -errno;
}

+ if (!noauto && !nofail) {
+ int r;
+ free(p);
+ p = strjoin(arg_dest, "/dev-mapper-", e, ".device.d/50-job-timeout-sec-0.conf", NULL);
+ if (!p)
+ return log_oom();
+
+ mkdir_parents_label(p, 0755);
+
+ r = write_string_file(p,
+ "# Automatically generated by systemd-cryptsetup-generator\n\n"
+ "[Unit]\n"
+ "JobTimeoutSec=0\n"); /* the binary handles timeouts anyway */
+ if (r)
+ return r;
+ }
+
return 0;
}

[Harald Hoyer]

Post by h***@redhat.com
The password query for a crypto device currently times out after 90s,
which is too short to grab a cup of coffee when a machine boots up.
The resulting decrypted device /dev/mapper/luks-<uuid> might not
be a mountpoint (but part of a LVM PV or raid array)
and therefore the timeout cannot be controlled by the settings
in /etc/fstab. For this reason this device should not carry its own timeout.
Also the encrypted device /dev/disk/by-*/* already has a timeout and
additionally the timeout for the password query is set in /etc/crypttab.
This patch disables the timeout of the resulting decrypted devices by creating
<device-unit>.d/50-job-timeout-sec-0.conf files with "JobTimeoutSec=0".
---
src/cryptsetup/cryptsetup-generator.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c
index 6b9bc55..fd2080b 100644
--- a/src/cryptsetup/cryptsetup-generator.c
+++ b/src/cryptsetup/cryptsetup-generator.c
@@ -213,6 +213,23 @@ static int create_disk(
return -errno;
}
+ if (!noauto && !nofail) {
+ int r;
+ free(p);
+ p = strjoin(arg_dest, "/dev-mapper-", e, ".device.d/50-job-timeout-sec-0.conf", NULL);
+ if (!p)
+ return log_oom();
+
+ mkdir_parents_label(p, 0755);
+
+ r = write_string_file(p,
+ "# Automatically generated by systemd-cryptsetup-generator\n\n"
+ "[Unit]\n"
+ "JobTimeoutSec=0\n"); /* the binary handles timeouts anyway */
+ if (r)
+ return r;
+ }
+
return 0;
}

pushed to master

[Thomas Bächler]

Post by Lennart Poettering

Post by Harald Hoyer

Post by Lennart Poettering
Well, this explanation is too short. You need to clarify that this only
has an effect on devices listed in fstab with their device mapper path
(i.e. rather than LABEL= or UUID=, which is by far more common). For the
generic case there is no way around setting x-systemd.device-timeout=0
in fstab, because we wouldn't know that the fs with that label or uuid
is hidden behind a certain encrypted device before we have decrypted
it.

This is not true. This also affects LVM on top of the cryptsetup devices, where
none of the /dev/mapper devices is and can be in /etc/fstab.

Well, LVM does not care about proper dependencies. They still are so
naive to assume they are run at a point in time where all devices are
probed.

That is untrue. LVM can be enabled entirely from udev if it is so
configured.

Post by Lennart Poettering
Whether or not .device units exist is pretty much ignored by LVM.

That's true, LVM cannot hook into systemd,

[Harald Hoyer]

Post by Thomas Bächler

Post by Lennart Poettering

Post by Harald Hoyer

Post by Lennart Poettering
Well, this explanation is too short. You need to clarify that this
only has an effect on devices listed in fstab with their device
mapper path (i.e. rather than LABEL= or UUID=, which is by far more
common). For the generic case there is no way around setting
x-systemd.device-timeout=0 in fstab, because we wouldn't know that
the fs with that label or uuid is hidden behind a certain encrypted
device before we have decrypted it.

This is not true. This also affects LVM on top of the cryptsetup
devices, where none of the /dev/mapper devices is and can be in
/etc/fstab.

Well, LVM does not care about proper dependencies. They still are so
naive to assume they are run at a point in time where all devices are
probed.

That is untrue. LVM can be enabled entirely from udev if it is so
configured.

This is exactly what is going on in dracut and because we don't know the
physical volumes, we cannot create systemd units with dependencies.