Hello all, I have a better fix now.
Martin Pitt [2015-05-19 8:59 +0200]: > - Boot, dev-foo.device becomes DEVICE_FOUND_MOUNT/tentative > > - Do some more mounts from /dev/foo, e. g. > > mkdir /tmp/etc /tmp/boot > mount -o bind /etc /tmp/etc > mount -o bind /boot /tmp/boot > > (In practice, you'd probably do such bind mounts with nspawn --bind > or lxc.mount.entry) > > tmp-etc.mount and tmp-boot.mount will be BindsTo=dev-foo.device, > and dev-foo.device's status is still unchanged > (DEVICE_FOUND_MOUNT/tentative) > > - Now do umount /tmp/boot. This *also* umounts /tmp/etc! With this patch, the unit states are now as expected: Id=dev-foo.device BindsTo= BoundBy=tmp-boot.mount ActiveState=activating SubState=tentative Id=tmp-boot.mount BindsTo=dev-foo.device BoundBy= ActiveState=active SubState=mounted And /tmp/etc stays around. After manually unmounting /tmp/mount, the device state properly goes to "dead": Id=dev-foo.device BindsTo= BoundBy= ActiveState=inactive SubState=dead > So the problem is that this tentative → dead transition only works if > a device is referenced once, but causes overzealous unmounts if there > are more references. > > We need a reference count, or check if the device is bound by other > device still. Come to think of it now, we actually already have that: > > Id=dev-foo.device > BoundBy=tmp-boot.mount tmp-etc.mount > > But my patch doesn't take that into account yet. This one does now. I left a fairly comprehensive commit log as this isn't that easy to explain/understand. If it's too verbose for you I can also trim it back a bit. Thanks, Martin -- Martin Pitt | http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From bfd1d6f2032dd68d36fbba716ec0e645cb3cbf97 Mon Sep 17 00:00:00 2001
From: Martin Pitt <martin.p...@ubuntu.com>
Date: Tue, 19 May 2015 09:59:21 +0200
Subject: [PATCH 2/2] device: Fix overzealous unmounting of tentative device
mounts
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Scenario: booting with plan9 fs, a container with writable /sys, or some other
thing with a mount that references a device "/dev/foo" which isn't actually
available as /dev/foo and in udev, i. e. it is state "tentative".
- Boot, dev-foo.device becomes DEVICE_FOUND_MOUNT/tentative
- Do some more mounts from /dev/foo, e. g.
mkdir /tmp/etc /tmp/boot
mount -o bind /etc /tmp/etc
mount -o bind /boot /tmp/boot
(In practice, you'd probably do such bind mounts with nspawn --bind
or lxc.mount.entry)
tmp-etc.mount and tmp-boot.mount will be BindsTo=dev-foo.device,
and dev-foo.device's status is still unchanged
(DEVICE_FOUND_MOUNT/tentative)
- Now do umount /tmp/boot. This *also* umounts /tmp/etc!
Journal says
systemd[1]: tmp-boot.mount: Changed mounted -> dead
systemd[1]: dev-foo.device: Changed tentative -> dead
systemd[1]: tmp-etc.mount: Changed mounted -> unmounting
systemd[1]: Unmounted /tmp/etc.
and dev-foo.device is now inactive/dead, which tears down
the bound tmp-etc.mount. Boom!
Reason: Unmounting /tmp/boot triggers
device_update_found_one(found==DEVICE_FOUND_MOUNT, add==0).
d->found previously was DEVICE_FOUND_MOUNT, and this
n = add ? (d->found | found) : (d->found & ~found);
computes the new state to 0 (i. e. DEVICE_NOT_FOUND), and calls
device_set_state(DEVICE_DEAD). Thus here we (1) don't consider that
dev-foo.device is still bound by other units (tmp-etc.mount) and (2) lose the
information that dev-foo.device is tentative.
We must only do this tentative → dead transition if there are no other mount
references to dev-foo.device any more, otherwise it must keep its state to
prevent systemd from unmounting everything else from a tentative device as soon
as one mount from it gets unmounted.
Add a check device_update_found_one() to see if there any active mounts bound
to a device when we pick up an unmount from mountinfo, and keep the state if
so.
https://launchpad.net/bugs/1444402
http://lists.freedesktop.org/archives/systemd-devel/2015-May/031658.html
http://lists.freedesktop.org/archives/systemd-devel/2015-May/031952.html
---
src/core/device.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/src/core/device.c b/src/core/device.c
index c784cab..65eb773 100644
--- a/src/core/device.c
+++ b/src/core/device.c
@@ -450,6 +450,25 @@ static void device_update_found_one(Device *d, bool add, DeviceFound found, bool
if (n == d->found)
return;
+ /* If we found a tentative device via mountinfo which is still
+ * referenced by other mounts than the one that just got unmounted, we
+ * must leave it FOUND_MOUNT/tentative; otherwise we'd trigger
+ * unmounting of all other bound mounts. */
+ if (d->found == DEVICE_FOUND_MOUNT && n == 0) {
+ Iterator i;
+ Unit *bound;
+
+ SET_FOREACH(bound, UNIT(d)->dependencies[UNIT_BOUND_BY], i) {
+ if (bound->type == UNIT_MOUNT &&
+ MOUNT(bound)->state != MOUNT_DEAD && MOUNT(bound)->state != MOUNT_FAILED) {
+ log_unit_debug(UNIT(d), "Still bound by %s (%s), keeping state.",
+ bound->id, mount_state_to_string(MOUNT(bound)->state));
+ return;
+ }
+ }
+ log_unit_debug(UNIT(d), "Not referenced by any mounts any more, changing to dead.");
+ }
+
previous = d->found;
d->found = n;
--
2.1.4
signature.asc
Description: Digital signature
_______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
