From 5fe4c30ca78679551edd7152b70d66b481453b66 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 9 Feb 2025 19:59:13 +0100 Subject: [PATCH 01/10] core/dbus-service: fix alignment --- src/core/dbus-service.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c index 85eb99f218..a1c452e29f 100644 --- a/src/core/dbus-service.c +++ b/src/core/dbus-service.c @@ -393,10 +393,10 @@ const sd_bus_vtable bus_service_vtable[] = { SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD_WITH_ARGS("MountImage", - SD_BUS_ARGS("s", source, "s", destination, "b", read_only, "b", mkdir, "a(ss)", options), - SD_BUS_NO_RESULT, - bus_service_method_mount_image, - SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_ARGS("s", source, "s", destination, "b", read_only, "b", mkdir, "a(ss)", options), + SD_BUS_NO_RESULT, + bus_service_method_mount_image, + SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD_WITH_ARGS("DumpFileDescriptorStore", SD_BUS_NO_ARGS, From 7e9a78d6bea96bf8e9de615f6013c72f912f3e2d Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 9 Feb 2025 20:25:21 +0100 Subject: [PATCH 02/10] core/mount: filter out "fail" option as well --- src/core/mount.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/mount.c b/src/core/mount.c index 6fff94d19b..8a396f67f6 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1149,7 +1149,7 @@ static int mount_set_mount_command(Mount *m, ExecCommand *c, const MountParamete } _cleanup_free_ char *opts = NULL; - r = fstab_filter_options(p->options, "nofail\0" "noauto\0" "auto\0", NULL, NULL, NULL, &opts); + r = fstab_filter_options(p->options, "nofail\0" "fail\0" "noauto\0" "auto\0", NULL, NULL, NULL, &opts); if (r < 0) return r; From 65bc0c03b97065847b187af2092458977173def4 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 9 Feb 2025 20:41:20 +0100 Subject: [PATCH 03/10] core/mount: check parameters_fragment first in mount_enter_(re)mounting() I.e. don't perform any action if we can't spawn mount task anyway. Later the same check would be added to mount_can_start/reload(), so this makes things more coherent too. --- src/core/mount.c | 89 +++++++++++++++++++++++------------------------- 1 file changed, 43 insertions(+), 46 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 8a396f67f6..6998f02d5a 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1178,7 +1178,12 @@ static void mount_enter_mounting(Mount *m) { goto fail; p = get_mount_parameters_fragment(m); - if (p && mount_is_bind(p)) { + if (!p) { + r = log_unit_warning_errno(UNIT(m), SYNTHETIC_ERRNO(ENOENT), "No mount parameters to operate on."); + goto fail; + } + + if (mount_is_bind(p)) { r = is_dir(p->what, /* follow = */ true); if (r < 0 && r != -ENOENT) log_unit_info_errno(UNIT(m), r, "Failed to determine type of bind mount source '%s', ignoring: %m", p->what); @@ -1194,7 +1199,7 @@ static void mount_enter_mounting(Mount *m) { log_unit_warning_errno(UNIT(m), r, "Failed to create mount point '%s', ignoring: %m", m->where); /* If we are asked to create an OverlayFS, create the upper/work directories if they are missing */ - if (p && streq_ptr(p->fstype, "overlay")) { + if (streq_ptr(p->fstype, "overlay")) { _cleanup_strv_free_ char **dirs = NULL; r = fstab_filter_options( @@ -1223,13 +1228,9 @@ static void mount_enter_mounting(Mount *m) { if (source_is_dir) unit_warn_if_dir_nonempty(UNIT(m), m->where); - unit_warn_leftover_processes(UNIT(m), /* start = */ true); - - m->control_command_id = MOUNT_EXEC_MOUNT; - m->control_command = m->exec_command + MOUNT_EXEC_MOUNT; /* Create the source directory for bind-mounts if needed */ - if (p && mount_is_bind(p)) { + if (mount_is_bind(p)) { r = mkdir_p_label(p->what, m->directory_mode); /* mkdir_p_label() can return -EEXIST if the target path exists and is not a directory - which is * totally OK, in case the user wants us to overmount a non-directory inode. Also -EROFS can be @@ -1241,19 +1242,18 @@ static void mount_enter_mounting(Mount *m) { r, "Failed to make bind mount source '%s', ignoring: %m", p->what); } - if (p) { - r = mount_set_mount_command(m, m->control_command, p); - if (r < 0) { - log_unit_warning_errno(UNIT(m), r, "Failed to prepare mount command line: %m"); - goto fail; - } - } else { - r = log_unit_warning_errno(UNIT(m), SYNTHETIC_ERRNO(ENOENT), "No mount parameters to operate on."); + mount_unwatch_control_pid(m); + unit_warn_leftover_processes(UNIT(m), /* start = */ true); + + m->control_command_id = MOUNT_EXEC_MOUNT; + m->control_command = m->exec_command + MOUNT_EXEC_MOUNT; + + r = mount_set_mount_command(m, m->control_command, p); + if (r < 0) { + log_unit_warning_errno(UNIT(m), r, "Failed to prepare mount command line: %m"); goto fail; } - mount_unwatch_control_pid(m); - r = mount_spawn(m, m->control_command, mount_exec_flags(MOUNT_MOUNTING), &m->control_pid); if (r < 0) { log_unit_warning_errno(UNIT(m), r, "Failed to spawn 'mount' task: %m"); @@ -1283,41 +1283,38 @@ static void mount_enter_remounting(Mount *m) { assert(m); - /* Reset reload result when we are about to start a new remount operation */ - m->reload_result = MOUNT_SUCCESS; - - m->control_command_id = MOUNT_EXEC_REMOUNT; - m->control_command = m->exec_command + MOUNT_EXEC_REMOUNT; - p = get_mount_parameters_fragment(m); - if (p) { - const char *o; - - if (p->options) - o = strjoina("remount,", p->options); - else - o = "remount"; - - r = exec_command_set(m->control_command, MOUNT_PATH, - p->what, m->where, - "-o", o, NULL); - if (r >= 0 && m->sloppy_options) - r = exec_command_append(m->control_command, "-s", NULL); - if (r >= 0 && m->read_write_only) - r = exec_command_append(m->control_command, "-w", NULL); - if (r >= 0 && p->fstype) - r = exec_command_append(m->control_command, "-t", p->fstype, NULL); - if (r < 0) { - log_unit_warning_errno(UNIT(m), r, "Failed to prepare remount command line: %m"); - goto fail; - } - - } else { + if (!p) { r = log_unit_warning_errno(UNIT(m), SYNTHETIC_ERRNO(ENOENT), "No mount parameters to operate on."); goto fail; } mount_unwatch_control_pid(m); + m->reload_result = MOUNT_SUCCESS; + + m->control_command_id = MOUNT_EXEC_REMOUNT; + m->control_command = m->exec_command + MOUNT_EXEC_REMOUNT; + + const char *o; + + if (p->options) + o = strjoina("remount,", p->options); + else + o = "remount"; + + r = exec_command_set(m->control_command, MOUNT_PATH, + p->what, m->where, + "-o", o, NULL); + if (r >= 0 && m->sloppy_options) + r = exec_command_append(m->control_command, "-s", NULL); + if (r >= 0 && m->read_write_only) + r = exec_command_append(m->control_command, "-w", NULL); + if (r >= 0 && p->fstype) + r = exec_command_append(m->control_command, "-t", p->fstype, NULL); + if (r < 0) { + log_unit_warning_errno(UNIT(m), r, "Failed to prepare remount command line: %m"); + goto fail; + } r = mount_spawn(m, m->control_command, mount_exec_flags(MOUNT_REMOUNTING), &m->control_pid); if (r < 0) { From 74c0d9726cfbd4ecdf1807adf5433fcf4b1d86b4 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 10 Feb 2025 20:22:09 +0100 Subject: [PATCH 04/10] core/mount: report accurate can_start and can_reload --- src/core/mount.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/core/mount.c b/src/core/mount.c index 6998f02d5a..9632a12ca6 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1424,6 +1424,12 @@ static int mount_reload(Unit *u) { return 1; } +static bool mount_can_reload(Unit *u) { + Mount *m = ASSERT_PTR(MOUNT(u)); + + return get_mount_parameters_fragment(m); +} + static int mount_serialize(Unit *u, FILE *f, FDSet *fds) { Mount *m = ASSERT_PTR(MOUNT(u)); @@ -2361,6 +2367,9 @@ static int mount_can_start(Unit *u) { return r; } + if (!get_mount_parameters_fragment(m)) + return -ENOENT; + return 1; } @@ -2475,7 +2484,9 @@ const UnitVTable mount_vtable = { .start = mount_start, .stop = mount_stop, + .reload = mount_reload, + .can_reload = mount_can_reload, .clean = mount_clean, .can_clean = mount_can_clean, From c7c6cf2031c1c0eb1839d2ea83b5a8de4af6da00 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 12 Feb 2025 14:56:34 +0100 Subject: [PATCH 05/10] core/mount: trivial coding style cleanups --- src/core/mount.c | 7 +++---- src/core/mount.h | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 9632a12ca6..779c8db77c 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -45,9 +45,9 @@ static const UnitActiveState state_translation_table[_MOUNT_STATE_MAX] = { [MOUNT_MOUNTING_DONE] = UNIT_ACTIVATING, [MOUNT_MOUNTED] = UNIT_ACTIVE, [MOUNT_REMOUNTING] = UNIT_RELOADING, - [MOUNT_UNMOUNTING] = UNIT_DEACTIVATING, [MOUNT_REMOUNTING_SIGTERM] = UNIT_RELOADING, [MOUNT_REMOUNTING_SIGKILL] = UNIT_RELOADING, + [MOUNT_UNMOUNTING] = UNIT_DEACTIVATING, [MOUNT_UNMOUNTING_SIGTERM] = UNIT_DEACTIVATING, [MOUNT_UNMOUNTING_SIGKILL] = UNIT_DEACTIVATING, [MOUNT_FAILED] = UNIT_FAILED, @@ -1056,6 +1056,8 @@ static void mount_enter_unmounting(Mount *m) { MOUNT_UNMOUNTING_SIGKILL)) m->n_retry_umount = 0; + mount_unwatch_control_pid(m); + m->control_command_id = MOUNT_EXEC_UNMOUNT; m->control_command = m->exec_command + MOUNT_EXEC_UNMOUNT; @@ -1065,8 +1067,6 @@ static void mount_enter_unmounting(Mount *m) { goto fail; } - mount_unwatch_control_pid(m); - r = mount_spawn(m, m->control_command, mount_exec_flags(MOUNT_UNMOUNTING), &m->control_pid); if (r < 0) { log_unit_warning_errno(UNIT(m), r, "Failed to spawn 'umount' task: %m"); @@ -1074,7 +1074,6 @@ static void mount_enter_unmounting(Mount *m) { } mount_set_state(m, MOUNT_UNMOUNTING); - return; fail: diff --git a/src/core/mount.h b/src/core/mount.h index 28cc7785d8..a8ae25e638 100644 --- a/src/core/mount.h +++ b/src/core/mount.h @@ -81,7 +81,7 @@ struct Mount { MountState state, deserialized_state; - ExecCommand* control_command; + ExecCommand *control_command; MountExecCommand control_command_id; PidRef control_pid; From 0fa062f9836ea5ed09bc42026d2d576dfb52c409 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 10 Feb 2025 20:24:22 +0100 Subject: [PATCH 06/10] core/dbus-mount: add missing ReloadResult and CleanResult properties --- man/org.freedesktop.systemd1.xml | 16 ++++++++++++++-- src/core/dbus-mount.c | 2 ++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index 0664d026f1..fab5db53a7 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -7048,6 +7048,8 @@ node /org/freedesktop/systemd1/unit/home_2emount { @org.freedesktop.DBus.Property.EmitsChangedSignal("const") readonly b ReadWriteOnly = ...; readonly s Result = '...'; + readonly s ReloadResult = '...'; + readonly s CleanResult = '...'; readonly u UID = ...; readonly u GID = ...; @org.freedesktop.DBus.Property.EmitsChangedSignal("const") @@ -7652,6 +7654,10 @@ node /org/freedesktop/systemd1/unit/home_2emount { + + + + @@ -8200,6 +8206,10 @@ node /org/freedesktop/systemd1/unit/home_2emount { + + + + @@ -12462,8 +12472,10 @@ $ gdbus introspect --system --dest org.freedesktop.systemd1 \ ProtectControlGroupsEx, and PrivatePIDs were added in version 257. ProtectHostnameEx, - RemoveSubgroup(), and - GracefulOptions were added in version 258. + RemoveSubgroup(), + GracefulOptions, + ReloadResult, and + CleanResult were added in version 258. Swap Unit Objects diff --git a/src/core/dbus-mount.c b/src/core/dbus-mount.c index 855300d025..72bd4c697a 100644 --- a/src/core/dbus-mount.c +++ b/src/core/dbus-mount.c @@ -73,6 +73,8 @@ const sd_bus_vtable bus_mount_vtable[] = { SD_BUS_PROPERTY("ForceUnmount", "b", bus_property_get_bool, offsetof(Mount, force_unmount), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("ReadWriteOnly", "b", bus_property_get_bool, offsetof(Mount, read_write_only), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Result", "s", property_get_result, offsetof(Mount, result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_PROPERTY("ReloadResult", "s", property_get_result, offsetof(Mount, reload_result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_PROPERTY("CleanResult", "s", property_get_result, offsetof(Mount, clean_result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("UID", "u", bus_property_get_uid, offsetof(Unit, ref_uid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("GID", "u", bus_property_get_gid, offsetof(Unit, ref_gid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("GracefulOptions", "as", NULL, offsetof(Mount, graceful_options), SD_BUS_VTABLE_PROPERTY_CONST), From 85f759baeeab59f25080bd19b2f338ae2d05ad8b Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 11 Feb 2025 18:13:01 +0100 Subject: [PATCH 07/10] bus-unit-util: add missing assertions --- src/shared/bus-unit-util.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index ed73950355..ee09c4f231 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -148,9 +148,11 @@ static int bus_append_string(sd_bus_message *m, const char *field, const char *e } static int bus_append_strv(sd_bus_message *m, const char *field, const char *eq, const char *separator, ExtractFlags flags) { - const char *p; int r; + assert(m); + assert(field); + r = sd_bus_message_open_container(m, 'r', "sv"); if (r < 0) return bus_log_create_error(r); @@ -167,16 +169,16 @@ static int bus_append_strv(sd_bus_message *m, const char *field, const char *eq, if (r < 0) return bus_log_create_error(r); - for (p = eq;;) { + for (const char *p = eq;;) { _cleanup_free_ char *word = NULL; r = extract_first_word(&p, &word, separator, flags); - if (r == 0) - break; if (r == -ENOMEM) return log_oom(); if (r < 0) return log_error_errno(r, "Invalid syntax: %s", eq); + if (r == 0) + break; r = sd_bus_message_append_basic(m, 's', word); if (r < 0) From 0d76f1c4237a3e4adda828c694721fd709374b36 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 11 Feb 2025 18:43:25 +0100 Subject: [PATCH 08/10] core/mount: rework GracefulOptions= to be just x-systemd.graceful-option= 09fbff57fcde47782a73f23b3d5cfdcd0e8f699b introduced new knob for such functionality. However, that seems unnecessary. The mount option string is ubiquitous in that all of fstab, kernel cmdline, credentials, systemd-mount, ... speak it. And we already have x-systemd.device-bound= that's parsed by pid1 instead of fstab-generator. It feels hence more natural for graceful options to be an extension of that, rather than its own property. There's also one nice side effect that the setting itself is now more graceful for systemd versions not supporting such feature. --- man/org.freedesktop.systemd1.xml | 7 --- man/systemd.mount.xml | 29 +++++------ src/core/dbus-mount.c | 26 +--------- src/core/load-fragment-gperf.gperf.in | 1 - src/core/load-fragment.c | 45 ----------------- src/core/load-fragment.h | 1 - src/core/mount.c | 62 +++++++++++------------- src/core/mount.h | 2 - src/shared/bus-unit-util.c | 3 -- test/units/TEST-87-AUX-UTILS-VM.mount.sh | 4 +- units/tmp.mount | 3 +- 11 files changed, 46 insertions(+), 137 deletions(-) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index fab5db53a7..1342419947 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -7052,8 +7052,6 @@ node /org/freedesktop/systemd1/unit/home_2emount { readonly s CleanResult = '...'; readonly u UID = ...; readonly u GID = ...; - @org.freedesktop.DBus.Property.EmitsChangedSignal("const") - readonly as GracefulOptions = ['...', ...]; @org.freedesktop.DBus.Property.EmitsChangedSignal("invalidates") readonly a(sasbttttuii) ExecMount = [...]; @org.freedesktop.DBus.Property.EmitsChangedSignal("invalidates") @@ -7662,8 +7660,6 @@ node /org/freedesktop/systemd1/unit/home_2emount { - - @@ -8214,8 +8210,6 @@ node /org/freedesktop/systemd1/unit/home_2emount { - - @@ -12473,7 +12467,6 @@ $ gdbus introspect --system --dest org.freedesktop.systemd1 \ PrivatePIDs were added in version 257. ProtectHostnameEx, RemoveSubgroup(), - GracefulOptions, ReloadResult, and CleanResult were added in version 258. diff --git a/man/systemd.mount.xml b/man/systemd.mount.xml index 991c1f8506..26bc116f97 100644 --- a/man/systemd.mount.xml +++ b/man/systemd.mount.xml @@ -284,6 +284,19 @@ + + + + Additional mount option that shall be appended if supported by the kernel. + This may be used to configure mount options that are optional and only enabled on kernels that + support them. Note that this is supported only for native kernel mount options (i.e. explicitly not + for mount options implemented in userspace, such as those processed by + /usr/bin/mount itself, by FUSE or by mount helpers such as + mount.nfs). This option may be specified more than once. + + + + @@ -650,22 +663,6 @@ systemd-system.conf5. - - - GracefulOptions= - - Additional mount options that shall be appended to Options= if - supported by the kernel. This may be used to configure mount options that are optional and only - enabled on kernels that support them. Note that this is supported only for native kernel mount - options (i.e. explicitly not for mount options implemented in userspace, such as those processed by - /usr/bin/mount itself, by FUSE or by mount helpers such as - mount.nfs). - - May be specified multiple times. If specified multiple times, all listed, supported mount - options are combined. If an empty string is assigned, the list is reset. - - - diff --git a/src/core/dbus-mount.c b/src/core/dbus-mount.c index 72bd4c697a..6b30b90f7f 100644 --- a/src/core/dbus-mount.c +++ b/src/core/dbus-mount.c @@ -77,7 +77,7 @@ const sd_bus_vtable bus_mount_vtable[] = { SD_BUS_PROPERTY("CleanResult", "s", property_get_result, offsetof(Mount, clean_result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("UID", "u", bus_property_get_uid, offsetof(Unit, ref_uid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("GID", "u", bus_property_get_gid, offsetof(Unit, ref_gid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), - SD_BUS_PROPERTY("GracefulOptions", "as", NULL, offsetof(Mount, graceful_options), SD_BUS_VTABLE_PROPERTY_CONST), + BUS_EXEC_COMMAND_VTABLE("ExecMount", offsetof(Mount, exec_command[MOUNT_EXEC_MOUNT]), SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), BUS_EXEC_COMMAND_VTABLE("ExecUnmount", offsetof(Mount, exec_command[MOUNT_EXEC_UNMOUNT]), SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), BUS_EXEC_COMMAND_VTABLE("ExecRemount", offsetof(Mount, exec_command[MOUNT_EXEC_REMOUNT]), SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), @@ -153,30 +153,6 @@ static int bus_mount_set_transient_property( if (streq(name, "ReadWriteOnly")) return bus_set_transient_bool(u, name, &m->read_write_only, message, flags, error); - if (streq(name, "GracefulOptions")) { - _cleanup_strv_free_ char **add = NULL; - r = sd_bus_message_read_strv(message, &add); - if (r < 0) - return r; - - if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - - if (strv_isempty(add)) { - m->graceful_options = strv_free(m->graceful_options); - unit_write_settingf(u, flags, name, "GracefulOptions="); - } else { - r = strv_extend_strv(&m->graceful_options, add, /* filter_duplicates= */ false); - if (r < 0) - return r; - - STRV_FOREACH(a, add) - unit_write_settingf(u, flags|UNIT_ESCAPE_SPECIFIERS, name, "GracefulOptions=%s", *a); - } - } - - return 1; - } - return 0; } diff --git a/src/core/load-fragment-gperf.gperf.in b/src/core/load-fragment-gperf.gperf.in index 1363d7903f..5104c10719 100644 --- a/src/core/load-fragment-gperf.gperf.in +++ b/src/core/load-fragment-gperf.gperf.in @@ -549,7 +549,6 @@ Mount.SloppyOptions, config_parse_bool, Mount.LazyUnmount, config_parse_bool, 0, offsetof(Mount, lazy_unmount) Mount.ForceUnmount, config_parse_bool, 0, offsetof(Mount, force_unmount) Mount.ReadWriteOnly, config_parse_bool, 0, offsetof(Mount, read_write_only) -Mount.GracefulOptions, config_parse_mount_graceful_options, 0, offsetof(Mount, graceful_options) {{ EXEC_CONTEXT_CONFIG_ITEMS('Mount') }} {{ CGROUP_CONTEXT_CONFIG_ITEMS('Mount') }} {{ KILL_CONTEXT_CONFIG_ITEMS('Mount') }} diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index a5b9f9eca9..8460de263e 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -6131,51 +6131,6 @@ int config_parse_mount_node( return config_parse_string(unit, filename, line, section, section_line, lvalue, ltype, path, data, userdata); } -int config_parse_mount_graceful_options( - const char *unit, - const char *filename, - unsigned line, - const char *section, - unsigned section_line, - const char *lvalue, - int ltype, - const char *rvalue, - void *data, - void *userdata) { - - const Unit *u = ASSERT_PTR(userdata); - char ***sv = ASSERT_PTR(data); - int r; - - assert(filename); - assert(lvalue); - assert(rvalue); - - if (isempty(rvalue)) { - *sv = strv_free(*sv); - return 1; - } - - _cleanup_free_ char *resolved = NULL; - r = unit_full_printf(u, rvalue, &resolved); - if (r < 0) { - log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to resolve unit specifiers in '%s', ignoring: %m", rvalue); - return 0; - } - - _cleanup_strv_free_ char **strv = NULL; - - r = strv_split_full(&strv, resolved, ",", EXTRACT_RETAIN_ESCAPE|EXTRACT_UNESCAPE_SEPARATORS); - if (r < 0) - return log_syntax_parse_error(unit, filename, line, r, lvalue, rvalue); - - r = strv_extend_strv_consume(sv, TAKE_PTR(strv), /* filter_duplicates = */ false); - if (r < 0) - return log_oom(); - - return 1; -} - static int merge_by_names(Unit *u, Set *names, const char *id) { char *k; int r; diff --git a/src/core/load-fragment.h b/src/core/load-fragment.h index 28275af8d8..881ce152d5 100644 --- a/src/core/load-fragment.h +++ b/src/core/load-fragment.h @@ -165,7 +165,6 @@ CONFIG_PARSER_PROTOTYPE(config_parse_open_file); CONFIG_PARSER_PROTOTYPE(config_parse_memory_pressure_watch); CONFIG_PARSER_PROTOTYPE(config_parse_cgroup_nft_set); CONFIG_PARSER_PROTOTYPE(config_parse_mount_node); -CONFIG_PARSER_PROTOTYPE(config_parse_mount_graceful_options); /* gperf prototypes */ const struct ConfigPerfItem* load_fragment_gperf_lookup(const char *key, GPERF_LEN_TYPE length); diff --git a/src/core/mount.c b/src/core/mount.c index 779c8db77c..6610c02c2c 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -231,8 +231,6 @@ static void mount_done(Unit *u) { mount_unwatch_control_pid(m); m->timer_event_source = sd_event_source_disable_unref(m->timer_event_source); - - m->graceful_options = strv_free(m->graceful_options); } static int update_parameters_proc_self_mountinfo( @@ -1080,22 +1078,25 @@ fail: mount_enter_dead_or_mounted(m, MOUNT_FAILURE_RESOURCES, /* flush_result = */ false); } -static int mount_append_graceful_options(Mount *m, const MountParameters *p, char **opts) { +static int mount_apply_graceful_options(Mount *m, const MountParameters *p, char **opts) { + _cleanup_strv_free_ char **graceful = NULL; + _cleanup_free_ char *filtered = NULL; int r; assert(m); assert(p); assert(opts); - if (strv_isempty(m->graceful_options)) - return 0; + r = fstab_filter_options(*opts, "x-systemd.graceful-option\0", NULL, NULL, &graceful, &filtered); + if (r <= 0) + return r; if (!p->fstype) { - log_unit_warning(UNIT(m), "GracefulOptions= used but file system type not known, suppressing all graceful options."); + log_unit_warning(UNIT(m), "x-systemd.graceful-option= used but file system type not known, suppressing all graceful options."); return 0; } - STRV_FOREACH(o, m->graceful_options) { + STRV_FOREACH(o, graceful) { _cleanup_free_ char *k = NULL, *v = NULL; r = split_pair(*o, "=", &k, &v); @@ -1104,21 +1105,22 @@ static int mount_append_graceful_options(Mount *m, const MountParameters *p, cha r = mount_option_supported(p->fstype, k ?: *o, v); if (r < 0) - log_unit_warning_errno(UNIT(m), r, "GracefulOptions=%s specified, but cannot determine availability, suppressing.", *o); + log_unit_warning_errno(UNIT(m), r, + "x-systemd.graceful-option=%s specified, but cannot determine availability, suppressing: %m", *o); else if (r == 0) - log_unit_info(UNIT(m), "GracefulOptions=%s specified, but option is not available, suppressing.", *o); + log_unit_info(UNIT(m), "x-systemd.graceful-option=%s specified, but option is not available, suppressing.", *o); else { - log_unit_debug(UNIT(m), "GracefulOptions=%s specified and supported, appending to mount option string.", *o); + log_unit_debug(UNIT(m), "x-systemd.graceful-option=%s specified and supported, appending to mount option string.", *o); - if (!strextend_with_separator(opts, ",", *o)) + if (!strextend_with_separator(&filtered, ",", *o)) return -ENOMEM; } } - return 0; + return free_and_replace(*opts, filtered); } -static int mount_set_mount_command(Mount *m, ExecCommand *c, const MountParameters *p) { +static int mount_set_mount_command(Mount *m, ExecCommand *c, const MountParameters *p, bool remount) { int r; assert(m); @@ -1152,10 +1154,19 @@ static int mount_set_mount_command(Mount *m, ExecCommand *c, const MountParamete if (r < 0) return r; - r = mount_append_graceful_options(m, p, &opts); + r = mount_apply_graceful_options(m, p, &opts); if (r < 0) return r; + if (remount) { + if (isempty(opts)) { + opts = strdup("remount"); + if (!opts) + return -ENOMEM; + } else if (!strprepend(&opts, "remount,")) + return -ENOMEM; + } + if (!isempty(opts)) { r = exec_command_append(c, "-o", opts, NULL); if (r < 0) @@ -1247,9 +1258,9 @@ static void mount_enter_mounting(Mount *m) { m->control_command_id = MOUNT_EXEC_MOUNT; m->control_command = m->exec_command + MOUNT_EXEC_MOUNT; - r = mount_set_mount_command(m, m->control_command, p); + r = mount_set_mount_command(m, m->control_command, p, /* remount = */ false); if (r < 0) { - log_unit_warning_errno(UNIT(m), r, "Failed to prepare mount command line: %m"); + log_unit_error_errno(UNIT(m), r, "Failed to prepare mount command line: %m"); goto fail; } @@ -1294,24 +1305,9 @@ static void mount_enter_remounting(Mount *m) { m->control_command_id = MOUNT_EXEC_REMOUNT; m->control_command = m->exec_command + MOUNT_EXEC_REMOUNT; - const char *o; - - if (p->options) - o = strjoina("remount,", p->options); - else - o = "remount"; - - r = exec_command_set(m->control_command, MOUNT_PATH, - p->what, m->where, - "-o", o, NULL); - if (r >= 0 && m->sloppy_options) - r = exec_command_append(m->control_command, "-s", NULL); - if (r >= 0 && m->read_write_only) - r = exec_command_append(m->control_command, "-w", NULL); - if (r >= 0 && p->fstype) - r = exec_command_append(m->control_command, "-t", p->fstype, NULL); + r = mount_set_mount_command(m, m->control_command, p, /* remount = */ true); if (r < 0) { - log_unit_warning_errno(UNIT(m), r, "Failed to prepare remount command line: %m"); + log_unit_error_errno(UNIT(m), r, "Failed to prepare remount command line: %m"); goto fail; } diff --git a/src/core/mount.h b/src/core/mount.h index a8ae25e638..7fd643f6fa 100644 --- a/src/core/mount.h +++ b/src/core/mount.h @@ -88,8 +88,6 @@ struct Mount { sd_event_source *timer_event_source; unsigned n_retry_umount; - - char **graceful_options; }; extern const UnitVTable mount_vtable; diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index ee09c4f231..3a246e82bc 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -2335,9 +2335,6 @@ static int bus_append_mount_property(sd_bus_message *m, const char *field, const "ReadwriteOnly")) return bus_append_parse_boolean(m, field, eq); - if (streq(field, "GracefulOptions")) - return bus_append_strv(m, field, eq, /* separator= */ ",", 0); - return 0; } diff --git a/test/units/TEST-87-AUX-UTILS-VM.mount.sh b/test/units/TEST-87-AUX-UTILS-VM.mount.sh index c1bfbdc1a2..8ad7ae6fc5 100755 --- a/test/units/TEST-87-AUX-UTILS-VM.mount.sh +++ b/test/units/TEST-87-AUX-UTILS-VM.mount.sh @@ -181,9 +181,9 @@ touch "$WORK_DIR/mnt/hello" [[ "$(stat -c "%U:%G" "$WORK_DIR/mnt/hello")" == "testuser:testuser" ]] systemd-umount LABEL=owner-vfat -# Mkae sure that graceful mount options work +# Make sure that graceful mount options work GRACEFULTEST="/tmp/graceful/$RANDOM" -systemd-mount --tmpfs -p GracefulOptions=idefinitelydontexist,nr_inodes=4711,idonexisteither "$GRACEFULTEST" +systemd-mount --tmpfs --options="x-systemd.graceful-option=idefinitelydontexist,x-systemd.graceful-option=nr_inodes=4711,x-systemd.graceful-option=idonexisteither" "$GRACEFULTEST" findmnt -n -o options "$GRACEFULTEST" findmnt -n -o options "$GRACEFULTEST" | grep -q nr_inodes=4711 umount "$GRACEFULTEST" diff --git a/units/tmp.mount b/units/tmp.mount index 66ce92ad01..92599817e6 100644 --- a/units/tmp.mount +++ b/units/tmp.mount @@ -22,5 +22,4 @@ After=swap.target What=tmpfs Where=/tmp Type=tmpfs -Options=mode=1777,strictatime,nosuid,nodev,size=50%%,nr_inodes=1m -GracefulOptions=usrquota +Options=mode=1777,strictatime,nosuid,nodev,size=50%%,nr_inodes=1m,x-systemd.graceful-option=usrquota From c2198d0c3fdbebd30b0653f2e3d148372c5fff31 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 12 Feb 2025 15:44:13 +0100 Subject: [PATCH 09/10] mountpoint-util: assume fsopen() works in mount_option_supported() Our baseline includes it now. --- src/basic/mountpoint-util.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/basic/mountpoint-util.c b/src/basic/mountpoint-util.c index aac8b19430..d366b3aa51 100644 --- a/src/basic/mountpoint-util.c +++ b/src/basic/mountpoint-util.c @@ -797,14 +797,10 @@ int mount_option_supported(const char *fstype, const char *key, const char *valu assert(key); fd = fsopen(fstype, FSOPEN_CLOEXEC); - if (fd < 0) { - if (ERRNO_IS_NOT_SUPPORTED(errno)) - return -EAGAIN; /* new mount API not available → don't know */ - + if (fd < 0) return log_debug_errno(errno, "Failed to open superblock context for '%s': %m", fstype); - } - /* Various file systems have not been converted to the new mount API yet. For such file systems + /* Various file systems support fs context only in recent kernels (e.g. btrfs). For older kernels * fsconfig() with FSCONFIG_SET_STRING/FSCONFIG_SET_FLAG never fail. Which sucks, because we want to * use it for testing support, after all. Let's hence do a check if the file system got converted yet * first. */ @@ -813,9 +809,9 @@ int mount_option_supported(const char *fstype, const char *key, const char *valu * the new mount API yet. If it returns EINVAL the mount option doesn't exist, but the fstype * is converted. */ if (errno == EOPNOTSUPP) - return -EAGAIN; /* FSCONFIG_SET_FD not supported on the fs, hence not converted to new mount API → don't know */ + return -EAGAIN; /* fs not converted to new mount API → don't know */ if (errno != EINVAL) - return log_debug_errno(errno, "Failed to check if file system has been converted to new mount API: %m"); + return log_debug_errno(errno, "Failed to check if file system '%s' has been converted to new mount API: %m", fstype); /* So FSCONFIG_SET_FD worked, but the option didn't exist (we got EINVAL), this means the fs * is converted. Let's now ask the actual question we wonder about. */ From f565e5a94a778979172705e8c3a8581ec9f15865 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 11 Feb 2025 19:44:59 +0100 Subject: [PATCH 10/10] core/mount: log only once about fs not supporting new mount API --- src/core/mount.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/core/mount.c b/src/core/mount.c index 6610c02c2c..d7557bf389 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1104,6 +1104,12 @@ static int mount_apply_graceful_options(Mount *m, const MountParameters *p, char return r; r = mount_option_supported(p->fstype, k ?: *o, v); + if (r == -EAGAIN) { + log_unit_warning_errno(UNIT(m), r, + "x-systemd.graceful-option= used but not supported by file system %s, suppressing all.", + p->fstype); + break; + } if (r < 0) log_unit_warning_errno(UNIT(m), r, "x-systemd.graceful-option=%s specified, but cannot determine availability, suppressing: %m", *o);