Merge 3a0ec74049 into a526b9ddfc

Some generators would ignore the value and warn, possibly logging twice. Other
generators would treat the failure as fatal, even though it probably doesn't
matter too much if drop-in with the timeout is not written.

fstab_filter_options() can only fail with -ENOMEM, which we want to treat as
a fatal error, so use log_error and drop ", ignoring".

Simplify the function to always log uniformly and simplify the callers.

src/cryptsetup/cryptsetup-generator.c

@@ -518,9 +518,9 @@ static int create_disk(
                         "After=modprobe@loop.service\n",
                         u_escaped);
 
-        r = generator_write_timeouts(arg_dest, device, name, options, &filtered);
+        r = generator_write_timeouts(arg_dest, device, options, &filtered);
         if (r < 0)
-                log_warning_errno(r, "Failed to write device timeout drop-in: %m");
+                return r;
 
         r = generator_write_cryptsetup_service_section(f, name, u, key_file, filtered);
         if (r < 0)

src/fstab-generator/fstab-generator.c

@@ -288,8 +288,7 @@ static int add_swap(
         if (r < 0)
                 return log_error_errno(r, "Failed to write unit file %s: %m", name);
 
-        /* use what as where, to have a nicer error message */
+        r = generator_write_timeouts(arg_dest, what, options, NULL);
-        r = generator_write_timeouts(arg_dest, what, what, options, NULL);
         if (r < 0)
                 return r;
 
@@ -589,6 +588,13 @@ static int add_mount(
         if (r < 0)
                 return log_error_errno(r, "Failed to generate unit name: %m");
 
+        /* Write timeout dropin and get filtered options */
+        r = generator_write_timeouts(dest, what, opts, &filtered);
+        if (r < 0)
+                return r;
+
+        /* Write main fragment */
+
         r = generator_open_unit_file(dest, source, name, &f);
         if (r < 0)
                 return r;
@@ -670,20 +676,6 @@ static int add_mount(
                 fprintf(f, "Type=%s\n", t);
         }
 
-        r = generator_write_timeouts(dest, what, where, opts, &filtered);
-        if (r < 0)
-                return r;
-
-        r = generator_write_device_deps(dest, what, where, opts);
-        if (r < 0)
-                return r;
-
-        if (in_initrd() && path_equal(where, "/sysroot") && is_device_path(what)) {
-                r = generator_write_initrd_root_device_deps(dest, what);
-                if (r < 0)
-                        return r;
-        }
-
         r = write_mount_timeout(f, where, opts);
         if (r < 0)
                 return r;
@@ -699,6 +691,18 @@ static int add_mount(
         if (r < 0)
                 return log_error_errno(r, "Failed to write unit file %s: %m", name);
 
+        /* Write other drop-ins */
+
+        r = generator_write_device_deps(dest, what, where, opts);
+        if (r < 0)
+                return r;
+
+        if (in_initrd() && path_equal(where, "/sysroot") && is_device_path(what)) {
+                r = generator_write_initrd_root_device_deps(dest, what);
+                if (r < 0)
+                        return r;
+        }
+
         if (flags & MOUNT_MAKEFS) {
                 r = generator_hook_up_mkfs(dest, what, where, fstype);
                 if (r < 0)

src/hibernate-resume/hibernate-resume-generator.c

@@ -76,12 +76,11 @@ static int process_resume(const HibernateInfo *info) {
                 log_warning_errno(r, "Failed to write device timeout drop-in, ignoring: %m");
 
         r = generator_write_timeouts(arg_dest,
-                                     info->device,
                                      info->device,
                                      arg_resume_options ?: arg_root_options,
                                      NULL);
         if (r < 0)
-                log_warning_errno(r, "Failed to write device timeout drop-in, ignoring: %m");
+                return r;
 
         r = write_drop_in_format(arg_dest, SPECIAL_HIBERNATE_RESUME_SERVICE, 90, "device-dependency",
                                  "# Automatically generated by systemd-hibernate-resume-generator\n\n"

src/shared/generator.c

@@ -371,31 +371,31 @@ int generator_write_fsck_deps(
 int generator_write_timeouts(
                 const char *dir,
                 const char *what,
-                const char *where,
                 const char *opts,
-                char **filtered) {
+                char **ret_filtered) {
 
-        /* Configure how long we wait for a device that backs a mount point or a
+        /* Configure how long we wait for a device that backs a mount point or a swap partition to
-         * swap partition to show up. This is useful to support endless device timeouts
+         * show up. This is useful to support endless device timeouts for devices that show up only
-         * for devices that show up only after user input, like crypto devices. */
+         * after user input, like crypto devices.
+         */
 
         _cleanup_free_ char *node = NULL, *unit = NULL, *timeout = NULL;
         usec_t u;
         int r;
 
+        assert(what);
+
         r = fstab_filter_options(opts, "comment=systemd.device-timeout\0"
                                        "x-systemd.device-timeout\0",
-                                 NULL, &timeout, NULL, filtered);
+                                 NULL, &timeout, NULL, ret_filtered);
-        if (r < 0) {
+        if (r < 0)
-                log_warning_errno(r, "Failed to parse fstab options, ignoring: %m");
+                return log_error_errno(r, "%s: failed to parse fstab options: %m", what);
-                return 0;
-        }
         if (r == 0)
                 return 0;
 
         r = parse_sec_fix_0(timeout, &u);
         if (r < 0) {
-                log_warning("Failed to parse timeout for %s, ignoring: %s", where, timeout);
+                log_warning_errno(r, "%s: failed to parse timeout, ignoring: %s", what, timeout);
                 return 0;
         }
 
@@ -403,15 +403,17 @@ int generator_write_timeouts(
         if (!node)
                 return log_oom();
         if (!is_device_path(node)) {
-                log_warning("x-systemd.device-timeout ignored for %s", what);
+                log_warning("x-systemd.device-timeout ignored for %s.", what);
                 return 0;
         }
 
         r = unit_name_from_path(node, ".device", &unit);
-        if (r < 0)
+        if (r < 0) {
-                return log_error_errno(r, "Failed to make unit name from path: %m");
+                log_error_errno(r, "%s: failed to make unit name from path '%s', ignoring: %m", what, node);
+                return 0;
+        }
 
-        return write_drop_in_format(dir, unit, 50, "device-timeout",
+        r = write_drop_in_format(dir, unit, 50, "device-timeout",
                                  "# Automatically generated by %s\n"
                                  "# from supplied options \"%s\"\n\n"
                                  "[Unit]\n"
@@ -419,6 +421,9 @@ int generator_write_timeouts(
                                  program_invocation_short_name,
                                  opts,
                                  timeout);
+        if (r < 0)
+                log_warning_errno(r, "%s: failed to write drop-in with device timeout, ignoring: %m", what);
+        return 0;
 }
 
 int generator_write_device_deps(

src/shared/generator.h

@@ -28,9 +28,8 @@ int generator_write_fsck_deps(
 int generator_write_timeouts(
         const char *dir,
         const char *what,
-        const char *where,
         const char *opts,
-        char **filtered);
+        char **ret_filtered);
 
 int generator_write_blockdev_dependency(
                 FILE *f,