[systemd-devel] [PATCH] automount: add expire support
Tom Gundersen
teg at jklm.no
Fri Mar 27 04:36:03 PDT 2015
Looks nice. Minor nitpicks below.
On Sun, Mar 22, 2015 at 1:36 PM, Michael Olbrich
<m.olbrich at pengutronix.de> wrote:
> ---
> man/systemd.automount.xml | 8 ++
> man/systemd.mount.xml | 9 ++
> src/core/automount.c | 209 ++++++++++++++++++++++++++++++++--
> src/core/automount.h | 6 +-
> src/core/dbus-automount.c | 1 +
> src/core/load-fragment-gperf.gperf.m4 | 1 +
> src/core/mount.c | 20 +---
> src/fstab-generator/fstab-generator.c | 27 +++++
> 8 files changed, 253 insertions(+), 28 deletions(-)
>
> diff --git a/man/systemd.automount.xml b/man/systemd.automount.xml
> index b5b5885cdff2..9561590c5c89 100644
> --- a/man/systemd.automount.xml
> +++ b/man/systemd.automount.xml
> @@ -135,6 +135,14 @@
> creating these directories. Takes an access mode in octal
> notation. Defaults to 0755.</para></listitem>
> </varlistentry>
> + <varlistentry>
> + <term><varname>TimeoutIdleSec=</varname></term>
> + <listitem><para>Configures an idleness timeout. Once the mount has been
> + idle for the specified time, systemd will attempt to unmount. Takes a
> + unit-less value in seconds, or a time span value such as "5min 20s".
> + Pass 0 to disable the timeout logic. The timeout is disabled by
> + default.</para></listitem>
> + </varlistentry>
> </variablelist>
> </refsect1>
>
> diff --git a/man/systemd.mount.xml b/man/systemd.mount.xml
> index fcb9a4416104..e102d27ab762 100644
> --- a/man/systemd.mount.xml
> +++ b/man/systemd.mount.xml
> @@ -148,6 +148,15 @@
> </varlistentry>
>
> <varlistentry>
> + <term><option>x-systemd.idle-timeout=</option></term>
> +
> + <listitem><para>Configures the idleness timeout of the
> + automount unit. See <varname>TimeoutIdleSec=</varname> in
> + <citerefentry><refentrytitle>systemd.automount</refentrytitle><manvolnum>5</manvolnum></citerefentry>
> + for details.</para></listitem>
> + </varlistentry>
> +
> + <varlistentry>
> <term><option>x-systemd.device-timeout=</option></term>
>
> <listitem><para>Configure how long systemd should wait for a
> diff --git a/src/core/automount.c b/src/core/automount.c
> index cec90cbb319d..d2c148943e0f 100644
> --- a/src/core/automount.c
> +++ b/src/core/automount.c
> @@ -40,6 +40,7 @@
> #include "dbus-automount.h"
> #include "bus-util.h"
> #include "bus-error.h"
> +#include "async.h"
>
> static const UnitActiveState state_translation_table[_AUTOMOUNT_STATE_MAX] = {
> [AUTOMOUNT_DEAD] = UNIT_INACTIVE,
> @@ -79,13 +80,16 @@ static void repeat_unmount(const char *path) {
> }
> }
>
> +static int automount_send_ready(Automount *a, Set *tokens, int status);
> +
> static void unmount_autofs(Automount *a) {
> assert(a);
>
> if (a->pipe_fd < 0)
> return;
>
> - automount_send_ready(a, -EHOSTDOWN);
> + automount_send_ready(a, a->tokens, -EHOSTDOWN);
> + automount_send_ready(a, a->expire_tokens, -EHOSTDOWN);
>
> a->pipe_event_source = sd_event_source_unref(a->pipe_event_source);
> a->pipe_fd = safe_close(a->pipe_fd);
> @@ -110,6 +114,8 @@ static void automount_done(Unit *u) {
>
> set_free(a->tokens);
> a->tokens = NULL;
> + set_free(a->expire_tokens);
> + a->expire_tokens = NULL;
> }
>
> static int automount_add_mount_links(Automount *a) {
> @@ -263,6 +269,7 @@ static int automount_coldplug(Unit *u, Hashmap *deferred_work) {
> }
>
> static void automount_dump(Unit *u, FILE *f, const char *prefix) {
> + char time_string[FORMAT_TIMESPAN_MAX];
> Automount *a = AUTOMOUNT(u);
>
> assert(a);
> @@ -271,11 +278,13 @@ static void automount_dump(Unit *u, FILE *f, const char *prefix) {
> "%sAutomount State: %s\n"
> "%sResult: %s\n"
> "%sWhere: %s\n"
> - "%sDirectoryMode: %04o\n",
> + "%sDirectoryMode: %04o\n"
> + "%sTimeoutIdleUSec: %s\n",
> prefix, automount_state_to_string(a->state),
> prefix, automount_result_to_string(a->result),
> prefix, a->where,
> - prefix, a->directory_mode);
> + prefix, a->directory_mode,
> + prefix, format_timespan(time_string, FORMAT_TIMESPAN_MAX, a->timeout_idle_usec, USEC_PER_SEC));
> }
>
> static void automount_enter_dead(Automount *a, AutomountResult f) {
> @@ -365,7 +374,7 @@ static int autofs_protocol(int dev_autofs_fd, int ioctl_fd) {
> return 0;
> }
>
> -static int autofs_set_timeout(int dev_autofs_fd, int ioctl_fd, time_t sec) {
> +static int autofs_set_timeout(int dev_autofs_fd, int ioctl_fd, usec_t usec) {
> struct autofs_dev_ioctl param;
>
> assert(dev_autofs_fd >= 0);
> @@ -373,7 +382,9 @@ static int autofs_set_timeout(int dev_autofs_fd, int ioctl_fd, time_t sec) {
>
> init_autofs_dev_ioctl(¶m);
> param.ioctlfd = ioctl_fd;
> - param.timeout.timeout = sec;
> +
> + /* Convert to seconds, rounding up. */
> + param.timeout.timeout = usec / USEC_PER_SEC + (usec % USEC_PER_SEC > 0);
This is an odd way to round, no? Usually we do
(usec + USEC_PER_SEC - 1) / USEC_PER_SEC;
> if (ioctl(dev_autofs_fd, AUTOFS_DEV_IOCTL_TIMEOUT, ¶m) < 0)
> return -errno;
> @@ -402,7 +413,7 @@ static int autofs_send_ready(int dev_autofs_fd, int ioctl_fd, uint32_t token, in
> return 0;
> }
>
> -int automount_send_ready(Automount *a, int status) {
> +static int automount_send_ready(Automount *a, Set *tokens, int status) {
> _cleanup_close_ int ioctl_fd = -1;
> unsigned token;
> int r;
> @@ -410,7 +421,7 @@ int automount_send_ready(Automount *a, int status) {
> assert(a);
> assert(status <= 0);
>
> - if (set_isempty(a->tokens))
> + if (set_isempty(tokens))
> return 0;
>
> ioctl_fd = open_ioctl_fd(UNIT(a)->manager->dev_autofs_fd, a->where, a->dev_id);
> @@ -425,7 +436,7 @@ int automount_send_ready(Automount *a, int status) {
> r = 0;
>
> /* Autofs thankfully does not hand out 0 as a token */
> - while ((token = PTR_TO_UINT(set_steal_first(a->tokens)))) {
> + while ((token = PTR_TO_UINT(set_steal_first(tokens)))) {
> int k;
>
> /* Autofs fun fact II:
> @@ -444,6 +455,45 @@ int automount_send_ready(Automount *a, int status) {
> return r;
> }
>
> +int automount_update_mount(Automount *a, MountState old_state, MountState state)
> +{
> + _cleanup_close_ int ioctl_fd = -1;
> +
> + assert(a);
These should be switch() statements, I guess:
> + if (state == MOUNT_MOUNTED || state == MOUNT_REMOUNTING)
> + automount_send_ready(a, a->tokens, 0);
> + else if (state == MOUNT_DEAD ||
> + state == MOUNT_UNMOUNTING ||
> + state == MOUNT_MOUNTING_SIGTERM ||
> + state == MOUNT_MOUNTING_SIGKILL ||
> + state == MOUNT_REMOUNTING_SIGTERM ||
> + state == MOUNT_REMOUNTING_SIGKILL ||
> + state == MOUNT_UNMOUNTING_SIGTERM ||
> + state == MOUNT_UNMOUNTING_SIGKILL ||
> + state == MOUNT_FAILED) {
> + if (old_state != state)
> + automount_send_ready(a, a->tokens, -ENODEV);
> + }
> +
> + if (state == MOUNT_DEAD)
> + automount_send_ready(a, a->expire_tokens, 0);
> + else if (state == MOUNT_MOUNTING ||
> + state == MOUNT_MOUNTING_DONE ||
> + state == MOUNT_MOUNTING_SIGTERM ||
> + state == MOUNT_MOUNTING_SIGKILL ||
> + state == MOUNT_REMOUNTING_SIGTERM ||
> + state == MOUNT_REMOUNTING_SIGKILL ||
> + state == MOUNT_UNMOUNTING_SIGTERM ||
> + state == MOUNT_UNMOUNTING_SIGKILL ||
> + state == MOUNT_FAILED) {
> + if (old_state != state)
> + automount_send_ready(a, a->expire_tokens, -ENODEV);
> + }
> +
> + return 0;
> +}
> +
> static void automount_enter_waiting(Automount *a) {
> _cleanup_close_ int ioctl_fd = -1;
> int p[2] = { -1, -1 };
> @@ -503,7 +553,7 @@ static void automount_enter_waiting(Automount *a) {
> if (r < 0)
> goto fail;
>
> - r = autofs_set_timeout(dev_autofs_fd, ioctl_fd, 300);
> + r = autofs_set_timeout(dev_autofs_fd, ioctl_fd, a->timeout_idle_usec);
> if (r < 0)
> goto fail;
>
> @@ -535,6 +585,100 @@ fail:
> automount_enter_dead(a, AUTOMOUNT_FAILURE_RESOURCES);
> }
>
> +static int automount_start_expire(Automount *a);
>
> +struct expire_data {
> + int dev_autofs_fd;
> + int ioctl_fd;
> +};
Structs at the top of the file please.
> +static inline void cleanup_expire(void *p) {
> + struct expire_data *data = *(struct expire_data**)p;
> +
> + if (!data)
> + return;
> +
> + safe_close(data->dev_autofs_fd);
> + safe_close(data->ioctl_fd);
> + free(data);
> +}
> +#define _cleanup_expire_ _cleanup_(cleanup_expire)
No need to #define this for internal stuff that is only used a few
times. However, you should use DEFINE_TRIVIAL_CLEANUP_FUNC() to get
the types right, then just use
_cleanup_(cleanup_expirep)
in your code (note the 'p').
> +static void *expire_thread(void *p) {
> + struct autofs_dev_ioctl param;
> + _cleanup_expire_ struct expire_data *data = (struct expire_data*)p;
> + int r;
> +
> + assert(data->dev_autofs_fd >= 0);
> + assert(data->ioctl_fd >= 0);
> +
> + init_autofs_dev_ioctl(¶m);
> + param.ioctlfd = data->ioctl_fd;
> +
> + do {
> + r = ioctl(data->dev_autofs_fd, AUTOFS_DEV_IOCTL_EXPIRE, ¶m);
> + } while (r == 0);
Could this be "r >= 0" instead, to make it clearer that the next
condition is only hit when r < 0 (so errno is guaranteed to be set
correctly).
> + if (errno != EAGAIN)
> + log_error_errno(errno, "failed to expire automount: %m");
No need for this, it is equivalent to
log_error("failed to exprie automount: %m");
> + return NULL;
> +}
> +
> +static int automount_dispatch_expire(sd_event_source *source, usec_t usec, void *userdata) {
> + Automount *a = AUTOMOUNT(userdata);
> + _cleanup_expire_ struct expire_data *data = NULL;
> + int r;
> +
> + assert(a);
> + assert(source == a->expire_event_source);
> +
> + data = malloc0(sizeof(struct expire_data));
> + if (!data)
> + return -ENOMEM;
> +
> + data->ioctl_fd = -1;
> +
> + data->dev_autofs_fd = dup(UNIT(a)->manager->dev_autofs_fd);
> + if (data->dev_autofs_fd < 0)
> + return data->dev_autofs_fd;
> +
> + data->ioctl_fd = open_ioctl_fd(UNIT(a)->manager->dev_autofs_fd, a->where, a->dev_id);
> + if (data->ioctl_fd < 0)
> + return data->ioctl_fd;
> +
> + r = asynchronous_job(expire_thread, data);
> + if (r < 0) {
> + log_unit_error(UNIT(a)->id, "Failed to start expire job: %s", strerror(-r));
> + return r;
> + }
> +
> + data = NULL;
> +
> + return automount_start_expire(a);
> +}
> +
> +static int automount_start_expire(Automount *a) {
> + int r;
> +
> + assert(a);
> +
> + if (a->expire_event_source) {
> + r = sd_event_source_set_time(a->expire_event_source, now(CLOCK_MONOTONIC) + 5 * USEC_PER_SEC);
> + if (r < 0)
> + return r;
> +
> + return sd_event_source_set_enabled(a->expire_event_source, SD_EVENT_ONESHOT);
> + }
> +
> + return sd_event_add_time(
> + UNIT(a)->manager->event,
> + &a->expire_event_source,
> + CLOCK_MONOTONIC,
> + now(CLOCK_MONOTONIC) + 5 * USEC_PER_SEC, 0,
> + automount_dispatch_expire, a);
> +}
> +
> static void automount_enter_runnning(Automount *a) {
> _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
> struct stat st;
> @@ -547,7 +691,8 @@ static void automount_enter_runnning(Automount *a) {
> if (unit_stop_pending(UNIT(a))) {
> log_unit_debug(UNIT(a)->id,
> "Suppressing automount request on %s since unit stop is scheduled.", UNIT(a)->id);
> - automount_send_ready(a, -EHOSTDOWN);
> + automount_send_ready(a, a->tokens, -EHOSTDOWN);
> + automount_send_ready(a, a->expire_tokens, -EHOSTDOWN);
> return;
> }
>
> @@ -573,6 +718,7 @@ static void automount_enter_runnning(Automount *a) {
> goto fail;
> }
> }
> + automount_start_expire(a);
>
> automount_set_state(a, AUTOMOUNT_RUNNING);
> return;
> @@ -627,6 +773,8 @@ static int automount_serialize(Unit *u, FILE *f, FDSet *fds) {
>
> SET_FOREACH(p, a->tokens, i)
> unit_serialize_item_format(u, f, "token", "%u", PTR_TO_UINT(p));
> + SET_FOREACH(p, a->expire_tokens, i)
> + unit_serialize_item_format(u, f, "expire-token", "%u", PTR_TO_UINT(p));
>
> if (a->pipe_fd >= 0) {
> int copy;
> @@ -686,6 +834,20 @@ static int automount_deserialize_item(Unit *u, const char *key, const char *valu
> if (r < 0)
> return r;
> }
> + } else if (streq(key, "expire-token")) {
> + unsigned token;
> +
> + if (safe_atou(value, &token) < 0)
> + log_unit_debug(u->id, "Failed to parse token value %s", value);
> + else {
> + if (!a->expire_tokens)
> + if (!(a->expire_tokens = set_new(NULL)))
> + return -ENOMEM;
> +
> + r = set_put(a->expire_tokens, UINT_TO_PTR(token));
> + if (r < 0)
> + return r;
> + }
> } else if (streq(key, "pipe-fd")) {
> int fd;
>
> @@ -723,6 +885,7 @@ static bool automount_check_gc(Unit *u) {
> }
>
> static int automount_dispatch_io(sd_event_source *s, int fd, uint32_t events, void *userdata) {
> + _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
> union autofs_v5_packet_union packet;
> Automount *a = AUTOMOUNT(userdata);
> int r;
> @@ -770,6 +933,32 @@ static int automount_dispatch_io(sd_event_source *s, int fd, uint32_t events, vo
> automount_enter_runnning(a);
> break;
>
> + case autofs_ptype_expire_direct:
> + log_unit_debug(UNIT(a)->id, "Got direct umount request on %s", a->where);
> +
> + sd_event_source_set_enabled(a->expire_event_source, SD_EVENT_OFF);
> +
> + r = set_ensure_allocated(&a->expire_tokens, NULL);
> + if (r < 0) {
> + log_unit_error(UNIT(a)->id, "Failed to allocate token set.");
> + goto fail;
> + }
> +
> + r = set_put(a->expire_tokens, UINT_TO_PTR(packet.v5_packet.wait_queue_token));
> + if (r < 0) {
> + log_unit_error_errno(UNIT(a)->id, r, "Failed to remember token: %m");
> + goto fail;
> + }
> + r = manager_add_job(UNIT(a)->manager, JOB_STOP, UNIT_TRIGGER(UNIT(a)),
> + JOB_REPLACE, true, &error, NULL);
> + if (r < 0) {
> + log_unit_warning(UNIT(a)->id,
> + "%s failed to queue umount startup job: %s",
> + UNIT(a)->id, bus_error_message(&error, r));
> + goto fail;
> + }
> + break;
> +
> default:
> log_unit_error(UNIT(a)->id, "Received unknown automount request %i", packet.hdr.type);
> break;
> diff --git a/src/core/automount.h b/src/core/automount.h
> index 60f55223897e..2a50fef68d01 100644
> --- a/src/core/automount.h
> +++ b/src/core/automount.h
> @@ -47,6 +47,7 @@ struct Automount {
> AutomountState state, deserialized_state;
>
> char *where;
> + usec_t timeout_idle_usec;
>
> int pipe_fd;
> sd_event_source *pipe_event_source;
> @@ -54,13 +55,16 @@ struct Automount {
> dev_t dev_id;
>
> Set *tokens;
> + Set *expire_tokens;
> +
> + sd_event_source *expire_event_source;
>
> AutomountResult result;
> };
>
> extern const UnitVTable automount_vtable;
>
> -int automount_send_ready(Automount *a, int status);
> +int automount_update_mount(Automount *a, MountState old_state, MountState state);
>
> const char* automount_state_to_string(AutomountState i) _const_;
> AutomountState automount_state_from_string(const char *s) _pure_;
> diff --git a/src/core/dbus-automount.c b/src/core/dbus-automount.c
> index 38acbd0c2625..5162ce34cb75 100644
> --- a/src/core/dbus-automount.c
> +++ b/src/core/dbus-automount.c
> @@ -30,5 +30,6 @@ const sd_bus_vtable bus_automount_vtable[] = {
> SD_BUS_PROPERTY("Where", "s", NULL, offsetof(Automount, where), SD_BUS_VTABLE_PROPERTY_CONST),
> SD_BUS_PROPERTY("DirectoryMode", "u", bus_property_get_mode, offsetof(Automount, directory_mode), SD_BUS_VTABLE_PROPERTY_CONST),
> SD_BUS_PROPERTY("Result", "s", property_get_result, offsetof(Automount, result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
> + SD_BUS_PROPERTY("TimeoutIdleUSec", "t", bus_property_get_usec, offsetof(Automount, timeout_idle_usec), SD_BUS_VTABLE_PROPERTY_CONST),
> SD_BUS_VTABLE_END
> };
> diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4
> index 53059845e30c..66c9145aa6ab 100644
> --- a/src/core/load-fragment-gperf.gperf.m4
> +++ b/src/core/load-fragment-gperf.gperf.m4
> @@ -318,6 +318,7 @@ KILL_CONTEXT_CONFIG_ITEMS(Mount)m4_dnl
> m4_dnl
> Automount.Where, config_parse_path, 0, offsetof(Automount, where)
> Automount.DirectoryMode, config_parse_mode, 0, offsetof(Automount, directory_mode)
> +Automount.TimeoutIdleSec, config_parse_sec, 0, offsetof(Automount, timeout_idle_usec)
> m4_dnl
> Swap.What, config_parse_path, 0, offsetof(Swap, parameters_fragment.what)
> Swap.Priority, config_parse_int, 0, offsetof(Swap, parameters_fragment.priority)
> diff --git a/src/core/mount.c b/src/core/mount.c
> index 1251c94608d6..4154dd708959 100644
> --- a/src/core/mount.c
> +++ b/src/core/mount.c
> @@ -545,7 +545,7 @@ static int mount_load(Unit *u) {
> return mount_verify(m);
> }
>
> -static int mount_notify_automount(Mount *m, int status) {
> +static int mount_notify_automount(Mount *m, MountState old_state, MountState state) {
> Unit *p;
> int r;
> Iterator i;
> @@ -554,7 +554,7 @@ static int mount_notify_automount(Mount *m, int status) {
>
> SET_FOREACH(p, UNIT(m)->dependencies[UNIT_TRIGGERED_BY], i)
> if (p->type == UNIT_AUTOMOUNT) {
> - r = automount_send_ready(AUTOMOUNT(p), status);
> + r = automount_update_mount(AUTOMOUNT(p), old_state, state);
> if (r < 0)
> return r;
> }
> @@ -585,21 +585,7 @@ static void mount_set_state(Mount *m, MountState state) {
> m->control_command_id = _MOUNT_EXEC_COMMAND_INVALID;
> }
>
> - if (state == MOUNT_MOUNTED ||
> - state == MOUNT_REMOUNTING)
> - mount_notify_automount(m, 0);
> - else if (state == MOUNT_DEAD ||
> - state == MOUNT_UNMOUNTING ||
> - state == MOUNT_MOUNTING_SIGTERM ||
> - state == MOUNT_MOUNTING_SIGKILL ||
> - state == MOUNT_REMOUNTING_SIGTERM ||
> - state == MOUNT_REMOUNTING_SIGKILL ||
> - state == MOUNT_UNMOUNTING_SIGTERM ||
> - state == MOUNT_UNMOUNTING_SIGKILL ||
> - state == MOUNT_FAILED) {
> - if (state != old_state)
> - mount_notify_automount(m, -ENODEV);
> - }
> + mount_notify_automount(m, old_state, state);
>
> if (state != old_state)
> log_unit_debug(UNIT(m)->id,
> diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c
> index 2ece12f79202..0b34f2fa3c43 100644
> --- a/src/fstab-generator/fstab-generator.c
> +++ b/src/fstab-generator/fstab-generator.c
> @@ -154,6 +154,29 @@ static bool mount_in_initrd(struct mntent *me) {
> streq(me->mnt_dir, "/usr");
> }
>
> +static int write_idle_timeout(FILE *f, const char *where, const char *opts, char **filtered) {
> + _cleanup_free_ char *timeout = NULL;
> + usec_t u;
> + int r;
> +
> + r = fstab_filter_options(opts, "x-systemd.idle-timeout\0",
> + NULL, &timeout, filtered);
> + if (r <= 0)
> + return r;
> +
> + r = parse_sec(timeout, &u);
> + if (r < 0) {
> + log_warning("Failed to parse timeout for %s, ignoring: %s",
> + where, timeout);
> + return 0;
> + }
> +
> + fprintf(f,
> + "TimeoutIdleSec=" USEC_FMT "\n",
> + u / USEC_PER_SEC);
> +
> + return 0;
> +}
> static int add_mount(
> const char *what,
> const char *where,
> @@ -293,6 +316,10 @@ static int add_mount(
> "Where=%s\n",
> where);
>
> + r = write_idle_timeout(f, where, opts, &filtered);
> + if (r < 0)
> + return r;
> +
> fflush(f);
> if (ferror(f))
> return log_error_errno(errno, "Failed to write unit file %s: %m", automount_unit);
> --
> 2.1.4
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
More information about the systemd-devel
mailing list