SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

pid1: use a cache for all unit aliases

Browse Source 
This reworks how we load units from disk. Instead of chasing symlinks every
time we are asked to load a unit by name, we slurp all symlinks from disk
and build two hashmaps:
1. from unit name to either alias target, or fragment on disk
   (if an alias, we put just the target name in the hashmap, if a fragment
    we put an absolute path, so we can distinguish both).
2. from a unit name to all aliases

Reading all this data can be pretty costly (40 ms) on my machine, so we keep it
around for reuse.

The advantage is that we can reliably know what all the aliases of a given unit
are. This means we can reliably load dropins under all names. This fixes #11972.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek
2019-07-30 14:01:46 +02:00
parent 7d1e91d1a9
commit e8630e6952
9 changed files with 532 additions and 390 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/load-fragment.c
+86 -288
View File
@@ -4559,251 +4559,48 @@ int config_parse_ip_filter_bpf_progs(
return 0;
}
#define FOLLOW_MAX 8
static int open_follow(char **filename, FILE **_f, Set *names, char **_final) {
char *id = NULL;
unsigned c = 0;
int fd, r;
FILE *f;
assert(filename);
assert(*filename);
assert(_f);
assert(names);
/* This will update the filename pointer if the loaded file is
* reached by a symlink. The old string will be freed. */
for (;;) {
char *target, *name;
if (c++ >= FOLLOW_MAX)
return -ELOOP;
path_simplify(*filename, false);
/* Add the file name we are currently looking at to
* the names of this unit, but only if it is a valid
* unit name. */
name = basename(*filename);
if (unit_name_is_valid(name, UNIT_NAME_ANY)) {
id = set_get(names, name);
if (!id) {
id = strdup(name);
if (!id)
return -ENOMEM;
r = set_consume(names, id);
if (r < 0)
return r;
}
}
/* Try to open the file name, but don't if its a symlink */
fd = open(*filename, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW);
if (fd >= 0)
break;
if (errno != ELOOP)
return -errno;
/* Hmm, so this is a symlink. Let's read the name, and follow it manually */
r = readlink_and_make_absolute(*filename, &target);
if (r < 0)
return r;
free_and_replace(*filename, target);
}
f = fdopen(fd, "r");
if (!f) {
safe_close(fd);
return -errno;
}
*_f = f;
*_final = id;
return 0;
}
static int merge_by_names(Unit **u, Set *names, const char *id) {
char *k;
int r;
assert(u);
assert(*u);
assert(names);
/* Let's try to add in all symlink names we found */
/* Let's try to add in all names that are aliases of this unit */
while ((k = set_steal_first(names))) {
_cleanup_free_ _unused_ char *free_k = k;
/* First try to merge in the other name into our
* unit */
/* First try to merge in the other name into our unit */
r = unit_merge_by_name(*u, k);
if (r < 0) {
Unit *other;
/* Hmm, we couldn't merge the other unit into
* ours? Then let's try it the other way
* round */
/* Hmm, we couldn't merge the other unit into ours? Then let's try it the other way
* round. */
/* If the symlink name we are looking at is unit template, then
we must search for instance of this template */
if (unit_name_is_valid(k, UNIT_NAME_TEMPLATE) && (*u)->instance) {
_cleanup_free_ char *instance = NULL;
other = manager_get_unit((*u)->manager, k);
if (!other)
return r; /* return previous failure */
r = unit_name_replace_instance(k, (*u)->instance, &instance);
if (r < 0)
return r;
r = unit_merge(other, *u);
if (r < 0)
return r;
other = manager_get_unit((*u)->manager, instance);
} else
other = manager_get_unit((*u)->manager, k);
free(k);
if (other) {
r = unit_merge(other, *u);
if (r >= 0) {
*u = other;
return merge_by_names(u, names, NULL);
}
}
return r;
*u = other;
return merge_by_names(u, names, NULL);
}
if (id == k)
if (streq_ptr(id, k))
unit_choose_id(*u, id);
free(k);
}
return 0;
}
static int load_from_path(Unit *u, const char *path) {
_cleanup_set_free_free_ Set *symlink_names = NULL;
_cleanup_fclose_ FILE *f = NULL;
_cleanup_free_ char *filename = NULL;
char *id = NULL;
Unit *merged;
struct stat st;
int r;
assert(u);
assert(path);
symlink_names = set_new(&string_hash_ops);
if (!symlink_names)
return -ENOMEM;
if (path_is_absolute(path)) {
filename = strdup(path);
if (!filename)
return -ENOMEM;
r = open_follow(&filename, &f, symlink_names, &id);
if (r < 0) {
filename = mfree(filename);
if (r != -ENOENT)
return r;
}
} else {
char **p;
STRV_FOREACH(p, u->manager->lookup_paths.search_path) {
/* Instead of opening the path right away, we manually
* follow all symlinks and add their name to our unit
* name set while doing so */
filename = path_make_absolute(path, *p);
if (!filename)
return -ENOMEM;
if (u->manager->unit_path_cache &&
!set_get(u->manager->unit_path_cache, filename))
r = -ENOENT;
else
r = open_follow(&filename, &f, symlink_names, &id);
if (r >= 0)
break;
/* ENOENT means that the file is missing or is a dangling symlink.
* ENOTDIR means that one of paths we expect to be is a directory
* is not a directory, we should just ignore that.
* EACCES means that the directory or file permissions are wrong.
*/
if (r == -EACCES)
log_debug_errno(r, "Cannot access \"%s\": %m", filename);
else if (!IN_SET(r, -ENOENT, -ENOTDIR))
return r;
filename = mfree(filename);
/* Empty the symlink names for the next run */
set_clear_free(symlink_names);
}
}
if (!filename)
/* Hmm, no suitable file found? */
return 0;
if (!unit_type_may_alias(u->type) && set_size(symlink_names) > 1) {
log_unit_warning(u, "Unit type of %s does not support alias names, refusing loading via symlink.", u->id);
return -ELOOP;
}
merged = u;
r = merge_by_names(&merged, symlink_names, id);
if (r < 0)
return r;
if (merged != u) {
u->load_state = UNIT_MERGED;
return 0;
}
if (fstat(fileno(f), &st) < 0)
return -errno;
if (null_or_empty(&st)) {
u->load_state = UNIT_MASKED;
u->fragment_mtime = 0;
} else {
u->load_state = UNIT_LOADED;
u->fragment_mtime = timespec_load(&st.st_mtim);
/* Now, parse the file contents */
r = config_parse(u->id, filename, f,
UNIT_VTABLE(u)->sections,
config_item_perf_lookup, load_fragment_gperf_lookup,
CONFIG_PARSE_ALLOW_INCLUDE, u);
if (r < 0)
return r;
}
free_and_replace(u->fragment_path, filename);
if (u->source_path) {
if (stat(u->source_path, &st) >= 0)
u->source_mtime = timespec_load(&st.st_mtim);
else
u->source_mtime = 0;
}
return 0;
}
int unit_load_fragment(Unit *u) {
const char *fragment;
_cleanup_set_free_free_ Set *names = NULL;
int r;
Iterator i;
const char *t;
assert(u);
assert(u->load_state == UNIT_STUB);
@@ -4814,78 +4611,79 @@ int unit_load_fragment(Unit *u) {
return 0;
}
/* First, try to find the unit under its id. We always look
* for unit files in the default directories, to make it easy
* to override things by placing things in /etc/systemd/system */
r = load_from_path(u, u->id);
r = unit_file_find_fragment(u->manager->unit_id_map,
u->manager->unit_name_map,
u->id,
&fragment,
&names);
if (r < 0 && r != -ENOENT)
return r;
if (fragment) {
/* Open the file, check if this is a mask, otherwise read. */
_cleanup_fclose_ FILE *f = NULL;
struct stat st;
/* Try to open the file name. A symlink is OK, for example for linked files or masks. We
* expect that all symlinks within the lookup paths have been already resolved, but we don't
* verify this here. */
f = fopen(fragment, "re");
if (!f)
return log_unit_notice_errno(u, errno, "Failed to open %s: %m", fragment);
if (fstat(fileno(f), &st) < 0)
return -errno;
r = free_and_strdup(&u->fragment_path, fragment);
if (r < 0)
return r;
if (null_or_empty(&st)) {
u->load_state = UNIT_MASKED;
u->fragment_mtime = 0;
} else {
u->load_state = UNIT_LOADED;
u->fragment_mtime = timespec_load(&st.st_mtim);
/* Now, parse the file contents */
r = config_parse(u->id, fragment, f,
UNIT_VTABLE(u)->sections,
config_item_perf_lookup, load_fragment_gperf_lookup,
CONFIG_PARSE_ALLOW_INCLUDE, u);
if (r == -ENOEXEC)
log_unit_notice_errno(u, r, "Unit configuration has fatal error, unit will not be started.");
if (r < 0)
return r;
}
}
/* We do the merge dance here because for some unit types, the unit might have aliases which are not
* declared in the file system. In particular, this is true (and frequent) for device and swap units.
*/
Unit *merged;
const char *id = u->id;
_cleanup_free_ char *free_id = NULL;
if (fragment) {
id = basename(fragment);
if (unit_name_is_valid(id, UNIT_NAME_TEMPLATE)) {
assert(u->instance); /* If we're not trying to use a template for non-instanced unit,
* this must be set. */
r = unit_name_replace_instance(id, u->instance, &free_id);
if (r < 0)
return log_debug_errno(r, "Failed to build id (%s + %s): %m", id, u->instance);
id = free_id;
}
}
merged = u;
r = merge_by_names(&merged, names, id);
if (r < 0)
return r;
/* Try to find an alias we can load this with */
if (u->load_state == UNIT_STUB) {
SET_FOREACH(t, u->names, i) {
if (t == u->id)
continue;
r = load_from_path(u, t);
if (r < 0)
return r;
if (u->load_state != UNIT_STUB)
break;
}
}
/* And now, try looking for it under the suggested (originally linked) path */
if (u->load_state == UNIT_STUB && u->fragment_path) {
r = load_from_path(u, u->fragment_path);
if (r < 0)
return r;
if (u->load_state == UNIT_STUB)
/* Hmm, this didn't work? Then let's get rid
* of the fragment path stored for us, so that
* we don't point to an invalid location. */
u->fragment_path = mfree(u->fragment_path);
}
/* Look for a template */
if (u->load_state == UNIT_STUB && u->instance) {
_cleanup_free_ char *k = NULL;
r = unit_name_template(u->id, &k);
if (r < 0)
return r;
r = load_from_path(u, k);
if (r < 0) {
if (r == -ENOEXEC)
log_unit_notice(u, "Unit configuration has fatal error, unit will not be started.");
return r;
}
if (u->load_state == UNIT_STUB) {
SET_FOREACH(t, u->names, i) {
_cleanup_free_ char *z = NULL;
if (t == u->id)
continue;
r = unit_name_template(t, &z);
if (r < 0)
return r;
r = load_from_path(u, z);
if (r < 0)
return r;
if (u->load_state != UNIT_STUB)
break;
}
}
}
if (merged != u)
u->load_state = UNIT_MERGED;
return 0;
}
src/core/manager.c
+18 -55
View File
@@ -674,6 +674,12 @@ static int manager_setup_prefix(Manager *m) {
return 0;
}
static void manager_free_unit_name_maps(Manager *m) {
m->unit_id_map = hashmap_free(m->unit_id_map);
m->unit_name_map = hashmap_free(m->unit_name_map);
m->unit_path_cache = set_free_free(m->unit_path_cache);
}
static int manager_setup_run_queue(Manager *m) {
int r;
@@ -1357,7 +1363,7 @@ Manager* manager_free(Manager *m) {
strv_free(m->client_environment);
hashmap_free(m->cgroup_unit);
set_free_free(m->unit_path_cache);
manager_free_unit_name_maps(m);
free(m->switch_root);
free(m->switch_root_init);
@@ -1461,56 +1467,6 @@ static void manager_catchup(Manager *m) {
}
}
static void manager_build_unit_path_cache(Manager *m) {
char **i;
int r;
assert(m);
set_free_free(m->unit_path_cache);
m->unit_path_cache = set_new(&path_hash_ops);
if (!m->unit_path_cache) {
r = -ENOMEM;
goto fail;
}
/* This simply builds a list of files we know exist, so that
* we don't always have to go to disk */
STRV_FOREACH(i, m->lookup_paths.search_path) {
_cleanup_closedir_ DIR *d = NULL;
struct dirent *de;
d = opendir(*i);
if (!d) {
if (errno != ENOENT)
log_warning_errno(errno, "Failed to open directory %s, ignoring: %m", *i);
continue;
}
FOREACH_DIRENT(de, d, r = -errno; goto fail) {
char *p;
p = path_join(*i, de->d_name);
if (!p) {
r = -ENOMEM;
goto fail;
}
r = set_consume(m->unit_path_cache, p);
if (r < 0)
goto fail;
}
}
return;
fail:
log_warning_errno(r, "Failed to build unit path cache, proceeding without: %m");
m->unit_path_cache = set_free_free(m->unit_path_cache);
}
static void manager_distribute_fds(Manager *m, FDSet *fds) {
Iterator i;
Unit *u;
@@ -1670,7 +1626,10 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
if (r < 0)
log_warning_errno(r, "Failed to reduce unit file paths, ignoring: %m");
manager_build_unit_path_cache(m);
manager_free_unit_name_maps(m);
r = unit_file_build_name_map(&m->lookup_paths, &m->unit_id_map, &m->unit_name_map, &m->unit_path_cache);
if (r < 0)
return log_error_errno(r, "Failed to build name map: %m");
{
/* This block is (optionally) done with the reloading counter bumped */
@@ -2888,8 +2847,9 @@ int manager_loop(Manager *m) {
assert(m);
assert(m->objective == MANAGER_OK); /* Ensure manager_startup() has been called */
/* Release the path cache */
m->unit_path_cache = set_free_free(m->unit_path_cache);
/* Release the path and unit name caches */
manager_free_unit_name_maps(m);
// FIXME: once this happens, we cannot load any more units
manager_check_finished(m);
@@ -3566,7 +3526,10 @@ int manager_reload(Manager *m) {
if (r < 0)
log_warning_errno(r, "Failed to reduce unit file paths, ignoring: %m");
manager_build_unit_path_cache(m);
manager_free_unit_name_maps(m);
r = unit_file_build_name_map(&m->lookup_paths, &m->unit_id_map, &m->unit_name_map, &m->unit_path_cache);
if (r < 0)
log_warning_errno(r, "Failed to build name map: %m");
/* First, enumerate what we can from kernel and suchlike */
manager_enumerate_perpetual(m);
src/core/manager.h
+2
View File
@@ -221,6 +221,8 @@ struct Manager {
UnitFileScope unit_file_scope;
LookupPaths lookup_paths;
Hashmap *unit_id_map;
Hashmap *unit_name_map;
Set *unit_path_cache;
char **transient_environment; /* The environment, as determined from config files, kernel cmdline and environment generators */
src/core/unit.c
+3
View File
@@ -949,6 +949,9 @@ int unit_merge_by_name(Unit *u, const char *name) {
Unit *other;
int r;
/* Either add name to u, or if a unit with name already exists, merge it with u.
* If name is a template, do the same for name@instance, where instance is u's instance. */
assert(u);
assert(name);