picnoir
/
Systemd
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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
This commit is contained in:
Lennart Poettering 2018-12-13 17:55:14 +01:00
parent 73484ecff9
commit e77b146f82
9 changed files with 160 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/libsystemd/libsystemd.sym
View File

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

8
src/libsystemd/sd-device/device-internal.h
View File

@ -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 */

21
src/libsystemd/sd-device/device-private.c
View File

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

2
src/libsystemd/sd-device/device-private.h
View File

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

5
src/libsystemd/sd-device/device-util.h
View File

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

135
src/libsystemd/sd-device/sd-device.c
View File

@ -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) {

3
src/systemd/sd-device.h
View File

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

22
src/udev/udev-event.c
View File

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

2
src/udev/udev-rules.c
View File

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