Hello,
Lennart Poettering [2015-05-18 22:57 +0200]:
+if (dev) {
+sysfs = udev_device_get_syspath(dev);
+if (!sysfs)
+return 0;
+}
Why move this down? In order to keep the patch small and easy to grok,
can this stay up where it was, but simply be enclosed in the if (dev) {}
check?
Right, that was probably just the result of multiple edits/iterations;
moved back.
-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();
Hmm, why are all these errors supposed to be OOM?
Not sure, perhaps hysterical raisins? But..
udev_device_new_from_devnum sets errno correctly, hence we should just
print what it really is set to with log_error_errno(), unless of
course it is ENOENT.
Makes sense, done now. It's an unrelated change to this patch, but if
you don't mind let's clean it up.
+} else {
+if (errno != ENOENT)
+return log_error_errno(errno, Failed to
stat device node file %s: %m, node);
The if else { and if (errn... lines could be merged into one
else if (errno != ..., no?
Right, done.
Updated patch attached. It doesn't look significantly smaller, mostly
because lots of it is an indentation change :/
Re-tested on a normal system, nspawn, LXC, and with ejecting a mounted
CD.
Subject: [PATCH 2/3] device: never transition from tentative to dead
Not following on this patch
Will reply on your other response. TL/DR: Current code in master is
overzealous, this patch is overcautious, this needs some more thought
and work; I don't have an updated patch yet.
Martin
--
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 8bbd9d1df6877867ce7958c2e51574b3e74c68e6 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] 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/102
http://lists.freedesktop.org/archives/systemd-devel/2015-May/031658.html
---
src/core/device.c | 51 ---
1 file changed, 24 insertions(+), 27 deletions(-)
diff --git a/src/core/device.c b/src/core/device.c
index 8eca4c2..c784cab 100644
--- a/src/core/device.c
+++ b/src/core/device.c
@@ -292,18 +292,19 @@ 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;
+if (dev) {
+sysfs = udev_device_get_syspath(dev);
+if (!sysfs)
+return 0;
+}
r = unit_name_from_path(path, .device, e);
if (r 0)
@@ -312,6 +313,7 @@ static int device_setup_unit(Manager *m, struct udev_device *dev, const char *pa
u = manager_get_unit(m, e);
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,
