ryantimwilson / rpms / systemd

Forked from rpms/systemd a month ago
Clone
Jan Synacek 2ec3b0
From a5866298597e2fd460c47d9fda7cceacbe9f3fcb Mon Sep 17 00:00:00 2001
Jan Synacek 2ec3b0
From: Harald Hoyer <harald@redhat.com>
Jan Synacek 2ec3b0
Date: Tue, 24 Nov 2015 09:41:26 +0100
Jan Synacek 2ec3b0
Subject: [PATCH 4/4] core: Do not bind a mount unit to a device, if it was
Jan Synacek 2ec3b0
 from mountinfo
Jan Synacek 2ec3b0
Jan Synacek 2ec3b0
If a mount unit is bound to a device, systemd tries to umount the
Jan Synacek 2ec3b0
mount point, if it thinks the device has gone away.
Jan Synacek 2ec3b0
Jan Synacek 2ec3b0
Due to the uevent queue and inotify of /proc/self/mountinfo being two
Jan Synacek 2ec3b0
different sources, systemd can never get the ordering reliably correct.
Jan Synacek 2ec3b0
Jan Synacek 2ec3b0
It can happen, that in the uevent queue ADD,REMOVE,ADD is queued
Jan Synacek 2ec3b0
and an inotify of mountinfo (or libmount event) happend with the
Jan Synacek 2ec3b0
device in question.
Jan Synacek 2ec3b0
Jan Synacek 2ec3b0
systemd cannot know, at which point of time the mount happend in the
Jan Synacek 2ec3b0
ADD,REMOVE,ADD sequence.
Jan Synacek 2ec3b0
Jan Synacek 2ec3b0
The real ordering might have been ADD,REMOVE,ADD,mount
Jan Synacek 2ec3b0
and systemd might think ADD,mount,REMOVE,ADD and would umount the
Jan Synacek 2ec3b0
mountpoint.
Jan Synacek 2ec3b0
Jan Synacek 2ec3b0
A test script which triggered this behaviour is:
Jan Synacek 2ec3b0
rm -f test-efi-disk.img
Jan Synacek 2ec3b0
dd if=/dev/null of=test-efi-disk.img bs=1M seek=512 count=1
Jan Synacek 2ec3b0
parted --script test-efi-disk.img \
Jan Synacek 2ec3b0
  "mklabel gpt" \
Jan Synacek 2ec3b0
  "mkpart ESP fat32 1MiB 511MiB" \
Jan Synacek 2ec3b0
  "set 1 boot on"
Jan Synacek 2ec3b0
LOOP=$(losetup --show -f -P test-efi-disk.img)
Jan Synacek 2ec3b0
udevadm settle
Jan Synacek 2ec3b0
mkfs.vfat -F32 ${LOOP}p1
Jan Synacek 2ec3b0
mkdir -p mnt
Jan Synacek 2ec3b0
mount ${LOOP}p1 mnt
Jan Synacek 2ec3b0
... <dostuffwith mnt>
Jan Synacek 2ec3b0
Jan Synacek 2ec3b0
Without the "udevadm settle" systemd unmounted mnt while the script was
Jan Synacek 2ec3b0
operating on mnt.
Jan Synacek 2ec3b0
Jan Synacek 2ec3b0
Of course the question is, why there was a REMOVE in the first place,
Jan Synacek 2ec3b0
but this is not part of this patch.
Jan Synacek 2ec3b0
Jan Synacek 2ec3b0
(cherry picked from commit 9d06297e262966de71095debd1537fc223f940a3)
Jan Synacek 2ec3b0
Jan Synacek 2ec3b0
Note: I'm adding a Related note, but I'm not entirely sure if it really is...
Jan Synacek 2ec3b0
Jan Synacek 2ec3b0
Related: #1195761
Jan Synacek 2ec3b0
---
Jan Synacek 2ec3b0
 src/core/mount.c  | 2 +-
Jan Synacek 2ec3b0
 src/core/socket.c | 2 +-
Jan Synacek 2ec3b0
 src/core/swap.c   | 2 +-
Jan Synacek 2ec3b0
 src/core/unit.c   | 6 ++++--
Jan Synacek 2ec3b0
 src/core/unit.h   | 2 +-
Jan Synacek 2ec3b0
 5 files changed, 8 insertions(+), 6 deletions(-)
Jan Synacek 2ec3b0
Jan Synacek 2ec3b0
diff --git a/src/core/mount.c b/src/core/mount.c
Jan Synacek 2ec3b0
index 9b44357..2ad4ad4 100644
Jan Synacek 2ec3b0
--- a/src/core/mount.c
Jan Synacek 2ec3b0
+++ b/src/core/mount.c
Jan Synacek 2ec3b0
@@ -335,7 +335,7 @@ static int mount_add_device_links(Mount *m) {
Jan Synacek 2ec3b0
         if (mount_is_auto(p) && UNIT(m)->manager->running_as == MANAGER_SYSTEM)
Jan Synacek 2ec3b0
                 device_wants_mount = true;
Jan Synacek 2ec3b0
 
Jan Synacek 2ec3b0
-        r = unit_add_node_link(UNIT(m), p->what, device_wants_mount);
Jan Synacek 2ec3b0
+        r = unit_add_node_link(UNIT(m), p->what, device_wants_mount, m->from_fragment ? UNIT_BINDS_TO : UNIT_REQUIRES);
Jan Synacek 2ec3b0
         if (r < 0)
Jan Synacek 2ec3b0
                 return r;
Jan Synacek 2ec3b0
 
Jan Synacek 2ec3b0
diff --git a/src/core/socket.c b/src/core/socket.c
Jan Synacek 2ec3b0
index 687675b..860a1e3 100644
Jan Synacek 2ec3b0
--- a/src/core/socket.c
Jan Synacek 2ec3b0
+++ b/src/core/socket.c
Jan Synacek 2ec3b0
@@ -289,7 +289,7 @@ static int socket_add_device_link(Socket *s) {
Jan Synacek 2ec3b0
                 return 0;
Jan Synacek 2ec3b0
 
Jan Synacek 2ec3b0
         t = strjoina("/sys/subsystem/net/devices/", s->bind_to_device);
Jan Synacek 2ec3b0
-        return unit_add_node_link(UNIT(s), t, false);
Jan Synacek 2ec3b0
+        return unit_add_node_link(UNIT(s), t, false, UNIT_BINDS_TO);
Jan Synacek 2ec3b0
 }
Jan Synacek 2ec3b0
 
Jan Synacek 2ec3b0
 static int socket_add_default_dependencies(Socket *s) {
Jan Synacek 2ec3b0
diff --git a/src/core/swap.c b/src/core/swap.c
Jan Synacek 2ec3b0
index b6e4372..5568898 100644
Jan Synacek 2ec3b0
--- a/src/core/swap.c
Jan Synacek 2ec3b0
+++ b/src/core/swap.c
Jan Synacek 2ec3b0
@@ -202,7 +202,7 @@ static int swap_add_device_links(Swap *s) {
Jan Synacek 2ec3b0
                 return 0;
Jan Synacek 2ec3b0
 
Jan Synacek 2ec3b0
         if (is_device_path(s->what))
Jan Synacek 2ec3b0
-                return unit_add_node_link(UNIT(s), s->what, UNIT(s)->manager->running_as == MANAGER_SYSTEM);
Jan Synacek 2ec3b0
+                return unit_add_node_link(UNIT(s), s->what, UNIT(s)->manager->running_as == MANAGER_SYSTEM, UNIT_BINDS_TO);
Jan Synacek 2ec3b0
         else
Jan Synacek 2ec3b0
                 /* File based swap devices need to be ordered after
Jan Synacek 2ec3b0
                  * systemd-remount-fs.service, since they might need a
Jan Synacek 2ec3b0
diff --git a/src/core/unit.c b/src/core/unit.c
Jan Synacek 2ec3b0
index 0a02e38..e6e67d2 100644
Jan Synacek 2ec3b0
--- a/src/core/unit.c
Jan Synacek 2ec3b0
+++ b/src/core/unit.c
Jan Synacek 2ec3b0
@@ -2840,7 +2840,7 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) {
Jan Synacek 2ec3b0
         }
Jan Synacek 2ec3b0
 }
Jan Synacek 2ec3b0
 
Jan Synacek 2ec3b0
-int unit_add_node_link(Unit *u, const char *what, bool wants) {
Jan Synacek 2ec3b0
+int unit_add_node_link(Unit *u, const char *what, bool wants, UnitDependency dep) {
Jan Synacek 2ec3b0
         Unit *device;
Jan Synacek 2ec3b0
         _cleanup_free_ char *e = NULL;
Jan Synacek 2ec3b0
         int r;
Jan Synacek 2ec3b0
@@ -2867,7 +2867,9 @@ int unit_add_node_link(Unit *u, const char *what, bool wants) {
Jan Synacek 2ec3b0
         if (r < 0)
Jan Synacek 2ec3b0
                 return r;
Jan Synacek 2ec3b0
 
Jan Synacek 2ec3b0
-        r = unit_add_two_dependencies(u, UNIT_AFTER, u->manager->running_as == MANAGER_SYSTEM ? UNIT_BINDS_TO : UNIT_WANTS, device, true);
Jan Synacek 2ec3b0
+        r = unit_add_two_dependencies(u, UNIT_AFTER,
Jan Synacek 2ec3b0
+                                      u->manager->running_as == MANAGER_SYSTEM ? dep : UNIT_WANTS,
Jan Synacek 2ec3b0
+                                      device, true);
Jan Synacek 2ec3b0
         if (r < 0)
Jan Synacek 2ec3b0
                 return r;
Jan Synacek 2ec3b0
 
Jan Synacek 2ec3b0
diff --git a/src/core/unit.h b/src/core/unit.h
Jan Synacek 2ec3b0
index 1681bbf..3eb3484 100644
Jan Synacek 2ec3b0
--- a/src/core/unit.h
Jan Synacek 2ec3b0
+++ b/src/core/unit.h
Jan Synacek 2ec3b0
@@ -528,7 +528,7 @@ int unit_serialize_item_escaped(Unit *u, FILE *f, const char *key, const char *v
Jan Synacek 2ec3b0
 int unit_serialize_item_fd(Unit *u, FILE *f, FDSet *fds, const char *key, int fd);
Jan Synacek 2ec3b0
 void unit_serialize_item_format(Unit *u, FILE *f, const char *key, const char *value, ...) _printf_(4,5);
Jan Synacek 2ec3b0
 
Jan Synacek 2ec3b0
-int unit_add_node_link(Unit *u, const char *what, bool wants);
Jan Synacek 2ec3b0
+int unit_add_node_link(Unit *u, const char *what, bool wants, UnitDependency d);
Jan Synacek 2ec3b0
 
Jan Synacek 2ec3b0
 int unit_coldplug(Unit *u);
Jan Synacek 2ec3b0
 
Jan Synacek 2ec3b0
-- 
Jan Synacek 2ec3b0
2.5.0
Jan Synacek 2ec3b0