udev: make tags "sticky"

This tries to address the "bind"/"unbind" uevent kernel API breakage, by
changing the semantics of device tags.

Previously, tags would be applied on uevents (and the database entries
they result in) only depending on the immediate context. This means that
if one uevent causes the tag to be set and the next to be unset, this
would immediately effect what apps would see and the database entries
would contain each time. This is problematic however, as tags are a
filtering concept, and if tags vanish then clients won't hence notice
when a device stops being relevant to them since not only the tags
disappear but immediately also the uevents for it are filtered including
the one necessary for the app to notice that the device lost its tag and
hence relevance.

With this change tags become "sticky". If a tag is applied is once
applied to a device it will stay in place forever, until the device is
removed. Tags can never be removed again. This means that an app
watching a specific set of devices by filtering for a tag is guaranteed
to not only see the events where the tag is set but also all follow-up
events where the tags might be removed again.

This change of behaviour is unfortunate, but is required due to the
kernel introducing new "bind" and "unbind" uevents that generally have
the effect that tags and properties disappear and apps hence don't
notice when a device looses relevance to it. "bind"/"unbind" events were
introduced in kernel 4.12, and are now used in more and more subsystems.
The introduction broke userspace widely, and this commit is an attempt
to provide a way for apps to deal with it.

While tags are now "sticky" a new automatic device property
CURRENT_TAGS is introduced (matching the existing TAGS property) that
always reflects the precise set of tags applied on the most recent
events. Thus, when subscribing to devices through tags, all devices that
ever had the tag put on them will be be seen, and by CURRENT_TAGS it may
be checked whether the device right at the moment matches the tag
requirements.

See: #7587 #7018 #8221

diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym
index a3659d00b385ee11eea663f8afc8127d90121bfa..1bee5318d672b3a1b363f201bee0d3409e927032 100644 (file)
--- 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 1fe5c1a6bf291b1c4dd2e09815dd8ab68b7832f9..9c6b8a345f60ff401237bbea8fb42d8e8739d1e6 100644 (file)
--- 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 1e61732dfe6d15caf9c5be196d21518153b8f1f3..1ad7713ec7e4f88de14a9b16802891288d9bd15d 100644 (file)
--- 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 2bde53e969b1463fbb19abad73f81de8c1eafc18..1f1c4ca1071719d5c7553e0516cad82184c6ed0d 100644 (file)
--- 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 1a1795d974fb53c7e6f18510b1c6c9fa36ad5ed3..eda0f2f49e18b41231aecfc83f5958154d77e4f4 100644 (file)
--- 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 3bba17aff853b0684da9798b313e77d13a747015..3041ce2e9c5080bc91181fbe797bb2929f975495 100644 (file)
--- 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->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->tags, &device->tags_iterator, &v);
+        (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/systemd/sd-device.h b/src/systemd/sd-device.h
index 3c5c88c56bdfddd40c061a00b3a8f92ec751f6f6..d720ce50da17ec09a9289a835ee6a263636a8d04 100644 (file)
--- 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 e1c2baf7f21247bbc44ef47dcdaa09731fdb5c57..e1daac21ed7034c7759a0bd77fefc263e7a8b6fd 100644 (file)
--- 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 eb28431325b55948e2bc1bb5bcf938466c196dea..018478c9869483111dd132531af28ef58b40fc44 100644 (file)
--- 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);
                 }