Merge pull request #5164 from Werkov/ordering-for-_netdev-devices

Ordering for _netdev devices

man/systemd.unit.xml

@@ -718,17 +718,20 @@
 
       <varlistentry>
         <term><varname>JobTimeoutSec=</varname></term>
+        <term><varname>JobRunningTimeoutSec=</varname></term>
         <term><varname>JobTimeoutAction=</varname></term>
         <term><varname>JobTimeoutRebootArgument=</varname></term>
 
-        <listitem><para>When a job for this unit is queued, a time-out may be configured. If this time limit is
-        reached, the job will be cancelled, the unit however will not change state or even enter the
-        <literal>failed</literal> mode. This value defaults to <literal>infinity</literal> (job timeouts disabled),
-        except for device units. NB: this timeout is independent from any unit-specific timeout (for example, the
-        timeout set with <varname>TimeoutStartSec=</varname> in service units) as the job timeout has no effect on the
-        unit itself, only on the job that might be pending for it. Or in other words: unit-specific timeouts are useful
-        to abort unit state changes, and revert them. The job timeout set with this option however is useful to abort
-        only the job waiting for the unit state to change.</para>
+        <listitem><para>When a job for this unit is queued, a time-out <varname>JobTimeoutSec=</varname> may be
+        configured. Similarly, <varname>JobRunningTimeoutSec=</varname> starts counting when the queued job is actually
+        started. If either time limit is reached, the job will be cancelled, the unit however will not change state or
+        even enter the <literal>failed</literal> mode. This value defaults to <literal>infinity</literal> (job timeouts
+        disabled), except for device units (<varname>JobRunningTimeoutSec=</varname> defaults to
+        <varname>DefaultTimeoutStartSec=</varname>). NB: this timeout is independent from any unit-specific timeout
+        (for example, the timeout set with <varname>TimeoutStartSec=</varname> in service units) as the job timeout has
+        no effect on the unit itself, only on the job that might be pending for it. Or in other words: unit-specific
+        timeouts are useful to abort unit state changes, and revert them. The job timeout set with this option however
+        is useful to abort only the job waiting for the unit state to change.</para>
 
         <para><varname>JobTimeoutAction=</varname>
         optionally configures an additional

src/core/dbus-unit.c

@@ -748,6 +748,7 @@ const sd_bus_vtable bus_unit_vtable[] = {
         SD_BUS_PROPERTY("IgnoreOnIsolate", "b", bus_property_get_bool, offsetof(Unit, ignore_on_isolate), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("NeedDaemonReload", "b", property_get_need_daemon_reload, 0, SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("JobTimeoutUSec", "t", bus_property_get_usec, offsetof(Unit, job_timeout), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("JobRunningTimeoutUSec", "t", bus_property_get_usec, offsetof(Unit, job_running_timeout), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("JobTimeoutAction", "s", property_get_emergency_action, offsetof(Unit, job_timeout_action), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("JobTimeoutRebootArgument", "s", NULL, offsetof(Unit, job_timeout_reboot_arg), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("ConditionResult", "b", bus_property_get_bool, offsetof(Unit, condition_result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),

src/core/device.c

@@ -112,7 +112,7 @@ static void device_init(Unit *u) {
          * indefinitely for plugged in devices, something which cannot
          * happen for the other units since their operations time out
          * anyway. */
-        u->job_timeout = u->manager->default_timeout_start_usec;
+        u->job_running_timeout = u->manager->default_timeout_start_usec;
 
         u->ignore_on_isolate = true;
 }

src/core/job.c

@@ -576,6 +576,7 @@ int job_run_and_invalidate(Job *j) {
         if (!job_is_runnable(j))
                 return -EAGAIN;
 
+        job_start_timer(j, true);
         job_set_state(j, JOB_RUNNING);
         job_add_to_dbus_queue(j);
 
@@ -949,22 +950,45 @@ static int job_dispatch_timer(sd_event_source *s, uint64_t monotonic, void *user
         return 0;
 }
 
-int job_start_timer(Job *j) {
+int job_start_timer(Job *j, bool job_running) {
         int r;
+        usec_t run_begin, timeout_time, old_timeout_time;
 
-        if (j->timer_event_source)
-                return 0;
+        if (job_running) {
+                if (j->unit->job_running_timeout == USEC_INFINITY)
+                        return 0;
 
-        j->begin_usec = now(CLOCK_MONOTONIC);
+                run_begin = now(CLOCK_MONOTONIC);
+                timeout_time = usec_add(run_begin, j->unit->job_running_timeout);
 
-        if (j->unit->job_timeout == USEC_INFINITY)
-                return 0;
+                if (j->timer_event_source) {
+                        /* Update only if JobRunningTimeoutSec= results in earlier timeout */
+                        r = sd_event_source_get_time(j->timer_event_source, &old_timeout_time);
+                        if (r < 0)
+                                return r;
+
+                        if (old_timeout_time <= timeout_time)
+                                return 0;
+
+                        return sd_event_source_set_time(j->timer_event_source, timeout_time);
+                }
+        } else {
+                if (j->timer_event_source)
+                        return 0;
+
+                j->begin_usec = now(CLOCK_MONOTONIC);
+
+                if (j->unit->job_timeout == USEC_INFINITY)
+                        return 0;
+
+                timeout_time = usec_add(j->begin_usec, j->unit->job_timeout);
+        }
 
         r = sd_event_add_time(
                         j->manager->event,
                         &j->timer_event_source,
                         CLOCK_MONOTONIC,
-                        usec_add(j->begin_usec, j->unit->job_timeout), 0,
+                        timeout_time, 0,
                         job_dispatch_timer, j);
         if (r < 0)
                 return r;

src/core/job.h

@@ -220,7 +220,7 @@ int job_type_merge_and_collapse(JobType *a, JobType b, Unit *u);
 void job_add_to_run_queue(Job *j);
 void job_add_to_dbus_queue(Job *j);
 
-int job_start_timer(Job *j);
+int job_start_timer(Job *j, bool job_running);
 
 int job_run_and_invalidate(Job *j);
 int job_finish_and_invalidate(Job *j, JobResult result, bool recursive, bool already);

src/core/load-fragment-gperf.gperf.m4

@@ -194,6 +194,7 @@ Unit.OnFailureIsolate,           config_parse_job_mode_isolate,      0,
 Unit.IgnoreOnIsolate,            config_parse_bool,                  0,                             offsetof(Unit, ignore_on_isolate)
 Unit.IgnoreOnSnapshot,           config_parse_warn_compat,           DISABLED_LEGACY,               0
 Unit.JobTimeoutSec,              config_parse_sec_fix_0,             0,                             offsetof(Unit, job_timeout)
+Unit.JobRunningTimeoutSec,       config_parse_sec,                   0,                             offsetof(Unit, job_running_timeout)
 Unit.JobTimeoutAction,           config_parse_emergency_action,      0,                             offsetof(Unit, job_timeout_action)
 Unit.JobTimeoutRebootArgument,   config_parse_unit_string_printf,    0,                             offsetof(Unit, job_timeout_reboot_arg)
 Unit.StartLimitIntervalSec,      config_parse_sec,                   0,                             offsetof(Unit, start_limit.interval)

src/core/transaction.c

@@ -632,7 +632,7 @@ static int transaction_apply(Transaction *tr, Manager *m, JobMode mode) {
 
                 job_add_to_run_queue(j);
                 job_add_to_dbus_queue(j);
-                job_start_timer(j);
+                job_start_timer(j, false);
                 job_shutdown_magic(j);
         }
 

src/core/unit.c

@@ -99,6 +99,7 @@ Unit *unit_new(Manager *m, size_t size) {
         u->on_failure_job_mode = JOB_REPLACE;
         u->cgroup_inotify_wd = -1;
         u->job_timeout = USEC_INFINITY;
+        u->job_running_timeout = USEC_INFINITY;
         u->ref_uid = UID_INVALID;
         u->ref_gid = GID_INVALID;
         u->cpu_usage_last = NSEC_INFINITY;
@@ -1336,6 +1337,9 @@ int unit_load(Unit *u) {
                         goto fail;
                 }
 
+                if (u->job_running_timeout != USEC_INFINITY && u->job_running_timeout > u->job_timeout)
+                        log_unit_warning(u, "JobRunningTimeoutSec= is greater than JobTimeoutSec=, it has no effect.");
+
                 unit_update_cgroup_members_masks(u);
         }
 

src/core/unit.h

@@ -114,6 +114,7 @@ struct Unit {
 
         /* Job timeout and action to take */
         usec_t job_timeout;
+        usec_t job_running_timeout;
         EmergencyAction job_timeout_action;
         char *job_timeout_reboot_arg;
 

src/fstab-generator/fstab-generator.c

@@ -399,6 +399,10 @@ static int add_mount(
         if (r < 0)
                 return r;
 
+        r = generator_write_device_deps(dest, what, where, opts);
+        if (r < 0)
+                return r;
+
         r = write_mount_timeout(f, where, opts);
         if (r < 0)
                 return r;

src/shared/generator.c

@@ -188,10 +188,49 @@ int generator_write_timeouts(
 
         return write_drop_in_format(dir, unit, 50, "device-timeout",
                                     "# Automatically generated by %s\n\n"
-                                    "[Unit]\nJobTimeoutSec=%s",
+                                    "[Unit]\nJobRunningTimeoutSec=%s",
                                     program_invocation_short_name, timeout);
 }
 
+int generator_write_device_deps(
+                const char *dir,
+                const char *what,
+                const char *where,
+                const char *opts) {
+
+        /* fstab records that specify _netdev option should apply the network
+         * ordering on the actual device depending on network connection. If we
+         * are not mounting real device (NFS, CIFS), we rely on _netdev effect
+         * on the mount unit itself. */
+
+        _cleanup_free_ char *node = NULL, *unit = NULL;
+        int r;
+
+        if (!fstab_test_option(opts, "_netdev\0"))
+                return 0;
+
+        node = fstab_node_to_udev_node(what);
+        if (!node)
+                return log_oom();
+
+        /* Nothing to apply dependencies to. */
+        if (!is_device_path(node))
+                return 0;
+
+        r = unit_name_from_path(node, ".device", &unit);
+        if (r < 0)
+                return log_error_errno(r, "Failed to make unit name from path: %m");
+
+        /* See mount_add_default_dependencies for explanation why we create such
+         * dependencies. */
+        return write_drop_in_format(dir, unit, 50, "netdev-dependencies",
+                                    "# Automatically generated by %s\n\n"
+                                    "[Unit]\n"
+                                    "After=" SPECIAL_NETWORK_ONLINE_TARGET " " SPECIAL_NETWORK_TARGET "\n"
+                                    "Wants=" SPECIAL_NETWORK_ONLINE_TARGET "\n",
+                                    program_invocation_short_name);
+}
+
 int generator_write_initrd_root_device_deps(const char *dir, const char *what) {
         _cleanup_free_ char *unit = NULL;
         int r;

src/shared/generator.h

@@ -35,6 +35,12 @@ int generator_write_timeouts(
         const char *opts,
         char **filtered);
 
+int generator_write_device_deps(
+                const char *dir,
+                const char *what,
+                const char *where,
+                const char *opts);
+
 int generator_write_initrd_root_device_deps(
         const char *dir,
         const char *what);