Martin Pitt [2015-05-17 15:54 +0200]: > This fixes the original "systemd immediately unmounts my mounts" bug, > but not for very long: If you remount or unmount just one mount on a > tentative device, mountinfo changes, and device_found_node() now calls > device_update_found_one() with "add == 0". Then > > n = add ? (d->found | found) : (d->found & ~found); > > would unset the previous DEVICE_FOUND_MOUNT flag, leaving 0 (i. e. > DEVICE_NOT_FOUND). Thus the previously "tentative" device would once > again be set to "dead", and systemd would unmount all other mounts > from it. This must never happen, we simply can't declare tentative > devices as dead and clean up their unmounts, this only works for > "plugged" ones that we know via udev.
Eek, I attached the wrong 0002- patch, sorry about that. The above is fixed by the attached patch, please ignore 0002- from the previous mail. For completeness I also re-attach 0001- again (unchanged). Martin -- Martin Pitt | http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From bc080c3a0ddd24fabd94d11dc609967d557d044d Mon Sep 17 00:00:00 2001 From: Martin Pitt <martin.p...@ubuntu.com> Date: Sun, 17 May 2015 15:07:47 +0200 Subject: [PATCH 1/3] device: create units with intended "found" value Change device_found_node() to also create a .device unit if a device is not known by udev; this is the case for "tentative" devices picked up by mountinfo (DEVICE_FOUND_MOUNT). With that we can record the "found" attribute on the unit. Change device_setup_unit() to also accept a NULL udev_device, and don't add the extra udev information in that case. Previously device_found_node() would not create a .device unit, and unit_add_node_link() would then create a "dead" stub one via manager_load_unit(), so we lost the "found" attribute and unmounted everything from that device. https://launchpad.net/bugs/1444402 http://lists.freedesktop.org/archives/systemd-devel/2015-May/031658.html --- src/core/device.c | 53 ++++++++++++++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/src/core/device.c b/src/core/device.c index 8eca4c2..3fddc62 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -292,26 +292,28 @@ static int device_add_udev_wants(Unit *u, struct udev_device *dev) { static int device_setup_unit(Manager *m, struct udev_device *dev, const char *path, bool main) { _cleanup_free_ char *e = NULL; - const char *sysfs; + const char *sysfs = NULL; Unit *u = NULL; bool delete; int r; assert(m); - assert(dev); assert(path); - sysfs = udev_device_get_syspath(dev); - if (!sysfs) - return 0; - r = unit_name_from_path(path, ".device", &e); if (r < 0) return log_error_errno(r, "Failed to generate unit name from device path: %m"); u = manager_get_unit(m, e); + if (dev) { + sysfs = udev_device_get_syspath(dev); + if (!sysfs) + return 0; + } + if (u && + sysfs && DEVICE(u)->sysfs && !path_equal(DEVICE(u)->sysfs, sysfs)) { log_unit_debug(u, "Device %s appeared twice with different sysfs paths %s and %s", e, DEVICE(u)->sysfs, sysfs); @@ -336,17 +338,19 @@ static int device_setup_unit(Manager *m, struct udev_device *dev, const char *pa /* If this was created via some dependency and has not * actually been seen yet ->sysfs will not be * initialized. Hence initialize it if necessary. */ + if (sysfs) { + r = device_set_sysfs(DEVICE(u), sysfs); + if (r < 0) + goto fail; - r = device_set_sysfs(DEVICE(u), sysfs); - if (r < 0) - goto fail; + (void) device_update_description(u, dev, path); - (void) device_update_description(u, dev, path); + /* The additional systemd udev properties we only interpret + * for the main object */ + if (main) + (void) device_add_udev_wants(u, dev); + } - /* The additional systemd udev properties we only interpret - * for the main object */ - if (main) - (void) device_add_udev_wants(u, dev); /* Note that this won't dispatch the load queue, the caller * has to do that if needed and appropriate */ @@ -788,22 +792,17 @@ int device_found_node(Manager *m, const char *node, bool add, DeviceFound found, * will still have a device node even when the medium * is not there... */ - if (stat(node, &st) < 0) { - if (errno == ENOENT) + if (stat(node, &st) >= 0) { + if (!S_ISBLK(st.st_mode) && !S_ISCHR(st.st_mode)) return 0; - return log_error_errno(errno, "Failed to stat device node file %s: %m", node); - } - - if (!S_ISBLK(st.st_mode) && !S_ISCHR(st.st_mode)) - return 0; + dev = udev_device_new_from_devnum(m->udev, S_ISBLK(st.st_mode) ? 'b' : 'c', st.st_rdev); + if (!dev && errno != ENOENT) + return log_oom(); - dev = udev_device_new_from_devnum(m->udev, S_ISBLK(st.st_mode) ? 'b' : 'c', st.st_rdev); - if (!dev) { - if (errno == ENOENT) - return 0; - - return log_oom(); + } else { + if (errno != ENOENT) + return log_error_errno(errno, "Failed to stat device node file %s: %m", node); } /* If the device is known in the kernel and newly -- 2.1.4
From c18dbd432ecd16c57123b5fc04313d625e8a8e88 Mon Sep 17 00:00:00 2001 From: Martin Pitt <martin.p...@ubuntu.com> Date: Sun, 17 May 2015 15:17:58 +0200 Subject: [PATCH 2/3] device: never transition from "tentative" to "dead" Only set a device to state "dead" if it was previously plugged through udev. We must never transition from "tentative" to "dead", as there is no way to be sure that this is actually true. This fixes systemd unmounting everything from a tentative device as soon as mountinfo changes. https://launchpad.net/bugs/1444402 http://lists.freedesktop.org/archives/systemd-devel/2015-May/031658.html --- src/core/device.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/core/device.c b/src/core/device.c index 3fddc62..c4a5e65 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -465,12 +465,13 @@ static void device_update_found_one(Device *d, bool add, DeviceFound found, bool * now referenced by the kernel, then we assume the * kernel knows it now, and udev might soon too. */ device_set_state(d, DEVICE_TENTATIVE); - else - /* If nobody sees the device, or if the device was - * previously seen by udev and now is only referenced - * from the kernel, then we consider the device is + else if (previous & DEVICE_FOUND_UDEV) + /* If the device was previously seen by udev and now is only + * referenced from the kernel, then we consider the device is * gone, the kernel just hasn't noticed it yet. */ device_set_state(d, DEVICE_DEAD); + /* We never move from TENTATIVE to DEAD, as we can only determine this + * status update with udev, not with mountinfo */ } static int device_update_found_by_sysfs(Manager *m, const char *sysfs, bool add, DeviceFound found, bool now) { -- 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
