Merge pull request #19801 from poettering/device-unit-name-length

pid1: reduce amount of warnings about sysfs device paths we cannot convert into device unit names
This commit is contained in:
Lennart Poettering 2021-06-03 18:08:07 +02:00 committed by GitHub
commit 92ca7e052d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 83 additions and 68 deletions

View file

@ -3,6 +3,8 @@
#include <errno.h>
#include <sys/epoll.h>
#include "sd-messages.h"
#include "alloc-util.h"
#include "bus-error.h"
#include "dbus-device.h"
@ -12,6 +14,7 @@
#include "log.h"
#include "parse-util.h"
#include "path-util.h"
#include "ratelimit.h"
#include "serialize.h"
#include "stat-util.h"
#include "string-util.h"
@ -122,8 +125,8 @@ static int device_load(Unit *u) {
return r;
if (!u->description) {
/* Generate a description based on the path, to be used until the
device is initialized properly */
/* Generate a description based on the path, to be used until the device is initialized
properly */
r = unit_name_to_path(u->id, &u->description);
if (r < 0)
log_unit_debug_errno(u, r, "Failed to unescape name: %m");
@ -490,15 +493,37 @@ static int device_setup_unit(Manager *m, sd_device *dev, const char *path, bool
if (dev) {
r = sd_device_get_syspath(dev, &sysfs);
if (r < 0) {
log_device_debug_errno(dev, r, "Couldn't get syspath from device, ignoring: %m");
return 0;
}
if (r < 0)
return log_device_debug_errno(dev, r, "Couldn't get syspath from device, ignoring: %m");
}
r = unit_name_from_path(path, ".device", &e);
if (r < 0)
return log_device_error_errno(dev, r, "Failed to generate unit name from device path: %m");
if (r < 0) {
/* Let's complain about overly long device names only at most once every 5s or so. This is
* something we should mention, since relevant devices are not manageable by systemd, but not
* flood the log about. */
static RateLimit rate_limit = {
.interval = 5 * USEC_PER_SEC,
.burst = 1,
};
/* If we cannot convert a device name to a unit name then let's ignore the device. So far,
* devices with such long names weren't really the kind you want to manage with systemd
* anyway, hence this shouldn't be a problem. */
if (r == -ENAMETOOLONG)
return log_struct_errno(
ratelimit_below(&rate_limit) ? LOG_WARNING : LOG_DEBUG, r,
"MESSAGE_ID=" SD_MESSAGE_DEVICE_PATH_NOT_SUITABLE_STR,
"DEVICE=%s", path,
LOG_MESSAGE("Device path '%s' too long to fit into unit name, ignoring device.", path));
return log_struct_errno(
ratelimit_below(&rate_limit) ? LOG_WARNING : LOG_DEBUG, r,
"MESSAGE_ID=" SD_MESSAGE_DEVICE_PATH_NOT_SUITABLE_STR,
"DEVICE=%s", path,
LOG_MESSAGE("Failed to generate valid unit name from device path '%s', ignoring device: %m", path));
}
u = manager_get_unit(m, e);
if (u) {
@ -551,9 +576,10 @@ static int device_setup_unit(Manager *m, sd_device *dev, const char *path, bool
(void) device_update_description(u, dev, path);
/* So the user wants the mount units to be bound to the device but a mount unit might has been seen by systemd
* before the device appears on its radar. In this case the device unit is partially initialized and includes
* the deps on the mount unit but at that time the "bind mounts" flag wasn't not present. Fix this up now. */
/* So the user wants the mount units to be bound to the device but a mount unit might has been seen
* by systemd before the device appears on its radar. In this case the device unit is partially
* initialized and includes the deps on the mount unit but at that time the "bind mounts" flag wasn't
* present. Fix this up now. */
if (dev && device_is_bound_by_mounts(DEVICE(u), dev))
device_upgrade_mount_deps(u);
@ -566,7 +592,7 @@ fail:
return r;
}
static int device_process_new(Manager *m, sd_device *dev) {
static void device_process_new(Manager *m, sd_device *dev) {
const char *sysfs, *dn, *alias;
dev_t devnum;
int r;
@ -574,12 +600,13 @@ static int device_process_new(Manager *m, sd_device *dev) {
assert(m);
if (sd_device_get_syspath(dev, &sysfs) < 0)
return 0;
return;
/* Add the main unit named after the sysfs path */
r = device_setup_unit(m, dev, sysfs, true);
if (r < 0)
return r;
/* Add the main unit named after the sysfs path. If this one fails, don't bother with the rest, as
* this one shall be the main device unit the others just follow. (Compare with how
* device_following() is implemented, see below, which looks for the sysfs device.) */
if (device_setup_unit(m, dev, sysfs, true) < 0)
return;
/* Add an additional unit for the device node */
if (sd_device_get_devname(dev, &dn) >= 0)
@ -595,13 +622,11 @@ static int device_process_new(Manager *m, sd_device *dev) {
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 */
/* 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))
@ -613,7 +638,7 @@ static int device_process_new(Manager *m, sd_device *dev) {
/* Add additional units for all explicitly configured aliases */
if (sd_device_get_property_value(dev, "SYSTEMD_ALIAS", &alias) < 0)
return 0;
return;
for (;;) {
_cleanup_free_ char *word = NULL;
@ -622,9 +647,9 @@ static int device_process_new(Manager *m, sd_device *dev) {
if (r == 0)
break;
if (r == -ENOMEM)
return log_oom();
return (void) log_oom();
if (r < 0)
return log_device_warning_errno(dev, r, "Failed to parse SYSTEMD_ALIAS property: %m");
return (void) log_device_warning_errno(dev, r, "Failed to parse SYSTEMD_ALIAS property, ignoring: %m");
if (!path_is_absolute(word))
log_device_warning(dev, "SYSTEMD_ALIAS is not an absolute path, ignoring: %s", word);
@ -633,8 +658,6 @@ static int device_process_new(Manager *m, sd_device *dev) {
else
(void) device_setup_unit(m, dev, word, false);
}
return 0;
}
static void device_found_changed(Device *d, DeviceFound previous, DeviceFound now) {
@ -698,7 +721,7 @@ static void device_update_found_by_sysfs(Manager *m, const char *sysfs, DeviceFo
device_update_found_one(d, found, mask);
}
static int device_update_found_by_name(Manager *m, const char *path, DeviceFound found, DeviceFound mask) {
static void device_update_found_by_name(Manager *m, const char *path, DeviceFound found, DeviceFound mask) {
_cleanup_free_ char *e = NULL;
Unit *u;
int r;
@ -707,18 +730,17 @@ static int device_update_found_by_name(Manager *m, const char *path, DeviceFound
assert(path);
if (mask == 0)
return 0;
return;
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");
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 0;
return;
device_update_found_one(DEVICE(u), found, mask);
return 0;
}
static bool device_is_ready(sd_device *dev) {
@ -859,7 +881,7 @@ static void device_enumerate(Manager *m) {
if (!device_is_ready(dev))
continue;
(void) device_process_new(m, dev);
device_process_new(m, dev);
if (sd_device_get_syspath(dev, &sysfs) < 0)
continue;
@ -891,27 +913,24 @@ static void device_propagate_reload_by_sysfs(Manager *m, const char *sysfs) {
}
}
static int device_remove_old(Manager *m, sd_device *dev) {
static void device_remove_old_on_move(Manager *m, sd_device *dev) {
_cleanup_free_ char *syspath_old = NULL, *e = NULL;
const char *devpath_old;
int r;
r = sd_device_get_property_value(dev, "DEVPATH_OLD", &devpath_old);
if (r < 0) {
log_device_debug_errno(dev, r, "Failed to get DEVPATH_OLD= property on 'move' uevent, ignoring: %m");
return 0;
}
if (r < 0)
return (void) log_device_debug_errno(dev, r, "Failed to get DEVPATH_OLD= property on 'move' uevent, ignoring: %m");
syspath_old = path_join("/sys", devpath_old);
if (!syspath_old)
return log_oom();
return (void) log_oom();
r = unit_name_from_path(syspath_old, ".device", &e);
if (r < 0)
return log_device_error_errno(dev, r, "Failed to generate unit name from old device path: %m");
return (void) log_device_debug_errno(dev, r, "Failed to generate unit name from old device path, ignoring: %m");
device_update_found_by_sysfs(m, syspath_old, 0, DEVICE_FOUND_UDEV|DEVICE_FOUND_MOUNT|DEVICE_FOUND_SWAP);
return 0;
}
static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *userdata) {
@ -939,7 +958,7 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *
device_propagate_reload_by_sysfs(m, sysfs);
if (action == SD_DEVICE_MOVE)
(void) device_remove_old(m, dev);
device_remove_old_on_move(m, dev);
/* A change event can signal that a device is becoming ready, in particular if the device is using
* the SYSTEMD_READY logic in udev so we need to reach the else block of the following if, even for
@ -955,7 +974,7 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *
} else if (device_is_ready(dev)) {
(void) device_process_new(m, dev);
device_process_new(m, dev);
r = swap_process_device_new(m, dev);
if (r < 0)
@ -965,7 +984,6 @@ static int device_dispatch_io(sd_device_monitor *monitor, sd_device *dev, void *
/* The device is found now, set the udev found bit */
device_update_found_by_sysfs(m, sysfs, DEVICE_FOUND_UDEV, DEVICE_FOUND_UDEV);
} else
/* The device is nominally around, but not ready for us. Hence unset the udev bit, but leave
* the rest around. */
@ -1028,8 +1046,6 @@ static int validate_node(Manager *m, const char *node, sd_device **ret) {
}
void device_found_node(Manager *m, const char *node, DeviceFound found, DeviceFound mask) {
int r;
assert(m);
assert(node);
@ -1054,8 +1070,7 @@ void device_found_node(Manager *m, const char *node, DeviceFound found, DeviceFo
* under the name referenced in /proc/swaps or /proc/self/mountinfo. But first, let's validate if
* everything is alright with the device node. */
r = validate_node(m, node, &dev);
if (r <= 0)
if (validate_node(m, node, &dev) <= 0)
return; /* Don't create a device unit for this if the device node is borked. */
(void) device_setup_unit(m, dev, node, false);

View file

@ -492,7 +492,7 @@ fail:
return r;
}
static int swap_process_new(Manager *m, const char *device, int prio, bool set_flags) {
static void swap_process_new(Manager *m, const char *device, int prio, bool set_flags) {
_cleanup_(sd_device_unrefp) sd_device *d = NULL;
const char *dn, *devlink;
struct stat st, st_link;
@ -500,25 +500,22 @@ static int swap_process_new(Manager *m, const char *device, int prio, bool set_f
assert(m);
r = swap_setup_unit(m, device, device, prio, set_flags);
if (r < 0)
return r;
if (swap_setup_unit(m, device, device, prio, set_flags) < 0)
return;
/* If this is a block device, then let's add duplicates for
* all other names of this block device */
if (stat(device, &st) < 0 || !S_ISBLK(st.st_mode))
return 0;
return;
r = sd_device_new_from_stat_rdev(&d, &st);
if (r < 0) {
log_full_errno(r == -ENOENT ? LOG_DEBUG : LOG_WARNING, r,
"Failed to allocate device for swap %s: %m", device);
return 0;
}
if (r < 0)
return (void) log_full_errno(r == -ENOENT ? LOG_DEBUG : LOG_WARNING, r,
"Failed to allocate device for swap %s: %m", device);
/* Add the main device node */
if (sd_device_get_devname(d, &dn) >= 0 && !streq(dn, device))
swap_setup_unit(m, dn, device, prio, set_flags);
(void) swap_setup_unit(m, dn, device, prio, set_flags);
/* Add additional units for all symlinks */
FOREACH_DEVICE_DEVLINK(d, devlink) {
@ -535,10 +532,8 @@ static int swap_process_new(Manager *m, const char *device, int prio, bool set_f
st_link.st_rdev != st.st_rdev))
continue;
swap_setup_unit(m, devlink, device, prio, set_flags);
(void) swap_setup_unit(m, devlink, device, prio, set_flags);
}
return 0;
}
static void swap_set_state(Swap *s, SwapState state) {
@ -1431,13 +1426,14 @@ int swap_process_device_new(Manager *m, sd_device *dev) {
assert(m);
assert(dev);
r = sd_device_get_devname(dev, &dn);
if (r < 0)
if (sd_device_get_devname(dev, &dn) < 0)
return 0;
r = unit_name_from_path(dn, ".swap", &e);
if (r < 0)
return r;
if (r < 0) {
log_debug_errno(r, "Cannot convert device name '%s' to unit name, ignoring: %m", dn);
return 0;
}
u = manager_get_unit(m, e);
if (u)

View file

@ -170,6 +170,10 @@ _SD_BEGIN_DECLARATIONS;
SD_ID128_MAKE(1b,3b,b9,40,37,f0,4b,bf,81,02,8e,13,5a,12,d2,93)
#define SD_MESSAGE_MOUNT_POINT_PATH_NOT_SUITABLE_STR \
SD_ID128_MAKE_STR(1b,3b,b9,40,37,f0,4b,bf,81,02,8e,13,5a,12,d2,93)
#define SD_MESSAGE_DEVICE_PATH_NOT_SUITABLE \
SD_ID128_MAKE(01,01,90,13,8f,49,4e,29,a0,ef,66,69,74,95,31,aa)
#define SD_MESSAGE_DEVICE_PATH_NOT_SUITABLE_STR \
SD_ID128_MAKE_STR(01,01,90,13,8f,49,4e,29,a0,ef,66,69,74,95,31,aa)
#define SD_MESSAGE_NOBODY_USER_UNSUITABLE SD_ID128_MAKE(b4,80,32,5f,9c,39,4a,7b,80,2c,23,1e,51,a2,75,2c)
#define SD_MESSAGE_NOBODY_USER_UNSUITABLE_STR \