Merge pull request #16491 from keszybz/udev-logging

Improvements to udev logging and related code
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2020-07-17 07:12:58 +02:00 committed by GitHub
commit dc9e9a18be
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 119 additions and 129 deletions

View File

@ -280,13 +280,17 @@
<varlistentry>
<term><varname>PROGRAM</varname></term>
<listitem>
<para>Execute a program to determine whether there
is a match; the key is true if the program returns
successfully. The device properties are made available to the
executed program in the environment. The program's standard output
is available in the <varname>RESULT</varname> key.</para>
<para>This can only be used for very short-running foreground tasks. For details,
see <varname>RUN</varname>.</para>
<para>Execute a program to determine whether there is a match; the key is true if the program
returns successfully. The device properties are made available to the executed program in the
environment. The program's standard output is available in the <varname>RESULT</varname>
key.</para>
<para>This can only be used for very short-running foreground tasks. For details, see
<varname>RUN</varname>.</para>
<para>Note that multiple <varname>PROGRAM</varname> keys may be specified in one rule, and
<literal>=</literal>, <literal>:=</literal>, and <literal>+=</literal> have the same effect as
<literal>==</literal>.</para>
</listitem>
</varlistentry>
@ -429,9 +433,14 @@
<varlistentry>
<term><varname>RUN{<replaceable>type</replaceable>}</varname></term>
<listitem>
<para>Add a program to the list of programs to be executed after
processing all the rules for a specific event, depending on
<literal>type</literal>:</para>
<para>Specify a program to be executed after processing of all the rules for the event. With
<literal>+=</literal>, this invocation is added to the list, and with <literal>=</literal> or
<literal>:=</literal>, it replaces any previous contents of the list. Please note that both
<literal>program</literal> and <literal>builtin</literal> types described below use a single
list, so clearing the list with <literal>:=</literal> and <literal>=</literal> affects both
types.</para>
<para><replaceable>type</replaceable> may be:</para>
<variablelist>
<varlistentry>
<term><literal>program</literal></term>
@ -452,21 +461,21 @@
</listitem>
</varlistentry>
</variablelist>
<para>The program name and following arguments are separated by spaces.
Single quotes can be used to specify arguments with spaces.</para>
<para>This can only be used for very short-running foreground tasks. Running an
event process for a long period of time may block all further events for
this or a dependent device.</para>
<para>Starting daemons or other long-running processes is not appropriate
for udev; the forked processes, detached or not, will be unconditionally
killed after the event handling has finished.</para>
<para>Note that running programs that access the network or mount/unmount
filesystems is not allowed inside of udev rules, due to the default sandbox
that is enforced on <filename>systemd-udevd.service</filename>.</para>
<para>Please also note that <literal>:=</literal> and <literal>=</literal> are clearing
both, program and builtin commands.</para>
<para>In order to activate long-running processes from udev rules, provide a service unit, and
pull it in from a udev device using the <varname>SYSTEMD_WANTS</varname> device property. See
<para>The program name and following arguments are separated by spaces. Single quotes can be
used to specify arguments with spaces.</para>
<para>This can only be used for very short-running foreground tasks. Running an event process for
a long period of time may block all further events for this or a dependent device.</para>
<para>Note that running programs that access the network or mount/unmount filesystems is not
allowed inside of udev rules, due to the default sandbox that is enforced on
<filename>systemd-udevd.service</filename>.</para>
<para>Starting daemons or other long-running processes is not allowed; the forked processes,
detached or not, will be unconditionally killed after the event handling has finished. In order
to activate long-running processes from udev rules, provide a service unit and pull it in from a
udev device using the <varname>SYSTEMD_WANTS</varname> device property. See
<citerefentry><refentrytitle>systemd.device</refentrytitle><manvolnum>5</manvolnum></citerefentry>
for details.</para>
</listitem>
@ -489,8 +498,9 @@
<varlistentry>
<term><varname>IMPORT{<replaceable>type</replaceable>}</varname></term>
<listitem>
<para>Import a set of variables as device properties,
depending on <literal>type</literal>:</para>
<para>Import a set of variables as device properties, depending on
<replaceable>type</replaceable>:</para>
<variablelist>
<varlistentry>
<term><literal>program</literal></term>
@ -542,8 +552,14 @@
</listitem>
</varlistentry>
</variablelist>
<para>This can only be used for very short-running foreground tasks. For details
see <option>RUN</option>.</para>
<para>This can only be used for very short-running foreground tasks. For details see
<option>RUN</option>.</para>
<para>Note that multiple <varname>IMPORT{}</varname> keys may be specified in one rule, and
<literal>=</literal>, <literal>:=</literal>, and <literal>+=</literal> have the same effect as
<literal>==</literal>. The key is true if the import is successful, unless <literal>!=</literal>
is used as the operator which causes the key to be true if the import failed.</para>
</listitem>
</varlistentry>
@ -633,9 +649,8 @@
<varlistentry>
<term><option>$number</option>, <option>%n</option></term>
<listitem>
<para>The kernel number for this device. For example,
<literal>sda3</literal> has kernel number <literal>3</literal>.
</para>
<para>The kernel number for this device. For example, <literal>sda3</literal> has kernel number
3.</para>
</listitem>
</varlistentry>

View File

@ -4,12 +4,10 @@
#include "string-util.h"
ssize_t string_table_lookup(const char * const *table, size_t len, const char *key) {
size_t i;
if (!key)
return -1;
for (i = 0; i < len; ++i)
for (size_t i = 0; i < len; ++i)
if (streq_ptr(table[i], key))
return (ssize_t) i;

View File

@ -94,8 +94,8 @@ int device_read_uevent_file(sd_device *device);
int device_set_syspath(sd_device *device, const char *_syspath, bool verify);
int device_set_ifindex(sd_device *device, const char *ifindex);
int device_set_devmode(sd_device *device, const char *devmode);
int device_set_devname(sd_device *device, const char *_devname);
int device_set_devtype(sd_device *device, const char *_devtype);
int device_set_devname(sd_device *device, const char *devname);
int device_set_devtype(sd_device *device, const char *devtype);
int device_set_devnum(sd_device *device, const char *major, const char *minor);
int device_set_subsystem(sd_device *device, const char *_subsystem);
int device_set_driver(sd_device *device, const char *_driver);

View File

@ -558,10 +558,9 @@ int device_monitor_send_device(
r = device_get_properties_nulstr(device, (const uint8_t **) &buf, &blen);
if (r < 0)
return log_device_debug_errno(device, r, "sd-device-monitor: Failed to get device properties: %m");
if (blen < 32) {
log_device_debug(device, "sd-device-monitor: Length of device property nulstr is too small to contain valid device information");
return -EINVAL;
}
if (blen < 32)
log_device_debug_errno(device, SYNTHETIC_ERRNO(EINVAL),
"sd-device-monitor: Length of device property nulstr is too small to contain valid device information");
/* fill in versioned header */
r = sd_device_get_subsystem(device, &val);

View File

@ -363,10 +363,9 @@ static int device_append(sd_device *device, char *key, const char **_major, cons
assert(_minor);
value = strchr(key, '=');
if (!value) {
log_device_debug(device, "sd-device: Not a key-value pair: '%s'", key);
return -EINVAL;
}
if (!value)
return log_device_debug_errno(device, SYNTHETIC_ERRNO(EINVAL),
"sd-device: Not a key-value pair: '%s'", key);
*value = '\0';
@ -400,10 +399,9 @@ void device_seal(sd_device *device) {
static int device_verify(sd_device *device) {
assert(device);
if (!device->devpath || !device->subsystem || device->action < 0 || device->seqnum == 0) {
log_device_debug(device, "sd-device: Device created from strv or nulstr lacks devpath, subsystem, action or seqnum.");
return -EINVAL;
}
if (!device->devpath || !device->subsystem || device->action < 0 || device->seqnum == 0)
return log_device_debug_errno(device, SYNTHETIC_ERRNO(EINVAL),
"sd-device: Device created from strv or nulstr lacks devpath, subsystem, action or seqnum.");
device->sealed = true;
@ -464,10 +462,10 @@ int device_new_from_nulstr(sd_device **ret, uint8_t *nulstr, size_t len) {
key = (char*)&nulstr[i];
end = memchr(key, '\0', len - i);
if (!end) {
log_device_debug(device, "sd-device: Failed to parse nulstr");
return -EINVAL;
}
if (!end)
return log_device_debug_errno(device, SYNTHETIC_ERRNO(EINVAL),
"sd-device: Failed to parse nulstr");
i += end - key + 1;
r = device_append(device, key, &major, &minor);

View File

@ -37,7 +37,7 @@
sd_device *_d = (device); \
int _level = (level), _error = (error); \
\
if (_d && _unlikely_(log_get_max_level() >= _level)) \
if (_d && _unlikely_(log_get_max_level() >= LOG_PRI(_level))) \
(void) sd_device_get_sysname(_d, &_sysname); \
log_object_internal(_level, _error, PROJECT_FILE, __LINE__, __func__, \
_sysname ? "DEVICE=" : NULL, _sysname, \

View File

@ -320,24 +320,22 @@ _public_ int sd_device_new_from_subsystem_sysname(sd_device **ret, const char *s
return -ENODEV;
}
int device_set_devtype(sd_device *device, const char *_devtype) {
_cleanup_free_ char *devtype = NULL;
int device_set_devtype(sd_device *device, const char *devtype) {
_cleanup_free_ char *t = NULL;
int r;
assert(device);
assert(_devtype);
assert(devtype);
devtype = strdup(_devtype);
if (!devtype)
t = strdup(devtype);
if (!t)
return -ENOMEM;
r = device_add_property_internal(device, "DEVTYPE", devtype);
r = device_add_property_internal(device, "DEVTYPE", t);
if (r < 0)
return r;
free_and_replace(device->devtype, devtype);
return 0;
return free_and_replace(device->devtype, t);
}
int device_set_ifindex(sd_device *device, const char *name) {
@ -359,30 +357,25 @@ int device_set_ifindex(sd_device *device, const char *name) {
return 0;
}
int device_set_devname(sd_device *device, const char *_devname) {
_cleanup_free_ char *devname = NULL;
int device_set_devname(sd_device *device, const char *devname) {
_cleanup_free_ char *t = NULL;
int r;
assert(device);
assert(_devname);
assert(devname);
if (_devname[0] != '/') {
r = asprintf(&devname, "/dev/%s", _devname);
if (r < 0)
return -ENOMEM;
} else {
devname = strdup(_devname);
if (!devname)
return -ENOMEM;
}
if (devname[0] != '/')
t = strjoin("/dev/", devname);
else
t = strdup(devname);
if (!t)
return -ENOMEM;
r = device_add_property_internal(device, "DEVNAME", devname);
r = device_add_property_internal(device, "DEVNAME", t);
if (r < 0)
return r;
free_and_replace(device->devname, devname);
return 0;
return free_and_replace(device->devname, t);
}
int device_set_devmode(sd_device *device, const char *_devmode) {
@ -1250,17 +1243,15 @@ int device_get_id_filename(sd_device *device, const char **ret) {
if (!subsystem)
return -EINVAL;
if (streq(subsystem, "drivers")) {
if (streq(subsystem, "drivers"))
/* the 'drivers' pseudo-subsystem is special, and needs the real subsystem
* encoded as well */
r = asprintf(&id, "+drivers:%s:%s", device->driver_subsystem, sysname);
if (r < 0)
return -ENOMEM;
} else {
r = asprintf(&id, "+%s:%s", subsystem, sysname);
if (r < 0)
return -ENOMEM;
}
id = strjoin("+drivers:", device->driver_subsystem, ":", sysname);
else
id = strjoin("+", subsystem, ":", sysname);
if (!id)
return -ENOMEM;
}
device->id_filename = TAKE_PTR(id);

View File

@ -936,10 +936,9 @@ static int umount_by_device(sd_bus *bus, const char *what) {
if (r < 0)
return log_device_error_errno(d, r, "Failed to get device property: %m");
if (!streq(v, "filesystem")) {
log_device_error(d, "%s does not contain a known file system.", what);
return -EINVAL;
}
if (!streq(v, "filesystem"))
return log_device_error_errno(d, SYNTHETIC_ERRNO(EINVAL),
"%s does not contain a known file system.", what);
if (sd_device_get_property_value(d, "SYSTEMD_MOUNT_WHERE", &v) >= 0)
r2 = stop_mounts(bus, v);
@ -1275,10 +1274,9 @@ static int discover_loop_backing_file(void) {
if (r < 0)
return log_error_errno(r, "Failed to get device from device number: %m");
if (sd_device_get_property_value(d, "ID_FS_USAGE", &v) < 0 || !streq(v, "filesystem")) {
log_device_error(d, "%s does not contain a known file system.", arg_mount_what);
return -EINVAL;
}
if (sd_device_get_property_value(d, "ID_FS_USAGE", &v) < 0 || !streq(v, "filesystem"))
return log_device_error_errno(d, SYNTHETIC_ERRNO(EINVAL),
"%s does not contain a known file system.", arg_mount_what);
r = acquire_mount_type(d);
if (r < 0)

View File

@ -47,10 +47,10 @@ static int node_symlink(sd_device *dev, const char *node, const char *slink) {
/* preserve link with correct target, do not replace node of other device */
if (lstat(slink, &stats) == 0) {
if (S_ISBLK(stats.st_mode) || S_ISCHR(stats.st_mode)) {
log_device_error(dev, "Conflicting device node '%s' found, link to '%s' will not be created.", slink, node);
return -EOPNOTSUPP;
} else if (S_ISLNK(stats.st_mode)) {
if (S_ISBLK(stats.st_mode) || S_ISCHR(stats.st_mode))
return log_device_error_errno(dev, SYNTHETIC_ERRNO(EOPNOTSUPP),
"Conflicting device node '%s' found, link to '%s' will not be created.", slink, node);
else if (S_ISLNK(stats.st_mode)) {
_cleanup_free_ char *buf = NULL;
if (readlink_malloc(slink, &buf) >= 0 &&

View File

@ -595,7 +595,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
if (!is_match) {
if (streq(value, "%k"))
return log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL),
"Ignoring NAME=\"%%k\" is ignored, as it breaks kernel supplied names.");
"NAME=\"%%k\" is ignored, as it breaks kernel supplied names.");
if (isempty(value))
return log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL),
"Ignoring NAME=\"\", as udev will not delete any device nodes.");
@ -755,10 +755,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
check_value_format_and_warn(rules, key, value, true);
if (op == OP_REMOVE)
return log_token_invalid_op(rules, key);
if (!is_match) {
log_token_debug(rules, "%s key takes '==' or '!=' operator, assuming '=='.", key);
if (!is_match)
op = OP_MATCH;
}
r = rule_line_add_token(rule_line, TK_M_PROGRAM, op, value, NULL);
} else if (streq(key, "IMPORT")) {
@ -767,10 +765,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
check_value_format_and_warn(rules, key, value, true);
if (op == OP_REMOVE)
return log_token_invalid_op(rules, key);
if (!is_match) {
log_token_debug(rules, "%s key takes '==' or '!=' operator, assuming '=='.", key);
if (!is_match)
op = OP_MATCH;
}
if (streq(attr, "file"))
r = rule_line_add_token(rule_line, TK_M_IMPORT_FILE, op, value, NULL);
@ -813,10 +809,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
return log_token_invalid_attr(rules, key);
if (is_match || op == OP_REMOVE)
return log_token_invalid_op(rules, key);
if (op == OP_ADD) {
log_token_debug(rules, "Operator '+=' is specified to %s key, assuming '='.", key);
if (op == OP_ADD)
op = OP_ASSIGN;
}
if (streq(value, "string_escape=none"))
r = rule_line_add_token(rule_line, TK_A_OPTIONS_STRING_ESCAPE_NONE, op, NULL, NULL);
@ -866,7 +860,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
check_value_format_and_warn(rules, key, value, true);
r = rule_line_add_token(rule_line, TK_A_OWNER, op, value, NULL);
} else {
log_token_debug(rules, "Resolving user name is disabled, ignoring %s=%s", key, value);
log_token_debug(rules, "User name resolution is disabled, ignoring %s=%s", key, value);
return 0;
}
} else if (streq(key, "GROUP")) {
@ -949,7 +943,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
if (op != OP_ASSIGN)
return log_token_invalid_op(rules, key);
if (FLAGS_SET(rule_line->type, LINE_HAS_GOTO)) {
log_token_warning(rules, "Contains multiple GOTO key, ignoring GOTO=\"%s\".", value);
log_token_warning(rules, "Contains multiple GOTO keys, ignoring GOTO=\"%s\".", value);
return 0;
}
@ -1661,7 +1655,7 @@ static int udev_rule_apply_token_to_event(
if (r == -ENOENT)
return token->op == OP_NOMATCH;
if (r < 0)
return log_rule_error_errno(dev, rules, r, "Failed to test the existence of '%s': %m", buf);
return log_rule_error_errno(dev, rules, r, "Failed to test for the existence of '%s': %m", buf);
if (stat(buf, &statbuf) < 0)
return token->op == OP_NOMATCH;
@ -1682,16 +1676,16 @@ static int udev_rule_apply_token_to_event(
r = udev_event_spawn(event, timeout_usec, timeout_signal, true, buf, result, sizeof(result));
if (r != 0) {
if (r < 0)
log_rule_warning_errno(dev, rules, r, "Failed to execute '%s', ignoring: %m", buf);
log_rule_warning_errno(dev, rules, r, "Failed to execute \"%s\": %m", buf);
else /* returned value is positive when program fails */
log_rule_debug(dev, rules, "Command \"%s\" returned %d (error), ignoring", buf, r);
log_rule_debug(dev, rules, "Command \"%s\" returned %d (error)", buf, r);
return token->op == OP_NOMATCH;
}
delete_trailing_chars(result, "\n");
count = util_replace_chars(result, UDEV_ALLOWED_CHARS_INPUT);
if (count > 0)
log_rule_debug(dev, rules, "Replaced %zu character(s) from result of '%s'",
log_rule_debug(dev, rules, "Replaced %zu character(s) in result of \"%s\"",
count, buf);
event->program_result = strdup(result);

View File

@ -125,8 +125,7 @@ int udev_watch_end(sd_device *dev) {
int wd, r;
if (inotify_fd < 0)
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL),
"Invalid inotify descriptor.");
return 0; /* Nothing to do. */
r = device_get_watch_handle(dev, &wd);
if (r == -ENOENT)

View File

@ -44,17 +44,15 @@ static const char *arg_export_prefix = NULL;
static usec_t arg_wait_for_initialization_timeout = 0;
static bool skip_attribute(const char *name) {
static const char* const skip[] = {
"uevent",
"dev",
"modalias",
"resource",
"driver",
"subsystem",
"module",
};
return string_table_lookup(skip, ELEMENTSOF(skip), name) >= 0;
/* Those are either displayed separately or should not be shown at all. */
return STR_IN_SET(name,
"uevent",
"dev",
"modalias",
"resource",
"driver",
"subsystem",
"module");
}
typedef struct SysAttr {