diff options
author | Yu Watanabe <watanabe.yu+github@gmail.com> | 2022-04-29 03:14:44 +0900 |
---|---|---|
committer | Yu Watanabe <watanabe.yu+github@gmail.com> | 2022-08-05 22:15:02 +0900 |
commit | 4a1a1caf21921a0c0997588552262b61eb556d19 (patch) | |
tree | 2078bc5d8db5e6db062d5005356984056e973c1f /src/core/device.c | |
parent | dce2d35ce53d2ac6ae4d1b3b1a716d6333fb84e7 (diff) | |
download | systemd-4a1a1caf21921a0c0997588552262b61eb556d19.tar.gz |
core/device: always accept syspath change
When multiple devices have the same devlink, then
adding/updating/removing one of the device may cause syspath change.
Fixes the following issue in
https://github.com/systemd/systemd/issues/23208#issue-1217909746
> the above shows an inconsistency between udev's and systemd's handling
> of the two different devices having the same alias. While udev replaces
> the by-uuid symlink which now points to sdh1 rather than sdd1, systemd
> keeps the previous mapping to sdd1 and emits a warning. This is not the
> problem cause but worth mentioning.
Diffstat (limited to 'src/core/device.c')
-rw-r--r-- | src/core/device.c | 234 |
1 files changed, 179 insertions, 55 deletions
diff --git a/src/core/device.c b/src/core/device.c index 835acd9745..e6f41588e8 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -632,16 +632,9 @@ static int device_setup_unit(Manager *m, sd_device *dev, const char *path, bool * dependency on the mount unit which was added during the loading of the later. When the device is * plugged the sysfs might not be initialized yet, as we serialize the device's state but do not * serialize the sysfs path across reloads/reexecs. Hence, when coming back from a reload/restart we - * might have the state valid, but not the sysfs path. Hence, let's filter out conflicting devices, but - * let's accept devices in any state with no sysfs path set. */ - - if (DEVICE(u)->state == DEVICE_PLUGGED && - DEVICE(u)->sysfs && - sysfs && - !path_equal(DEVICE(u)->sysfs, sysfs)) - return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EEXIST), - "Device %s appeared twice with different sysfs paths %s and %s, ignoring the latter.", - e, DEVICE(u)->sysfs, sysfs); + * might have the state valid, but not the sysfs path. Also, there is another possibility; when multiple + * devices have the same devlink (e.g. /dev/disk/by-uuid/xxxx), adding/updating/removing one of the + * device causes syspath change. Hence, let's always update sysfs path.*/ /* Let's remove all dependencies generated due to udev properties. We'll re-add whatever is configured * now below. */ @@ -719,9 +712,111 @@ static bool device_is_ready(sd_device *dev) { return r != 0; } +static int device_setup_devlink_unit_one(Manager *m, const char *devlink, sd_device **ret) { + _cleanup_(sd_device_unrefp) sd_device *dev = NULL; + int r; + + assert(m); + assert(devlink); + + if (sd_device_new_from_devname(&dev, devlink) < 0 || !device_is_ready(dev)) { + /* the devlink is already removed or not ready */ + device_update_found_by_name(m, devlink, 0, DEVICE_FOUND_UDEV); + *ret = NULL; + return 0; /* not ready */ + } + + r = device_setup_unit(m, dev, devlink, /* main = */ false); + if (r < 0) + return log_device_warning_errno(dev, r, "Failed to setup unit for '%s': %m", devlink); + + *ret = TAKE_PTR(dev); + return 1; /* ready */ +} + +static int device_setup_devlink_units(Manager *m, sd_device *dev, char ***ret_ready_devlinks) { + _cleanup_strv_free_ char **ready_devlinks = NULL; + const char *devlink, *syspath; + int r; + + assert(m); + assert(dev); + assert(ret_ready_devlinks); + + r = sd_device_get_syspath(dev, &syspath); + if (r < 0) + return r; + + FOREACH_DEVICE_DEVLINK(dev, devlink) { + _cleanup_(sd_device_unrefp) sd_device *assigned = NULL; + const char *s; + + if (PATH_STARTSWITH_SET(devlink, "/dev/block/", "/dev/char/")) + continue; + + if (device_setup_devlink_unit_one(m, devlink, &assigned) <= 0) + continue; + + if (sd_device_get_syspath(assigned, &s) < 0) + continue; + + if (path_equal(s, syspath)) + continue; + + r = strv_extend(&ready_devlinks, devlink); + if (r < 0) + return -ENOMEM; + } + + *ret_ready_devlinks = TAKE_PTR(ready_devlinks); + return 0; +} + +static int device_setup_devlink_units_on_remove(Manager *m, sd_device *dev, char ***ret_ready_devlinks) { + _cleanup_strv_free_ char **ready_devlinks = NULL; + const char *syspath; + Device *l; + int r; + + assert(m); + assert(dev); + assert(ret_ready_devlinks); + + r = sd_device_get_syspath(dev, &syspath); + if (r < 0) + return r; + + l = hashmap_get(m->devices_by_sysfs, syspath); + LIST_FOREACH(same_sysfs, d, l) { + _cleanup_(sd_device_unrefp) sd_device *assigned = NULL; + const char *s; + + if (!d->path) + continue; + + if (!path_startswith(d->path, "/dev/")) + continue; + + if (device_setup_devlink_unit_one(m, d->path, &assigned) <= 0) + continue; + + if (sd_device_get_syspath(assigned, &s) < 0) + continue; + + if (path_equal(s, syspath)) + continue; + + r = strv_extend(&ready_devlinks, d->path); + if (r < 0) + return -ENOMEM; + } + + *ret_ready_devlinks = TAKE_PTR(ready_devlinks); + return 0; +} + static void device_process_new(Manager *m, sd_device *dev, const char *sysfs) { const char *dn, *alias; - dev_t devnum; int r; assert(m); @@ -738,32 +833,6 @@ static void device_process_new(Manager *m, sd_device *dev, const char *sysfs) { if (sd_device_get_devname(dev, &dn) >= 0) (void) device_setup_unit(m, dev, dn, false); - /* Add additional units for all symlinks */ - if (sd_device_get_devnum(dev, &devnum) >= 0) { - const char *p; - - FOREACH_DEVICE_DEVLINK(dev, p) { - struct stat st; - - if (PATH_STARTSWITH_SET(p, "/dev/block/", "/dev/char/")) - continue; - - /* Verify that the symlink in the FS actually belongs to this device. This is useful - * to deal with conflicting devices, e.g. when two disks want the same - * /dev/disk/by-label/xxx link because they have the same label. We want to make sure - * that the same device that won the symlink wins in systemd, so we check the device - * node major/minor */ - if (stat(p, &st) >= 0 && - ((!S_ISBLK(st.st_mode) && !S_ISCHR(st.st_mode)) || - st.st_rdev != devnum)) { - log_device_debug(dev, "Skipping device unit creation for symlink %s not owned by device", p); - continue; - } - - (void) device_setup_unit(m, dev, p, false); - } - } - /* Add additional units for all explicitly configured aliases */ r = sd_device_get_property_value(dev, "SYSTEMD_ALIAS", &alias); if (r < 0) { @@ -907,10 +976,9 @@ static void device_enumerate(Manager *m) { } FOREACH_DEVICE(e, dev) { + _cleanup_strv_free_ char **ready_devlinks = NULL; const char *sysfs; - - if (!device_is_ready(dev)) - continue; + bool ready; r = sd_device_get_syspath(dev, &sysfs); if (r < 0) { @@ -918,8 +986,18 @@ static void device_enumerate(Manager *m) { continue; } - device_process_new(m, dev, sysfs); - device_update_found_by_sysfs(m, sysfs, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV); + ready = device_is_ready(dev); + if (ready) + device_process_new(m, dev, sysfs); + + /* Add additional units for all symlinks */ + (void) device_setup_devlink_units(m, dev, &ready_devlinks); + + if (ready) + device_update_found_by_sysfs(m, sysfs, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV); + + STRV_FOREACH(devlink, ready_devlinks) + device_update_found_by_name(m, *devlink, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV); } return; @@ -942,10 +1020,34 @@ static void device_propagate_reload_by_sysfs(Manager *m, const char *sysfs) { r = manager_propagate_reload(m, UNIT(d), JOB_REPLACE, NULL); if (r < 0) - log_warning_errno(r, "Failed to propagate reload, ignoring: %m"); + log_unit_warning_errno(UNIT(d), r, "Failed to propagate reload, ignoring: %m"); } } +static void device_propagate_reload_by_name(Manager *m, const char *path) { + _cleanup_free_ char *e = NULL; + Unit *u; + int r; + + assert(m); + assert(path); + + r = unit_name_from_path(path, ".device", &e); + if (r < 0) + return (void) log_debug_errno(r, "Failed to generate unit name from device path, ignoring: %m"); + + u = manager_get_unit(m, e); + if (!u) + return; + + if (DEVICE(u)->state == DEVICE_DEAD) + return; + + r = manager_propagate_reload(m, u, JOB_REPLACE, NULL); + if (r < 0) + log_unit_warning_errno(u, r, "Failed to propagate reload, ignoring: %m"); +} + static void device_remove_old_on_move(Manager *m, sd_device *dev) { _cleanup_free_ char *syspath_old = NULL; const char *devpath_old; @@ -966,9 +1068,11 @@ static void device_remove_old_on_move(Manager *m, sd_device *dev) { } static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *userdata) { + _cleanup_strv_free_ char **ready_devlinks = NULL; Manager *m = ASSERT_PTR(userdata); sd_device_action_t action; const char *sysfs; + bool ready; int r; assert(dev); @@ -987,9 +1091,6 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void * return 0; } - if (!IN_SET(action, SD_DEVICE_ADD, SD_DEVICE_REMOVE, SD_DEVICE_MOVE)) - device_propagate_reload_by_sysfs(m, sysfs); - if (action == SD_DEVICE_MOVE) device_remove_old_on_move(m, dev); @@ -1001,27 +1102,50 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void * if (r < 0) log_device_warning_errno(dev, r, "Failed to process swap device remove event, ignoring: %m"); - /* If we get notified that a device was removed by udev, then it's completely gone, hence - * unset all found bits */ - device_update_found_by_sysfs(m, sysfs, DEVICE_NOT_FOUND, DEVICE_FOUND_MASK); + ready = false; - } else if (device_is_ready(dev)) { + (void) device_setup_devlink_units_on_remove(m, dev, &ready_devlinks); - device_process_new(m, dev, sysfs); + } else { + ready = device_is_ready(dev); - r = swap_process_device_new(m, dev); - if (r < 0) - log_device_warning_errno(dev, r, "Failed to process swap device new event, ignoring: %m"); + if (ready) { + device_process_new(m, dev, sysfs); + r = swap_process_device_new(m, dev); + if (r < 0) + log_device_warning_errno(dev, r, "Failed to process swap device new event, ignoring: %m"); + } + + /* Add additional units for all symlinks */ + (void) device_setup_devlink_units(m, dev, &ready_devlinks); + } + + if (!IN_SET(action, SD_DEVICE_ADD, SD_DEVICE_REMOVE, SD_DEVICE_MOVE)) { + device_propagate_reload_by_sysfs(m, sysfs); + + STRV_FOREACH(devlink, ready_devlinks) + device_propagate_reload_by_name(m, *devlink); + } + + if (ready || !strv_isempty(ready_devlinks)) manager_dispatch_load_queue(m); + if (action == SD_DEVICE_REMOVE) + /* If we get notified that a device was removed by udev, then it's completely gone, hence + * unset all found bits */ + device_update_found_by_sysfs(m, sysfs, 0, DEVICE_FOUND_MASK); + else if (ready) /* The device is found now, set the udev found bit */ device_update_found_by_sysfs(m, sysfs, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV); - } else + else /* The device is nominally around, but not ready for us. Hence unset the udev bit, but leave * the rest around. */ device_update_found_by_sysfs(m, sysfs, DEVICE_NOT_FOUND, DEVICE_FOUND_UDEV); + STRV_FOREACH(devlink, ready_devlinks) + device_update_found_by_name(m, *devlink, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV); + return 0; } |