Merge pull request #16853 from poettering/udev-current-tag2

udev: make uevents "sticky"

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

man/udev_device_has_tag.xml

@@ -21,9 +21,11 @@
 
   <refnamediv>
     <refname>udev_device_has_tag</refname>
+    <refname>udev_device_has_current_tag</refname>
     <refname>udev_device_get_devlinks_list_entry</refname>
     <refname>udev_device_get_properties_list_entry</refname>
     <refname>udev_device_get_tags_list_entry</refname>
+    <refname>udev_device_get_current_tags_list_entry</refname>
     <refname>udev_device_get_sysattr_list_entry</refname>
     <refname>udev_device_get_property_value</refname>
     <refname>udev_device_get_sysattr_value</refname>
@@ -36,6 +38,18 @@
     <funcsynopsis>
       <funcsynopsisinfo>#include &lt;libudev.h&gt;</funcsynopsisinfo>
 
+      <funcprototype>
+        <funcdef>int <function>udev_device_has_tag</function></funcdef>
+        <paramdef>struct udev_device *<parameter>udev_device</parameter></paramdef>
+        <paramdef>const char *<parameter>tag</parameter></paramdef>
+      </funcprototype>
+
+      <funcprototype>
+        <funcdef>int <function>udev_device_has_current_tag</function></funcdef>
+        <paramdef>struct udev_device *<parameter>udev_device</parameter></paramdef>
+        <paramdef>const char *<parameter>tag</parameter></paramdef>
+      </funcprototype>
+
       <funcprototype>
         <funcdef>struct udev_list_entry *<function>udev_device_get_devlinks_list_entry</function></funcdef>
         <paramdef>struct udev_device *<parameter>udev_device</parameter></paramdef>
@@ -51,6 +65,11 @@
         <paramdef>struct udev_device *<parameter>udev_device</parameter></paramdef>
       </funcprototype>
 
+      <funcprototype>
+        <funcdef>struct udev_list_entry *<function>udev_device_get_current_tags_list_entry</function></funcdef>
+        <paramdef>struct udev_device *<parameter>udev_device</parameter></paramdef>
+      </funcprototype>
+
       <funcprototype>
         <funcdef>struct udev_list_entry *<function>udev_device_get_sysattr_list_entry</function></funcdef>
         <paramdef>struct udev_device *<parameter>udev_device</parameter></paramdef>
@@ -62,12 +81,6 @@
         <paramdef>const char *<parameter>key</parameter></paramdef>
       </funcprototype>
 
-      <funcprototype>
-        <funcdef>int <function>udev_device_has_tag</function></funcdef>
-        <paramdef>struct udev_device *<parameter>udev_device</parameter></paramdef>
-        <paramdef>const char *<parameter>tag</parameter></paramdef>
-      </funcprototype>
-
       <funcprototype>
         <funcdef>const char *<function>udev_device_get_sysattr_value</function></funcdef>
         <paramdef>struct udev_device *<parameter>udev_device</parameter></paramdef>
@@ -84,22 +97,40 @@
     </funcsynopsis>
   </refsynopsisdiv>
 
-  <!--<refsect1>
+  <refsect1>
     <title>Description</title>
 
-    <para>XXX: Add short description.</para>
+    <para><function>udev_device_has_tag()</function> returns a valuer larger than zero if the specified
-  </refsect1>-->
+    device object has the indicated tag assigned to it, and zero otherwise. See
+    <citerefentry><refentrytitle>udev</refentrytitle><manvolnum>7</manvolnum></citerefentry> for details on
+    the tags concept. <function>udev_device_has_current_tag()</function> 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 <function>udev_device_has_current_tag()</function> returns true will hence also return true when
+    passed to <function>udev_device_has_tag()</function>, 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.</para>
+
+    <para><function>udev_device_get_tags_list_entry()</function> returns a a
+    <function>udev_list_entry</function> object, encapsulating a list of tags set for the specified
+    device. Similar, <function>udev_device_get_current_tags_list_entry()</function> 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).</para>
+  </refsect1>
 
   <refsect1>
     <title>Return Value</title>
 
-    <para>On success,
+    <para>On success, <function>udev_device_has_tag()</function> and
-    <function>udev_device_get_devlinks_list_entry()</function>,
+    <function>udev_device_has_current_tag()</function> return positive or <constant>0</constant>, depending
+    on whether the device has the given tag or not.  On failure, a negative error code is returned.</para>
+
+    <para>On success, <function>udev_device_get_devlinks_list_entry()</function>,
     <function>udev_device_get_properties_list_entry()</function>,
-    <function>udev_device_get_tags_list_entry()</function> and
+    <function>udev_device_get_tags_list_entry()</function>,
-    <function>udev_device_get_sysattr_list_entry()</function> return
+    <function>udev_device_get_current_tags_list_entry()</function> and
-    a pointer to the first entry of the retrieved list. If that list
+    <function>udev_device_get_sysattr_list_entry()</function> return a pointer to the first entry of the
-    is empty, or if an error occurred, <constant>NULL</constant> is
+    retrieved list. If that list is empty, or if an error occurred, <constant>NULL</constant> is
     returned.</para>
 
     <para>On success,
@@ -119,17 +150,13 @@
     contain <constant>NUL</constant> bytes should not be set with
     this function; instead, write them directly to the files within
     the device's <property>syspath</property>.</para>
-
-    <para>On success, <function>udev_device_has_tag()</function>
-    returns <constant>1</constant> or <constant>0</constant>,
-    depending on whether the device has the given tag or not.
-    On failure, a negative error code is returned.</para>
   </refsect1>
 
   <refsect1>
     <title>See Also</title>
 
     <para>
+      <citerefentry><refentrytitle>udev</refentrytitle><manvolnum>7</manvolnum></citerefentry>,
       <citerefentry><refentrytitle>udev_new</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
       <citerefentry><refentrytitle>udev_device_new_from_syspath</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
       <citerefentry><refentrytitle>udev_device_get_syspath</refentrytitle><manvolnum>3</manvolnum></citerefentry>,

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

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;
 

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;

src/libsystemd/sd-device/device-internal.h

@@ -28,10 +28,10 @@ struct sd_device {
         Set *sysattrs; /* names of sysattrs */
         Iterator sysattrs_iterator;
 
-        Set *tags;
+        Set *all_tags, *current_tags;
-        Iterator tags_iterator;
+        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 */

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->all_tags = set_free_free(device->all_tags);
-        device->tags = NULL;
+        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);

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);

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;                                       \

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 device_add_tag(sd_device *device, const char *tag, bool both) {
-        int r;
+        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);
+        /* Definitely add to the "all" list of tags (i.e. the sticky list) */
-        if (r < 0)
+        added = set_put_strdup(&device->all_tags, tag);
-                return r;
+        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':
+        case 'G': /* Any tag */
-                r = device_add_tag(device, value);
+        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->all_tags_iterator_generation = device->tags_generation;
-        device->tags_iterator = ITERATOR_FIRST;
+        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)) {
+                if (!streq(tags, ":")) {
-                        char *e;
+                        r = device_add_property_internal(device, "TAGS", tags);
-
+                        if (r < 0)
-                        if (!GREEDY_REALLOC(tags, tags_allocated, tags_len + strlen(tag) + 2))
+                                return r;
-                                return -ENOMEM;
-                        e = stpcpy(stpcpy(tags + tags_len, tag), ":");
-                        tags_len = e - tags;
                 }
 
-                r = device_add_property_internal(device, "TAGS", tags);
+                free(tags);
-                if (r < 0)
+                tags = join_string_set(device->current_tags);
-                        return r;
+                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) {

src/libudev/libudev-device.c

@@ -56,12 +56,13 @@ struct udev_device {
 
         struct udev_list *properties;
         uint64_t properties_generation;
-        struct udev_list *tags;
+        struct udev_list *all_tags, *current_tags;
-        uint64_t tags_generation;
+        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);
+        all_tags = udev_list_new(true);
-        if (!tags)
+        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 ||
+        if (device_get_tags_generation(udev_device->device) != udev_device->all_tags_generation ||
-            !udev_device->tags_read) {
+            !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->all_tags_read = true;
-                udev_device->tags_generation = device_get_tags_generation(udev_device->device);
+                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);
 

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

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;

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";
 

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)

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)

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;
                         }
                 }

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);
 

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);

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);
                 }

test/TEST-55-UDEV-TAGS/Makefile

@@ -0,0 +1 @@
+../TEST-01-BASIC/Makefile

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

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
 }
 

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

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 <<EOF
+ACTION=="add", SUBSYSTEM=="mem", KERNEL=="null", TAG+="added"
+ACTION=="change", SUBSYSTEM=="mem", KERNEL=="null", TAG+="changed"
+EOF
+
+udevadm control --reload
+udevadm trigger -c add /dev/null
+
+while : ; do
+    test -f /run/udev/tags/added/c1:3 &&
+        ! test -f /run/udev/tags/changed/c1:3 &&
+        udevadm info /dev/null | grep -q 'E: TAGS=.*:added:.*' &&
+        udevadm info /dev/null | grep -q '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:.*' &&
+        break
+
+    sleep .5
+done
+
+udevadm control --reload
+udevadm trigger -c change /dev/null
+
+while : ; do
+    test -f /run/udev/tags/added/c1:3 &&
+        test -f /run/udev/tags/changed/c1:3 &&
+        udevadm info /dev/null | grep -q 'E: TAGS=.*:added:.*' &&
+        udevadm info /dev/null | grep -q -v 'E: CURRENT_TAGS=.*:added:.*' &&
+        udevadm info /dev/null | grep -q 'E: TAGS=.*:changed:.*' &&
+        udevadm info /dev/null | grep -q 'E: CURRENT_TAGS=.*:changed:.*' &&
+        break
+
+    sleep .5
+done
+
+udevadm control --reload
+udevadm trigger -c add /dev/null
+
+while : ; do
+    test -f /run/udev/tags/added/c1:3 &&
+        test -f /run/udev/tags/changed/c1:3 &&
+        udevadm info /dev/null | grep -q 'E: TAGS=.*:added:.*' &&
+        udevadm info /dev/null | grep -q 'E: CURRENT_TAGS=.*:added:.*' &&
+        udevadm info /dev/null | grep -q 'E: TAGS=.*:changed:.*' &&
+        udevadm info /dev/null | grep -q -v 'E: CURRENT_TAGS=.*:changed:.*' &&
+        break
+
+    sleep .5
+done
+
+echo OK > /testok
+
+exit 0