diff --git a/NEWS b/NEWS index ac29a5e0d4..6fc5815293 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,90 @@ systemd System and Service Manager +CHANGES WITH 247 in spe: + + * KERNEL API INCOMPATIBILTY: Linux 4.12 introduced two new uevents + "bind" and "unbind" to the Linux device model. When this kernel + change was made, systemd-udevd was only minimally updated to handle + and propagate these new event types. The introduction of these new + uevents (which are typically generated for USB devices and devices + needing a firmware upload before being functional) resulted in a + number of software issues, we so far didn't address (mostly because + there was hope the kernel maintainers would themeselves address these + issues in some form – which did not happen). To handle them properly, + many (if not most) udev rules files shipped in various packages need + updating, and so do many programs that monitor or enumerate devices + with libudev or sd-device, or otherwise process uevents. Please note + that this incompatibility is not fault of systemd or udev, but caused + by an incompatible kernel change that happened back in Linux 4.12. + + To minimize issues resulting from this kernel change (but not avoid + them entirely) starting with systemd-udevd 247 the udev "tags" + concept (which is a concept for marking and filtering devices during + enumeration and monitoring) has been reworked: udev tags are now + "sticky", meaning that once a tag is assigned to a device it will not + be removed from the device again until the device itself is removed + (i.e. unplugged). This makes sure that any application monitoring + devices that match a specific tag is guaranteed to both see uevents + where the device starts being relevant, and those where it stops + being relevant (the latter now regularly happening due to the new + "unbind" uevent type). The udev tags concept is hence now a concept + tied to a *device* instead of a device *event* — unlike for example + udev properties whose lifecycle (as before) is generally tied to a + device event, meaning that the previously determined properties are + forgotten whenever a new uevent is processed. + + With the newly redefined udev tags concept, sometimes it's necessary + to determine which tags are the ones applied by the most recent + uevent/database update, in order to discern them from those + originating from earlier uevents/database updates of the same + device. To accommodate for this a new automatic property CURRENT_TAGS + has been added that works similar to the existing TAGS property but + only lists tags set by the most recent uevent/database + update. Similar, the libudev/sd-device API has been updated with new + functions to enumerate these 'current' tags, in addition to the + existing APIs that now enumerate the 'sticky' ones. + + To properly handle "bind"/"unbind" on Linux 4.12 and newer it is + essential that all udev rules files and applications are updated to + handle the new events. Specifically: + + • All rule files that currently use a header guard similar to + ACTION!="add|change",GOTO="xyz_end" should be updated to use + ACTION=="remove",GOTO="xyz_end" instead, so that the + properties/tags they add are also applied whenever "bind" (or + "unbind") is seen. (This is most important for all physical device + types — as that's for which "bind" and "unbind" are currently + usually generated, for all other device types this change is still + recommended but not as important — but certainly prepares for + future kernel uevent type additions). + + • Similar, all code monitoring devices that contains an 'if' branch + discerning the "add" + "change" uevent actions from all other + uevents actions (i.e. considering devices only relevant after "add" + or "change", and irrelevant on all other events) should be reworked + to instead negatively check for "remove" only (i.e. considering + devices relevant after all event types, except for "remove", which + invalidates the device). Note that this also means that devices + should be considered relevant on "unbind", even though conceptually + this — in some form — invalidates the device. Since the precise + effect of "unbind" is not generically defined, devices should be + considered relevant even after "unbind", however I/O errors + accessing the device should then be handled gracefully. + + • Any code that uses device tags for deciding whether a device is + relevant or not most likely needs to be updated to use the new + udev_device_has_current_tag() API (or sd_device_has_current_tag() + in case sd-device is used), to check whether the tag is set + at the moment an uevent is seen (as opposed to the existing + udev_device_has_tag() API which checks if the tag ever existed on + the device, following the API concept redefinition explained + above). + + We are very sorry for this breakage and the requirement to update + packages using these interfaces. We'd again like to underline that + this is not caused by systemd/udev changes, but result of a kernel + behaviour change. + CHANGES WITH 246: * The service manager gained basic support for cgroup v2 freezer. Units diff --git a/man/udev_device_has_tag.xml b/man/udev_device_has_tag.xml index 9c64a4b45b..2e5b67e750 100644 --- a/man/udev_device_has_tag.xml +++ b/man/udev_device_has_tag.xml @@ -21,9 +21,11 @@ udev_device_has_tag + udev_device_has_current_tag udev_device_get_devlinks_list_entry udev_device_get_properties_list_entry udev_device_get_tags_list_entry + udev_device_get_current_tags_list_entry udev_device_get_sysattr_list_entry udev_device_get_property_value udev_device_get_sysattr_value @@ -36,6 +38,18 @@ #include <libudev.h> + + int udev_device_has_tag + struct udev_device *udev_device + const char *tag + + + + int udev_device_has_current_tag + struct udev_device *udev_device + const char *tag + + struct udev_list_entry *udev_device_get_devlinks_list_entry struct udev_device *udev_device @@ -51,6 +65,11 @@ struct udev_device *udev_device + + struct udev_list_entry *udev_device_get_current_tags_list_entry + struct udev_device *udev_device + + struct udev_list_entry *udev_device_get_sysattr_list_entry struct udev_device *udev_device @@ -62,12 +81,6 @@ const char *key - - int udev_device_has_tag - struct udev_device *udev_device - const char *tag - - const char *udev_device_get_sysattr_value struct udev_device *udev_device @@ -84,22 +97,40 @@ - + udev_device_has_tag() returns a valuer larger than zero if the specified + device object has the indicated tag assigned to it, and zero otherwise. See + udev7 for details on + the tags concept. udev_device_has_current_tag() executes a similar check, however + only determines whether the indicated tag was set as result of the most recent event seen for the + device. Tags are "sticky", i.e. once set for a device they remain on the device until the device is + unplugged, even if the rules run for later events of the same device do not set them anymore. Any tag for + which udev_device_has_current_tag() returns true will hence also return true when + passed to udev_device_has_tag(), but the opposite might not be true, in case a tag is + no longer configured by the rules applied to the most recent device even. + + udev_device_get_tags_list_entry() returns a a + udev_list_entry object, encapsulating a list of tags set for the specified + device. Similar, udev_device_get_current_tags_list_entry() returns a list of tags + set for the specified device as effect of the most recent device event seen (see above for details on the + difference). + Return Value - On success, - udev_device_get_devlinks_list_entry(), + On success, udev_device_has_tag() and + udev_device_has_current_tag() return positive or 0, depending + on whether the device has the given tag or not. On failure, a negative error code is returned. + + On success, udev_device_get_devlinks_list_entry(), udev_device_get_properties_list_entry(), - udev_device_get_tags_list_entry() and - udev_device_get_sysattr_list_entry() return - a pointer to the first entry of the retrieved list. If that list - is empty, or if an error occurred, NULL is + udev_device_get_tags_list_entry(), + udev_device_get_current_tags_list_entry() and + udev_device_get_sysattr_list_entry() return a pointer to the first entry of the + retrieved list. If that list is empty, or if an error occurred, NULL is returned. On success, @@ -119,17 +150,13 @@ contain NUL bytes should not be set with this function; instead, write them directly to the files within the device's syspath. - - On success, udev_device_has_tag() - returns 1 or 0, - depending on whether the device has the given tag or not. - On failure, a negative error code is returned. See Also + udev7, udev_new3, udev_device_new_from_syspath3, udev_device_get_syspath3, diff --git a/meson.build b/meson.build index 6837540313..217b17423b 100644 --- a/meson.build +++ b/meson.build @@ -14,7 +14,7 @@ project('systemd', 'c', ) libsystemd_version = '0.29.0' -libudev_version = '1.6.18' +libudev_version = '1.7.0' # We need the same data in two different formats, ugh! # Also, for hysterical reasons, we use different variable diff --git a/src/core/device.c b/src/core/device.c index 5b8134159a..53bc549bd3 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -736,6 +736,10 @@ static bool device_is_ready(sd_device *dev) { if (device_is_renaming(dev) > 0) return false; + /* Is it really tagged as 'systemd' right now? */ + if (sd_device_has_current_tag(dev, "systemd") <= 0) + return false; + if (sd_device_get_property_value(dev, "SYSTEMD_READY", &ready) < 0) return true; diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index a3659d00b3..1bee5318d6 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -728,4 +728,8 @@ global: sd_event_source_set_time_relative; sd_bus_error_has_names_sentinel; + + sd_device_get_current_tag_first; + sd_device_get_current_tag_next; + sd_device_has_current_tag; } LIBSYSTEMD_246; diff --git a/src/libsystemd/sd-device/device-internal.h b/src/libsystemd/sd-device/device-internal.h index 1fe5c1a6bf..9c6b8a345f 100644 --- a/src/libsystemd/sd-device/device-internal.h +++ b/src/libsystemd/sd-device/device-internal.h @@ -28,10 +28,10 @@ struct sd_device { Set *sysattrs; /* names of sysattrs */ Iterator sysattrs_iterator; - Set *tags; - Iterator tags_iterator; + Set *all_tags, *current_tags; + Iterator all_tags_iterator, current_tags_iterator; + uint64_t all_tags_iterator_generation, current_tags_iterator_generation; /* generation when iteration was started */ uint64_t tags_generation; /* changes whenever the tags are changed */ - uint64_t tags_iterator_generation; /* generation when iteration was started */ Set *devlinks; Iterator devlinks_iterator; @@ -71,7 +71,7 @@ struct sd_device { bool parent_set:1; /* no need to try to reload parent */ bool sysattrs_read:1; /* don't try to re-read sysattrs once read */ - bool property_tags_outdated:1; /* need to update TAGS= property */ + bool property_tags_outdated:1; /* need to update TAGS= or CURRENT_TAGS= property */ bool property_devlinks_outdated:1; /* need to update DEVLINKS= property */ bool properties_buf_outdated:1; /* need to reread hashmap */ bool sysname_set:1; /* don't reread sysname */ diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c index 1e61732dfe..1ad7713ec7 100644 --- a/src/libsystemd/sd-device/device-private.c +++ b/src/libsystemd/sd-device/device-private.c @@ -329,7 +329,7 @@ static int device_amend(sd_device *device, const char *key, const char *value) { if (r < 0) return log_device_debug_errno(device, r, "sd-device: Failed to add devlink '%s': %m", devlink); } - } else if (streq(key, "TAGS")) { + } else if (STR_IN_SET(key, "TAGS", "CURRENT_TAGS")) { const char *word, *state; size_t l; @@ -339,10 +339,11 @@ static int device_amend(sd_device *device, const char *key, const char *value) { (void) strncpy(tag, word, l); tag[l] = '\0'; - r = device_add_tag(device, tag); + r = device_add_tag(device, tag, streq(key, "CURRENT_TAGS")); if (r < 0) return log_device_debug_errno(device, r, "sd-device: Failed to add tag '%s': %m", tag); } + } else { r = device_add_property_internal(device, key, value); if (r < 0) @@ -759,8 +760,8 @@ int device_copy_properties(sd_device *device_dst, sd_device *device_src) { void device_cleanup_tags(sd_device *device) { assert(device); - set_free_free(device->tags); - device->tags = NULL; + device->all_tags = set_free_free(device->all_tags); + device->current_tags = set_free_free(device->current_tags); device->property_tags_outdated = true; device->tags_generation++; } @@ -778,7 +779,7 @@ void device_remove_tag(sd_device *device, const char *tag) { assert(device); assert(tag); - free(set_remove(device->tags, tag)); + free(set_remove(device->current_tags, tag)); device->property_tags_outdated = true; device->tags_generation++; } @@ -846,7 +847,10 @@ static bool device_has_info(sd_device *device) { if (!ordered_hashmap_isempty(device->properties_db)) return true; - if (!set_isempty(device->tags)) + if (!set_isempty(device->all_tags)) + return true; + + if (!set_isempty(device->current_tags)) return true; if (device->watch_handle >= 0) @@ -939,7 +943,10 @@ int device_update_db(sd_device *device) { fprintf(f, "E:%s=%s\n", property, value); FOREACH_DEVICE_TAG(device, tag) - fprintf(f, "G:%s\n", tag); + fprintf(f, "G:%s\n", tag); /* Any tag */ + + SET_FOREACH(tag, device->current_tags, i) + fprintf(f, "Q:%s\n", tag); /* Current tag */ } r = fflush_and_check(f); diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index 2bde53e969..1f1c4ca107 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -45,7 +45,7 @@ void device_set_devlink_priority(sd_device *device, int priority); int device_ensure_usec_initialized(sd_device *device, sd_device *device_old); int device_add_devlink(sd_device *device, const char *devlink); int device_add_property(sd_device *device, const char *property, const char *value); -int device_add_tag(sd_device *device, const char *tag); +int device_add_tag(sd_device *device, const char *tag, bool both); void device_remove_tag(sd_device *device, const char *tag); void device_cleanup_tags(sd_device *device); void device_cleanup_devlinks(sd_device *device); diff --git a/src/libsystemd/sd-device/device-util.h b/src/libsystemd/sd-device/device-util.h index 1a1795d974..eda0f2f49e 100644 --- a/src/libsystemd/sd-device/device-util.h +++ b/src/libsystemd/sd-device/device-util.h @@ -11,6 +11,11 @@ tag; \ tag = sd_device_get_tag_next(device)) +#define FOREACH_DEVICE_CURRENT_TAG(device, tag) \ + for (tag = sd_device_get_current_tag_first(device); \ + tag; \ + tag = sd_device_get_current_tag_next(device)) + #define FOREACH_DEVICE_SYSATTR(device, attr) \ for (attr = sd_device_get_sysattr_first(device); \ attr; \ diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 3bba17aff8..3041ce2e9c 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -69,7 +69,8 @@ static sd_device *device_free(sd_device *device) { ordered_hashmap_free_free_free(device->properties_db); hashmap_free_free_free(device->sysattr_values); set_free(device->sysattrs); - set_free(device->tags); + set_free(device->all_tags); + set_free(device->current_tags); set_free(device->devlinks); return mfree(device); @@ -1062,8 +1063,8 @@ static bool is_valid_tag(const char *tag) { return !strchr(tag, ':') && !strchr(tag, ' '); } -int device_add_tag(sd_device *device, const char *tag) { - int r; +int device_add_tag(sd_device *device, const char *tag, bool both) { + int r, added; assert(device); assert(tag); @@ -1071,9 +1072,21 @@ int device_add_tag(sd_device *device, const char *tag) { if (!is_valid_tag(tag)) return -EINVAL; - r = set_put_strdup(&device->tags, tag); - if (r < 0) - return r; + /* Definitely add to the "all" list of tags (i.e. the sticky list) */ + added = set_put_strdup(&device->all_tags, tag); + if (added < 0) + return added; + + /* And optionally, also add it to the current list of tags */ + if (both) { + r = set_put_strdup(&device->current_tags, tag); + if (r < 0) { + if (added > 0) + (void) set_remove(device->all_tags, tag); + + return r; + } + } device->tags_generation++; device->property_tags_outdated = true; @@ -1151,8 +1164,9 @@ static int handle_db_line(sd_device *device, char key, const char *value) { assert(value); switch (key) { - case 'G': - r = device_add_tag(device, value); + case 'G': /* Any tag */ + case 'Q': /* Current tag */ + r = device_add_tag(device, value, key == 'Q'); if (r < 0) return r; @@ -1407,10 +1421,10 @@ _public_ const char *sd_device_get_tag_first(sd_device *device) { (void) device_read_db(device); - device->tags_iterator_generation = device->tags_generation; - device->tags_iterator = ITERATOR_FIRST; + device->all_tags_iterator_generation = device->tags_generation; + device->all_tags_iterator = ITERATOR_FIRST; - (void) set_iterate(device->tags, &device->tags_iterator, &v); + (void) set_iterate(device->all_tags, &device->all_tags_iterator, &v); return v; } @@ -1421,10 +1435,38 @@ _public_ const char *sd_device_get_tag_next(sd_device *device) { (void) device_read_db(device); - if (device->tags_iterator_generation != device->tags_generation) + if (device->all_tags_iterator_generation != device->tags_generation) return NULL; - (void) set_iterate(device->tags, &device->tags_iterator, &v); + (void) set_iterate(device->all_tags, &device->all_tags_iterator, &v); + return v; +} + +_public_ const char *sd_device_get_current_tag_first(sd_device *device) { + void *v; + + assert_return(device, NULL); + + (void) device_read_db(device); + + device->current_tags_iterator_generation = device->tags_generation; + device->current_tags_iterator = ITERATOR_FIRST; + + (void) set_iterate(device->current_tags, &device->current_tags_iterator, &v); + return v; +} + +_public_ const char *sd_device_get_current_tag_next(sd_device *device) { + void *v; + + assert_return(device, NULL); + + (void) device_read_db(device); + + if (device->current_tags_iterator_generation != device->tags_generation) + return NULL; + + (void) set_iterate(device->current_tags, &device->current_tags_iterator, &v); return v; } @@ -1456,6 +1498,31 @@ _public_ const char *sd_device_get_devlink_next(sd_device *device) { return v; } +static char *join_string_set(Set *s) { + size_t ret_allocated = 0, ret_len; + _cleanup_free_ char *ret = NULL; + const char *tag; + Iterator i; + + if (!GREEDY_REALLOC(ret, ret_allocated, 2)) + return NULL; + + strcpy(ret, ":"); + ret_len = 1; + + SET_FOREACH(tag, s, i) { + char *e; + + if (!GREEDY_REALLOC(ret, ret_allocated, ret_len + strlen(tag) + 2)) + return NULL; + + e = stpcpy(stpcpy(ret + ret_len, tag), ":"); + ret_len = e - ret; + } + + return TAKE_PTR(ret); +} + int device_properties_prepare(sd_device *device) { int r; @@ -1494,26 +1561,27 @@ int device_properties_prepare(sd_device *device) { if (device->property_tags_outdated) { _cleanup_free_ char *tags = NULL; - size_t tags_allocated = 0, tags_len = 0; - const char *tag; - if (!GREEDY_REALLOC(tags, tags_allocated, 2)) + tags = join_string_set(device->all_tags); + if (!tags) return -ENOMEM; - stpcpy(tags, ":"); - tags_len++; - for (tag = sd_device_get_tag_first(device); tag; tag = sd_device_get_tag_next(device)) { - char *e; - - if (!GREEDY_REALLOC(tags, tags_allocated, tags_len + strlen(tag) + 2)) - return -ENOMEM; - e = stpcpy(stpcpy(tags + tags_len, tag), ":"); - tags_len = e - tags; + if (!streq(tags, ":")) { + r = device_add_property_internal(device, "TAGS", tags); + if (r < 0) + return r; } - r = device_add_property_internal(device, "TAGS", tags); - if (r < 0) - return r; + free(tags); + tags = join_string_set(device->current_tags); + if (!tags) + return -ENOMEM; + + if (!streq(tags, ":")) { + r = device_add_property_internal(device, "CURRENT_TAGS", tags); + if (r < 0) + return r; + } device->property_tags_outdated = false; } @@ -1689,7 +1757,16 @@ _public_ int sd_device_has_tag(sd_device *device, const char *tag) { (void) device_read_db(device); - return !!set_contains(device->tags, tag); + return set_contains(device->all_tags, tag); +} + +_public_ int sd_device_has_current_tag(sd_device *device, const char *tag) { + assert_return(device, -EINVAL); + assert_return(tag, -EINVAL); + + (void) device_read_db(device); + + return set_contains(device->current_tags, tag); } _public_ int sd_device_get_property_value(sd_device *device, const char *key, const char **_value) { diff --git a/src/libudev/libudev-device.c b/src/libudev/libudev-device.c index b993309911..704a09d01c 100644 --- a/src/libudev/libudev-device.c +++ b/src/libudev/libudev-device.c @@ -56,12 +56,13 @@ struct udev_device { struct udev_list *properties; uint64_t properties_generation; - struct udev_list *tags; - uint64_t tags_generation; + struct udev_list *all_tags, *current_tags; + uint64_t all_tags_generation, current_tags_generation; struct udev_list *devlinks; uint64_t devlinks_generation; bool properties_read:1; - bool tags_read:1; + bool all_tags_read:1; + bool current_tags_read:1; bool devlinks_read:1; struct udev_list *sysattrs; bool sysattrs_read; @@ -199,7 +200,7 @@ _public_ const char *udev_device_get_property_value(struct udev_device *udev_dev } struct udev_device *udev_device_new(struct udev *udev, sd_device *device) { - _cleanup_(udev_list_freep) struct udev_list *properties = NULL, *tags = NULL, *sysattrs = NULL, *devlinks = NULL; + _cleanup_(udev_list_freep) struct udev_list *properties = NULL, *all_tags = NULL, *current_tags = NULL, *sysattrs = NULL, *devlinks = NULL; struct udev_device *udev_device; assert(device); @@ -207,8 +208,11 @@ struct udev_device *udev_device_new(struct udev *udev, sd_device *device) { properties = udev_list_new(true); if (!properties) return_with_errno(NULL, ENOMEM); - tags = udev_list_new(true); - if (!tags) + all_tags = udev_list_new(true); + if (!all_tags) + return_with_errno(NULL, ENOMEM); + current_tags = udev_list_new(true); + if (!current_tags) return_with_errno(NULL, ENOMEM); sysattrs = udev_list_new(true); if (!sysattrs) @@ -226,7 +230,8 @@ struct udev_device *udev_device_new(struct udev *udev, sd_device *device) { .udev = udev, .device = sd_device_ref(device), .properties = TAKE_PTR(properties), - .tags = TAKE_PTR(tags), + .all_tags = TAKE_PTR(all_tags), + .current_tags = TAKE_PTR(current_tags), .sysattrs = TAKE_PTR(sysattrs), .devlinks = TAKE_PTR(devlinks), }; @@ -475,7 +480,8 @@ static struct udev_device *udev_device_free(struct udev_device *udev_device) { udev_list_free(udev_device->properties); udev_list_free(udev_device->sysattrs); - udev_list_free(udev_device->tags); + udev_list_free(udev_device->all_tags); + udev_list_free(udev_device->current_tags); udev_list_free(udev_device->devlinks); return mfree(udev_device); @@ -834,21 +840,41 @@ _public_ int udev_device_get_is_initialized(struct udev_device *udev_device) { _public_ struct udev_list_entry *udev_device_get_tags_list_entry(struct udev_device *udev_device) { assert_return_errno(udev_device, NULL, EINVAL); - if (device_get_tags_generation(udev_device->device) != udev_device->tags_generation || - !udev_device->tags_read) { + if (device_get_tags_generation(udev_device->device) != udev_device->all_tags_generation || + !udev_device->all_tags_read) { const char *tag; - udev_list_cleanup(udev_device->tags); + udev_list_cleanup(udev_device->all_tags); FOREACH_DEVICE_TAG(udev_device->device, tag) - if (!udev_list_entry_add(udev_device->tags, tag, NULL)) + if (!udev_list_entry_add(udev_device->all_tags, tag, NULL)) return_with_errno(NULL, ENOMEM); - udev_device->tags_read = true; - udev_device->tags_generation = device_get_tags_generation(udev_device->device); + udev_device->all_tags_read = true; + udev_device->all_tags_generation = device_get_tags_generation(udev_device->device); } - return udev_list_get_entry(udev_device->tags); + return udev_list_get_entry(udev_device->all_tags); +} + +_public_ struct udev_list_entry *udev_device_get_current_tags_list_entry(struct udev_device *udev_device) { + assert_return_errno(udev_device, NULL, EINVAL); + + if (device_get_tags_generation(udev_device->device) != udev_device->current_tags_generation || + !udev_device->current_tags_read) { + const char *tag; + + udev_list_cleanup(udev_device->current_tags); + + FOREACH_DEVICE_CURRENT_TAG(udev_device->device, tag) + if (!udev_list_entry_add(udev_device->current_tags, tag, NULL)) + return_with_errno(NULL, ENOMEM); + + udev_device->current_tags_read = true; + udev_device->current_tags_generation = device_get_tags_generation(udev_device->device); + } + + return udev_list_get_entry(udev_device->current_tags); } /** @@ -866,6 +892,12 @@ _public_ int udev_device_has_tag(struct udev_device *udev_device, const char *ta return sd_device_has_tag(udev_device->device, tag) > 0; } +_public_ int udev_device_has_current_tag(struct udev_device *udev_device, const char *tag) { + assert_return(udev_device, 0); + + return sd_device_has_current_tag(udev_device->device, tag) > 0; +} + sd_device *udev_device_get_sd_device(struct udev_device *udev_device) { assert(udev_device); diff --git a/src/libudev/libudev.h b/src/libudev/libudev.h index 02c2e5e8ed..c9d0bf233e 100644 --- a/src/libudev/libudev.h +++ b/src/libudev/libudev.h @@ -82,6 +82,7 @@ int udev_device_get_is_initialized(struct udev_device *udev_device); struct udev_list_entry *udev_device_get_devlinks_list_entry(struct udev_device *udev_device); struct udev_list_entry *udev_device_get_properties_list_entry(struct udev_device *udev_device); struct udev_list_entry *udev_device_get_tags_list_entry(struct udev_device *udev_device); +struct udev_list_entry *udev_device_get_current_tags_list_entry(struct udev_device *udev_device); struct udev_list_entry *udev_device_get_sysattr_list_entry(struct udev_device *udev_device); const char *udev_device_get_property_value(struct udev_device *udev_device, const char *key); const char *udev_device_get_driver(struct udev_device *udev_device); @@ -92,6 +93,7 @@ unsigned long long int udev_device_get_usec_since_initialized(struct udev_device const char *udev_device_get_sysattr_value(struct udev_device *udev_device, const char *sysattr); int udev_device_set_sysattr_value(struct udev_device *udev_device, const char *sysattr, const char *value); int udev_device_has_tag(struct udev_device *udev_device, const char *tag); +int udev_device_has_current_tag(struct udev_device *udev_device, const char *tag); /* * udev_monitor diff --git a/src/libudev/libudev.sym b/src/libudev/libudev.sym index fb2e03e432..bad8313904 100644 --- a/src/libudev/libudev.sym +++ b/src/libudev/libudev.sym @@ -118,3 +118,9 @@ global: udev_queue_flush; udev_queue_get_fd; } LIBUDEV_199; + +LIBUDEV_247 { +global: + udev_device_has_current_tag; + udev_device_get_current_tags_list_entry; +} LIBUDEV_215; diff --git a/src/login/logind-acl.c b/src/login/logind-acl.c index 76af208af1..5b75d8f362 100644 --- a/src/login/logind-acl.c +++ b/src/login/logind-acl.c @@ -195,6 +195,10 @@ int devnode_acl_all(const char *seat, FOREACH_DEVICE(e, d) { const char *node, *sn; + /* Make sure the tag is still in place */ + if (sd_device_has_current_tag(d, "uaccess") <= 0) + continue; + if (sd_device_get_property_value(d, "ID_SEAT", &sn) < 0 || isempty(sn)) sn = "seat0"; diff --git a/src/login/logind-core.c b/src/login/logind-core.c index e0d61f3e75..0487182225 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -243,7 +243,8 @@ int manager_process_seat_device(Manager *m, sd_device *d) { assert(m); - if (device_for_action(d, DEVICE_ACTION_REMOVE)) { + if (device_for_action(d, DEVICE_ACTION_REMOVE) || + sd_device_has_current_tag(d, "seat") <= 0) { const char *syspath; r = sd_device_get_syspath(d, &syspath); @@ -271,7 +272,7 @@ int manager_process_seat_device(Manager *m, sd_device *d) { } seat = hashmap_get(m->seats, sn); - master = sd_device_has_tag(d, "master-of-seat") > 0; + master = sd_device_has_current_tag(d, "master-of-seat") > 0; /* Ignore non-master devices for unknown seats */ if (!master && !seat) @@ -313,7 +314,8 @@ int manager_process_button_device(Manager *m, sd_device *d) { if (r < 0) return r; - if (device_for_action(d, DEVICE_ACTION_REMOVE)) { + if (device_for_action(d, DEVICE_ACTION_REMOVE) || + sd_device_has_current_tag(d, "power-switch") <= 0) { b = hashmap_get(m->buttons, sysname); if (!b) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index d883288b25..bbec23c850 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -1361,7 +1361,7 @@ static int attach_device(Manager *m, const char *seat, const char *sysfs) { if (r < 0) return r; - if (sd_device_has_tag(d, "seat") <= 0) + if (sd_device_has_current_tag(d, "seat") <= 0) return -ENODEV; if (sd_device_get_property_value(d, "ID_FOR_SEAT", &id_for_seat) < 0) diff --git a/src/login/sysfs-show.c b/src/login/sysfs-show.c index a66be28ad9..9b7fc20396 100644 --- a/src/login/sysfs-show.c +++ b/src/login/sysfs-show.c @@ -53,14 +53,14 @@ static int show_sysfs_one( /* Explicitly also check for tag 'seat' here */ if (!streq(seat, sn) || - sd_device_has_tag(dev_list[*i_dev], "seat") <= 0 || + sd_device_has_current_tag(dev_list[*i_dev], "seat") <= 0 || sd_device_get_subsystem(dev_list[*i_dev], &subsystem) < 0 || sd_device_get_sysname(dev_list[*i_dev], &sysname) < 0) { (*i_dev)++; continue; } - is_master = sd_device_has_tag(dev_list[*i_dev], "master-of-seat") > 0; + is_master = sd_device_has_current_tag(dev_list[*i_dev], "master-of-seat") > 0; if (sd_device_get_sysattr_value(dev_list[*i_dev], "name", &name) < 0) (void) sd_device_get_sysattr_value(dev_list[*i_dev], "id", &name); @@ -80,7 +80,7 @@ static int show_sysfs_one( isempty(lookahead_sn)) lookahead_sn = "seat0"; - if (streq(seat, lookahead_sn) && sd_device_has_tag(dev_list[lookahead], "seat") > 0) + if (streq(seat, lookahead_sn) && sd_device_has_current_tag(dev_list[lookahead], "seat") > 0) break; } } diff --git a/src/systemd/sd-device.h b/src/systemd/sd-device.h index 3c5c88c56b..d720ce50da 100644 --- a/src/systemd/sd-device.h +++ b/src/systemd/sd-device.h @@ -64,6 +64,8 @@ int sd_device_get_usec_since_initialized(sd_device *device, uint64_t *usec); const char *sd_device_get_tag_first(sd_device *device); const char *sd_device_get_tag_next(sd_device *device); +const char *sd_device_get_current_tag_first(sd_device *device); +const char *sd_device_get_current_tag_next(sd_device *device); const char *sd_device_get_devlink_first(sd_device *device); const char *sd_device_get_devlink_next(sd_device *device); const char *sd_device_get_property_first(sd_device *device, const char **value); @@ -72,6 +74,7 @@ const char *sd_device_get_sysattr_first(sd_device *device); const char *sd_device_get_sysattr_next(sd_device *device); int sd_device_has_tag(sd_device *device, const char *tag); +int sd_device_has_current_tag(sd_device *device, const char *tag); int sd_device_get_property_value(sd_device *device, const char *key, const char **value); int sd_device_get_sysattr_value(sd_device *device, const char *sysattr, const char **_value); diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index e1c2baf7f2..e1daac21ed 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -958,6 +958,24 @@ static int udev_event_on_move(UdevEvent *event) { return 0; } +static int copy_all_tags(sd_device *d, sd_device *s) { + const char *tag; + int r; + + assert(d); + + if (!s) + return 0; + + for (tag = sd_device_get_tag_first(s); tag; tag = sd_device_get_tag_next(s)) { + r = device_add_tag(d, tag, false); + if (r < 0) + return r; + } + + return 0; +} + int udev_event_execute_rules(UdevEvent *event, usec_t timeout_usec, int timeout_signal, @@ -990,6 +1008,10 @@ int udev_event_execute_rules(UdevEvent *event, if (r < 0) return log_device_debug_errno(dev, r, "Failed to clone sd_device object: %m"); + r = copy_all_tags(dev, event->dev_db_clone); + if (r < 0) + log_device_warning_errno(dev, r, "Failed to copy all tags from old database entry, ignoring: %m"); + if (sd_device_get_devnum(dev, NULL) >= 0) /* Disable watch during event processing. */ (void) udev_watch_end(event->dev_db_clone); diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index eb28431325..018478c986 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -2017,7 +2017,7 @@ static int udev_rule_apply_token_to_event( if (token->op == OP_REMOVE) device_remove_tag(dev, buf); else { - r = device_add_tag(dev, buf); + r = device_add_tag(dev, buf, true); if (r < 0) return log_rule_error_errno(dev, rules, r, "Failed to add tag '%s': %m", buf); } diff --git a/test/TEST-55-UDEV-TAGS/Makefile b/test/TEST-55-UDEV-TAGS/Makefile new file mode 120000 index 0000000000..e9f93b1104 --- /dev/null +++ b/test/TEST-55-UDEV-TAGS/Makefile @@ -0,0 +1 @@ +../TEST-01-BASIC/Makefile \ No newline at end of file diff --git a/test/TEST-55-UDEV-TAGS/test.sh b/test/TEST-55-UDEV-TAGS/test.sh new file mode 100755 index 0000000000..325d70f011 --- /dev/null +++ b/test/TEST-55-UDEV-TAGS/test.sh @@ -0,0 +1,8 @@ +#!/usr/bin/env bash +set -e +TEST_DESCRIPTION="UDEV tags management" +TEST_NO_NSPAWN=1 + +. $TEST_BASE_DIR/test-functions + +do_test "$@" 55 diff --git a/test/test-functions b/test/test-functions index a93bba4b07..9893864bcd 100644 --- a/test/test-functions +++ b/test/test-functions @@ -673,7 +673,7 @@ get_ldpath() { install_missing_libraries() { # install possible missing libraries for i in $initdir{,/usr}/{sbin,bin}/* $initdir{,/usr}/lib/systemd/{,tests/{,manual/,unsafe/}}*; do - LD_LIBRARY_PATH="${LD_LIBRARY_PATH:+$LD_LIBRARY_PATH:}$(get_ldpath $i)" inst_libs $i + LD_LIBRARY_PATH="${LD_LIBRARY_PATH:+$LD_LIBRARY_PATH:}$(get_ldpath $i):$(get_ldpath $i)/src/udev" inst_libs $i done } diff --git a/test/units/testsuite-55.service b/test/units/testsuite-55.service new file mode 100644 index 0000000000..2127a4db7f --- /dev/null +++ b/test/units/testsuite-55.service @@ -0,0 +1,7 @@ +[Unit] +Description=TESTSUITE-55-UDEV-TAGS + +[Service] +ExecStartPre=rm -f /failed /testok +ExecStart=/usr/lib/systemd/tests/testdata/units/%N.sh +Type=oneshot diff --git a/test/units/testsuite-55.sh b/test/units/testsuite-55.sh new file mode 100755 index 0000000000..ffceefb6a5 --- /dev/null +++ b/test/units/testsuite-55.sh @@ -0,0 +1,66 @@ +#!/bin/bash +set -ex +set -o pipefail + +mkdir -p /run/udev/rules.d/ + +! test -f /run/udev/tags/added/c1:3 && + ! test -f /run/udev/tags/changed/c1:3 && + udevadm info /dev/null | grep -q -v 'E: TAGS=.*:added:.*' && + udevadm info /dev/null | grep -q -v 'E: CURRENT_TAGS=.*:added:.*' && + udevadm info /dev/null | grep -q -v 'E: TAGS=.*:changed:.*' && + udevadm info /dev/null | grep -q -v 'E: CURRENT_TAGS=.*:changed:.*' + +cat > /run/udev/rules.d/50-testsuite.rules < /testok + +exit 0