Commit c9f3281e authored by Martin Pitt's avatar Martin Pitt
Browse files

Recognize and respect the "discard" mount option for swap devices

Cherry-picked patches from trunk (fixed in 217).

Thanks to Aurelien Jarno for finding and testing!

Closes: #769734
parent bc123be2
......@@ -15,6 +15,9 @@ systemd (215-7) UNRELEASED; urgency=medium
in 215).
* sysv-generator: Avoid wrong dependencies for failing units. Thanks to
Michael Biebl for the patch! (Closes: #771118)
* Cherry-pick patches to recognize and respect the "discard" mount option
for swap devices. Thanks to Aurelien Jarno for finding and testing!
(Closes: #769734)
[ Jon Severinsson]
* Add /run/shm -> /dev/shm symlink in debian/tmpfiles.d/debian.conf. This
......
From: =?utf-8?q?Zbigniew_J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Sun, 28 Sep 2014 22:13:07 -0400
Subject: core/swap: advertise Discard over dbus
---
src/core/dbus-swap.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/src/core/dbus-swap.c b/src/core/dbus-swap.c
index 93eae53..c854716 100644
--- a/src/core/dbus-swap.c
+++ b/src/core/dbus-swap.c
@@ -55,12 +55,36 @@ static int property_get_priority(
return sd_bus_message_append(reply, "i", p);
}
+static int property_get_discard(
+ sd_bus *bus,
+ const char *path,
+ const char *interface,
+ const char *property,
+ sd_bus_message *reply,
+ void *userdata,
+ sd_bus_error *error) {
+
+ Swap *s = SWAP(userdata);
+ const char *p;
+
+ assert(bus);
+ assert(reply);
+ assert(s);
+
+ if (s->from_fragment)
+ p = s->parameters_fragment.discard ?: "none";
+ else
+ p = "none";
+ return sd_bus_message_append(reply, "s", p);
+}
+
static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_result, swap_result, SwapResult);
const sd_bus_vtable bus_swap_vtable[] = {
SD_BUS_VTABLE_START(0),
SD_BUS_PROPERTY("What", "s", NULL, offsetof(Swap, what), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
SD_BUS_PROPERTY("Priority", "i", property_get_priority, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
+ SD_BUS_PROPERTY("Discard", "s", property_get_discard, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
SD_BUS_PROPERTY("TimeoutUSec", "t", bus_property_get_usec, offsetof(Swap, timeout_usec), SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY("ControlPID", "u", bus_property_get_pid, offsetof(Swap, control_pid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
SD_BUS_PROPERTY("Result", "s", property_get_result, offsetof(Swap, result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
From: =?utf-8?q?Zbigniew_J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Sun, 28 Sep 2014 10:37:52 -0400
Subject: core/swap: follow the configured unit by default
Phenomenon: parameters configured in /etc/fstab for swap units are
ignored. E.g. pri= settings have no effect when systemd starts swap
units. What is even more confusing, .swap units for the name used in
/etc/fstab initially show proper values for Priority=, but after
starting them, they are re-initalized from /proc/swaps and show the -1
value from /proc/swaps.
Change swap units to follow the original configured unit. This way
proper settings are used when starting the swap.
---
src/core/swap.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/src/core/swap.c b/src/core/swap.c
index 89f50a5..cec02f5 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -1208,11 +1208,25 @@ static Unit *swap_following(Unit *u) {
assert(s);
- if (streq_ptr(s->what, s->devnode))
+ /* If the user configured the swap through /etc/fstab or
+ * a device unit, follow that. */
+
+ if (s->from_fragment)
return NULL;
- /* Make everybody follow the unit that's named after the swap
- * device in the kernel */
+ LIST_FOREACH_AFTER(same_devnode, other, s)
+ if (other->from_fragment)
+ return UNIT(other);
+
+ LIST_FOREACH_BEFORE(same_devnode, other, s)
+ if (other->from_fragment)
+ return UNIT(other);
+
+ /* Otherwise make everybody follow the unit that's named after
+ * the swap device in the kernel */
+
+ if (streq_ptr(s->what, s->devnode))
+ return NULL;
LIST_FOREACH_AFTER(same_devnode, other, s)
if (streq_ptr(other->what, other->devnode))
@@ -1225,6 +1239,7 @@ static Unit *swap_following(Unit *u) {
first = other;
}
+ /* Fall back to the first on the list */
return UNIT(first);
}
From: =?utf-8?q?Zbigniew_J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Thu, 2 Oct 2014 00:11:36 -0400
Subject: core/swap: only make configured units part of swap.target
We used to make all .swap units either RequiredBy=swap.target or
WantedBy=swap.target. But swap.target should be the "configured swap
units", either through /etc/fstab or non-generated .swap units. It
is surprising when systemd starts treating a swap device that was
possibly temporarily enabled as a hard dependency for other units.
So do not add dependencies with swap.target for units gleaned from
/proc/swaps.
Similarly, we added dependencies for all aliases of the device name,
which clutters up the dependency graph but does not seem to bring any
value, since the status of those following units is consistent with
the main one anyway.
This should be a fix for [1], and it seems the right thing to do
anyway.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1114786
---
src/core/swap.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/src/core/swap.c b/src/core/swap.c
index cec02f5..42c2ef3 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -213,7 +213,7 @@ static int swap_add_device_links(Swap *s) {
}
static int swap_add_default_dependencies(Swap *s) {
- bool nofail = false, noauto = false;
+ bool nofail, noauto;
int r;
assert(s);
@@ -228,23 +228,25 @@ static int swap_add_default_dependencies(Swap *s) {
if (r < 0)
return r;
- if (s->from_fragment) {
- SwapParameters *p = &s->parameters_fragment;
+ if (!s->from_fragment)
+ /* The swap unit can either be for an alternative device name, in which
+ * case we don't need to add the dependency on swap.target because this unit
+ * is following a different unit which will have this dependency added,
+ * or it can be derived from /proc/swaps, in which case it was started
+ * manually, and should not become a dependency of swap.target. */
+ return 0;
- nofail = p->nofail;
- noauto = p->noauto;
- }
+ nofail = s->parameters_fragment.nofail;
+ noauto = s->parameters_fragment.noauto;
if (!noauto) {
if (nofail)
r = unit_add_dependency_by_name_inverse(UNIT(s), UNIT_WANTS, SPECIAL_SWAP_TARGET, NULL, true);
else
r = unit_add_two_dependencies_by_name_inverse(UNIT(s), UNIT_AFTER, UNIT_REQUIRES, SPECIAL_SWAP_TARGET, NULL, true);
- if (r < 0)
- return r;
}
- return 0;
+ return r < 0 ? r : 0;
}
static int swap_verify(Swap *s) {
From: =?utf-8?q?Zbigniew_J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Sat, 27 Sep 2014 22:02:04 -0400
Subject: fstab-generator: properly deal with discard as non-last option
Previous code would only return correct results when discard
was the last option.
While at it, avoid incorrect behaviour for (invalid) 'pri' option
not followed by '=...', and also do not return -1 as the error code.
---
src/core/swap.c | 2 +-
src/fstab-generator/fstab-generator.c | 73 ++++++++++++++++++++++++++---------
2 files changed, 55 insertions(+), 20 deletions(-)
diff --git a/src/core/swap.c b/src/core/swap.c
index 3640c48..89f50a5 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -610,7 +610,7 @@ static void swap_dump(Unit *u, FILE *f, const char *prefix) {
prefix, p->priority,
prefix, yes_no(p->noauto),
prefix, yes_no(p->nofail),
- prefix, p->discard);
+ prefix, p->discard ?: "none");
if (s->control_pid > 0)
fprintf(f,
diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c
index 8e3fd4a..7d7def9 100644
--- a/src/fstab-generator/fstab-generator.c
+++ b/src/fstab-generator/fstab-generator.c
@@ -46,34 +46,70 @@ static int arg_root_rw = -1;
static int mount_find_pri(struct mntent *me, int *ret) {
- char *end, *pri;
+ char *end, *opt;
unsigned long r;
assert(me);
assert(ret);
- pri = hasmntopt(me, "pri");
- if (!pri)
+ opt = hasmntopt(me, "pri");
+ if (!opt)
return 0;
- pri += 4;
+ opt += strlen("pri");
+
+ if (*opt != '=')
+ return -EINVAL;
errno = 0;
- r = strtoul(pri, &end, 10);
+ r = strtoul(opt + 1, &end, 10);
if (errno > 0)
return -errno;
- if (end == pri || (*end != ',' && *end != 0))
+ if (end == opt + 1 || (*end != ',' && *end != 0))
return -EINVAL;
*ret = (int) r;
return 1;
}
+static int mount_find_discard(struct mntent *me, char **ret) {
+ char *opt, *ans;
+ size_t len;
+
+ assert(me);
+ assert(ret);
+
+ opt = hasmntopt(me, "discard");
+ if (!opt)
+ return 0;
+
+ opt += strlen("discard");
+
+ if (*opt == ',' || *opt == '\0')
+ ans = strdup("all");
+ else {
+ if (*opt != '=')
+ return -EINVAL;
+
+ len = strcspn(opt + 1, ",");
+ if (len == 0)
+ return -EINVAL;
+
+ ans = strndup(opt + 1, len);
+ }
+
+ if (!ans)
+ return -ENOMEM;
+
+ *ret = ans;
+ return 1;
+}
+
static int add_swap(const char *what, struct mntent *me) {
_cleanup_free_ char *name = NULL, *unit = NULL, *lnk = NULL;
_cleanup_fclose_ FILE *f = NULL;
- char *discard = NULL;
+ _cleanup_free_ char *discard = NULL;
bool noauto;
int r, pri = -1;
@@ -89,7 +125,13 @@ static int add_swap(const char *what, struct mntent *me) {
r = mount_find_pri(me, &pri);
if (r < 0) {
log_error("Failed to parse priority");
- return pri;
+ return r;
+ }
+
+ r = mount_find_discard(me, &discard);
+ if (r < 0) {
+ log_error("Failed to parse discard");
+ return r;
}
noauto = !!hasmntopt(me, "noauto");
@@ -120,18 +162,11 @@ static int add_swap(const char *what, struct mntent *me) {
"What=%s\n",
what);
- discard = mount_test_option(me->mnt_opts, "discard");
- if (discard) {
- discard = strpbrk(discard, "=");
- fprintf(f,
- "Discard=%s\n",
- discard ? discard+1 : "all");
- }
-
if (pri >= 0)
- fprintf(f,
- "Priority=%i\n",
- pri);
+ fprintf(f, "Priority=%i\n", pri);
+
+ if (discard)
+ fprintf(f, "Discard=%s\n", discard);
fflush(f);
if (ferror(f)) {
......@@ -112,6 +112,12 @@ udev-hwdb-Support-shipping-pre-compiled-database-in-.patch
tmpfiles.d-Create-var-lib-container.patch
nspawn-Add-try-host-guest-journal-link-modes.patch
selinux-access-fix-broken-ternary-operator.patch
swap-introduce-Discard-property.patch
fstab-generator-properly-deal-with-discard-as-non-la.patch
core-swap-follow-the-configured-unit-by-default.patch
core-swap-advertise-Discard-over-dbus.patch
core-swap-only-make-configured-units-part-of-swap.ta.patch
swap-replace-Discard-setting-by-a-more-generic-Optio.patch
## Debian specific patches:
Add-back-support-for-Debian-specific-config-files.patch
......
From: Jan Synacek <jsynacek@redhat.com>
Date: Wed, 24 Sep 2014 14:29:05 +0200
Subject: swap: introduce Discard property
Process possible "discard" values from /etc/fstab.
---
man/systemd.swap.xml | 14 ++++++++++
src/core/execute.c | 25 ++++++++++++++++++
src/core/execute.h | 1 +
src/core/load-fragment-gperf.gperf.m4 | 1 +
src/core/swap.c | 49 +++++++++++++++++++++++------------
src/core/swap.h | 1 +
src/fstab-generator/fstab-generator.c | 10 +++++++
7 files changed, 84 insertions(+), 17 deletions(-)
diff --git a/man/systemd.swap.xml b/man/systemd.swap.xml
index 3c36d5d..7ffcd88 100644
--- a/man/systemd.swap.xml
+++ b/man/systemd.swap.xml
@@ -171,6 +171,20 @@
</varlistentry>
<varlistentry>
+ <term><varname>Discard=</varname></term>
+
+ <listitem><para>Enable discards, if the swap
+ backing device supports the discard or trim
+ operation. Can be one of <literal>none</literal>,
+ <literal>once</literal>, <literal>pages</literal>
+ or <literal>all</literal>. Defaults to
+ <literal>none</literal>. (See
+ <citerefentry><refentrytitle>swapon</refentrytitle><manvolnum>8</manvolnum></citerefentry>
+ for more information.)
+ </para></listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><varname>TimeoutSec=</varname></term>
<listitem><para>Configures the time to
wait for the swapon command to
diff --git a/src/core/execute.c b/src/core/execute.c
index 88d094e..6b09402 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -2538,6 +2538,31 @@ int exec_command_set(ExecCommand *c, const char *path, ...) {
return 0;
}
+int exec_command_append(ExecCommand *c, const char *path, ...) {
+ va_list ap;
+ char **l;
+ int r;
+
+ assert(c);
+ assert(path);
+
+ va_start(ap, path);
+ l = strv_new_ap(path, ap);
+ va_end(ap);
+
+ if (!l)
+ return -ENOMEM;
+
+ r = strv_extend_strv(&c->argv, l);
+ if (r < 0) {
+ strv_free(l);
+ return r;
+ }
+
+ return 0;
+}
+
+
static int exec_runtime_allocate(ExecRuntime **rt) {
if (*rt)
diff --git a/src/core/execute.h b/src/core/execute.h
index 9d05d3a..5d9e76e 100644
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -221,6 +221,7 @@ void exec_command_dump(ExecCommand *c, FILE *f, const char *prefix);
void exec_command_dump_list(ExecCommand *c, FILE *f, const char *prefix);
void exec_command_append_list(ExecCommand **l, ExecCommand *e);
int exec_command_set(ExecCommand *c, const char *path, ...);
+int exec_command_append(ExecCommand *c, const char *path, ...);
void exec_context_init(ExecContext *c);
void exec_context_done(ExecContext *c);
diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4
index a7c4469..15ca39a 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -285,6 +285,7 @@ Automount.DirectoryMode, config_parse_mode, 0,
m4_dnl
Swap.What, config_parse_path, 0, offsetof(Swap, parameters_fragment.what)
Swap.Priority, config_parse_int, 0, offsetof(Swap, parameters_fragment.priority)
+Swap.Discard, config_parse_string, 0, offsetof(Swap, parameters_fragment.discard)
Swap.TimeoutSec, config_parse_sec, 0, offsetof(Swap, timeout_usec)
EXEC_CONTEXT_CONFIG_ITEMS(Swap)m4_dnl
CGROUP_CONTEXT_CONFIG_ITEMS(Swap)m4_dnl
diff --git a/src/core/swap.c b/src/core/swap.c
index 9f353af..3640c48 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -152,6 +152,9 @@ static void swap_done(Unit *u) {
free(s->parameters_fragment.what);
s->parameters_fragment.what = NULL;
+ free(s->parameters_fragment.discard);
+ s->parameters_fragment.discard = NULL;
+
s->exec_runtime = exec_runtime_unref(s->exec_runtime);
exec_command_done_array(s->exec_command, _SWAP_EXEC_COMMAND_MAX);
s->control_command = NULL;
@@ -602,10 +605,12 @@ static void swap_dump(Unit *u, FILE *f, const char *prefix) {
fprintf(f,
"%sPriority: %i\n"
"%sNoAuto: %s\n"
- "%sNoFail: %s\n",
+ "%sNoFail: %s\n"
+ "%sDiscard: %s\n",
prefix, p->priority,
prefix, yes_no(p->noauto),
- prefix, yes_no(p->nofail));
+ prefix, yes_no(p->nofail),
+ prefix, p->discard);
if (s->control_pid > 0)
fprintf(f,
@@ -734,36 +739,46 @@ fail:
static void swap_enter_activating(Swap *s) {
int r, priority;
+ char *discard;
assert(s);
s->control_command_id = SWAP_EXEC_ACTIVATE;
s->control_command = s->exec_command + SWAP_EXEC_ACTIVATE;
- if (s->from_fragment)
+ if (s->from_fragment) {
priority = s->parameters_fragment.priority;
- else
+ discard = s->parameters_fragment.discard;
+ } else {
priority = -1;
+ discard = NULL;
+ }
+
+ r = exec_command_set(s->control_command, "/sbin/swapon", NULL);
+ if (r < 0)
+ goto fail;
if (priority >= 0) {
char p[DECIMAL_STR_MAX(int)];
sprintf(p, "%i", priority);
+ r = exec_command_append(s->control_command, "-p", p, NULL);
+ if (r < 0)
+ goto fail;
+ }
- r = exec_command_set(
- s->control_command,
- "/sbin/swapon",
- "-p",
- p,
- s->what,
- NULL);
- } else
- r = exec_command_set(
- s->control_command,
- "/sbin/swapon",
- s->what,
- NULL);
+ if (discard && !streq(discard, "none")) {
+ const char *discard_arg = "--discard";
+
+ if (!streq(discard, "all"))
+ discard_arg = strappenda("--discard=", discard);
+
+ r = exec_command_append(s->control_command, discard_arg, NULL);
+ if (r < 0)
+ goto fail;
+ }
+ r = exec_command_append(s->control_command, s->what, NULL);
if (r < 0)
goto fail;
diff --git a/src/core/swap.h b/src/core/swap.h
index f2ae49b..3482d65 100644
--- a/src/core/swap.h
+++ b/src/core/swap.h
@@ -63,6 +63,7 @@ typedef enum SwapResult {
typedef struct SwapParameters {
char *what;
+ char *discard;
int priority;
bool noauto:1;
bool nofail:1;
diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c
index 21d30c8..8e3fd4a 100644
--- a/src/fstab-generator/fstab-generator.c
+++ b/src/fstab-generator/fstab-generator.c
@@ -73,6 +73,8 @@ static int mount_find_pri(struct mntent *me, int *ret) {
static int add_swap(const char *what, struct mntent *me) {
_cleanup_free_ char *name = NULL, *unit = NULL, *lnk = NULL;
_cleanup_fclose_ FILE *f = NULL;
+ char *discard = NULL;
+
bool noauto;
int r, pri = -1;
@@ -118,6 +120,14 @@ static int add_swap(const char *what, struct mntent *me) {
"What=%s\n",
what);
+ discard = mount_test_option(me->mnt_opts, "discard");
+ if (discard) {
+ discard = strpbrk(discard, "=");
+ fprintf(f,
+ "Discard=%s\n",
+ discard ? discard+1 : "all");
+ }
+
if (pri >= 0)
fprintf(f,
"Priority=%i\n",
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 28 Oct 2014 14:24:46 +0100
Subject: swap: replace Discard= setting by a more generic Options= setting
For now, it's systemd itself that parses the options string, but as soon
as util-linux' swapon can take the option string directly with -o we
should pass it on unmodified.
---
man/systemd.swap.xml | 16 +++---
src/core/dbus-swap.c | 13 +++--
src/core/load-fragment-gperf.gperf.m4 | 2 +-
src/core/swap.c | 91 ++++++++++++++++++++++++++++++-----
src/core/swap.h | 2 +-
src/fstab-generator/fstab-generator.c | 56 +++------------------
6 files changed, 103 insertions(+), 77 deletions(-)
diff --git a/man/systemd.swap.xml b/man/systemd.swap.xml
index 7ffcd88..42b0eed 100644
--- a/man/systemd.swap.xml
+++ b/man/systemd.swap.xml
@@ -171,14 +171,14 @@
</varlistentry>
<varlistentry>
- <term><varname>Discard=</varname></term>
-
- <listitem><para>Enable discards, if the swap
- backing device supports the discard or trim
- operation. Can be one of <literal>none</literal>,
- <literal>once</literal>, <literal>pages</literal>
- or <literal>all</literal>. Defaults to
- <literal>none</literal>. (See