Hi, 2014-10-18 1:34 GMT+02:00 Ken Sedgwick <ksedg...@bonsai.com>: > The current find_symlinks_fd code traverses the config directories > duplicatively. This is a performance problem if 1000s of units are > being controlled. This patch adds a hashmap cache of symbolic link > state which is filled in one traversal and then consulted as needed to > prevent re-traversal. > > The enabled_context cache lives in the manager struct. Initially the > cache is empty and is filled on first use. A pointer to the cache is > passed to all routines which can call find_symlinks_fd. If a NULL is > passed to find_symlinks_fd a temporary cache is created and used for > the single call. > > The cache has two levels, the first is keyed by config_path and the > second by the names and paths of the units. > > The cache contains both forward and reverse mappings; from symbolic > link name to target and from target to symbolic link name. > > The cache contains entries for both the basename of the unit and the > full path of the link name and target. > > The test-enabled patch (previously submitted) checks that the results > of unit_file_get_state and unit_file_get_list do not change as a > result of this cache. This patch presumes the test-enabled patch has > already been applied. > > The test-manyunits patch (previously submitted) checks that the performance > of unit_file_get_list is acceptable in the face of thousands of units. > This patch presumes the test-manyunits patch has already been applied. If there is dependencies between your patches, you should use something like "git format-patch -3" to generate 3 patchs from the 3 last commits. It will add [PATCH 1/3] [PATCH 2/3]... on the subjects of the mails. It helps knowing if a patch needs to be applied before applying another.
> --- > src/core/dbus-manager.c | 6 +- > src/core/manager.c | 6 + > src/core/manager.h | 2 + > src/core/unit.c | 2 +- > src/shared/install.c | 273 > +++++++++++++++++++++++++++++++++++++--------- > src/shared/install.h | 18 ++- > src/systemctl/systemctl.c | 6 +- > src/test/test-enabled.c | 24 ++-- > src/test/test-install.c | 87 ++++++++------- > src/test/test-manyunits.c | 11 +- > src/test/test-unit-file.c | 2 +- > 11 files changed, 325 insertions(+), 112 deletions(-) > > diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c > index 57db1c9..0ce41b0 100644 > --- a/src/core/dbus-manager.c > +++ b/src/core/dbus-manager.c > @@ -1403,7 +1403,7 @@ static int method_list_unit_files(sd_bus *bus, > sd_bus_message *message, void *us > if (!h) > return -ENOMEM; > > - r = unit_file_get_list(m->running_as == SYSTEMD_SYSTEM ? > UNIT_FILE_SYSTEM : UNIT_FILE_USER, NULL, h); > + r = unit_file_get_list(m->running_as == SYSTEMD_SYSTEM ? > UNIT_FILE_SYSTEM : UNIT_FILE_USER, NULL, h, m->enabled); > if (r < 0) > goto fail; > > @@ -1454,7 +1454,7 @@ static int method_get_unit_file_state(sd_bus *bus, > sd_bus_message *message, void > > scope = m->running_as == SYSTEMD_SYSTEM ? UNIT_FILE_SYSTEM : > UNIT_FILE_USER; > > - state = unit_file_get_state(scope, NULL, name); > + state = unit_file_get_state(scope, NULL, name, m->enabled); > if (state < 0) > return state; > > @@ -1843,7 +1843,7 @@ static int method_add_dependency_unit_files(sd_bus > *bus, sd_bus_message *message > > scope = m->running_as == SYSTEMD_SYSTEM ? UNIT_FILE_SYSTEM : > UNIT_FILE_USER; > > - r = unit_file_add_dependency(scope, runtime, NULL, l, target, dep, > force, &changes, &n_changes); > + r = unit_file_add_dependency(scope, runtime, NULL, l, target, dep, > force, NULL, &changes, &n_changes); > if (r < 0) > return r; > > diff --git a/src/core/manager.c b/src/core/manager.c > index 726977f..f8f41fb 100644 > --- a/src/core/manager.c > +++ b/src/core/manager.c > @@ -465,6 +465,10 @@ int manager_new(SystemdRunningAs running_as, bool > test_run, Manager **_m) { > if (r < 0) > goto fail; > > + m->enabled = enabled_context_new(); > + if (!m->enabled) > + goto fail; > + > r = set_ensure_allocated(&m->startup_units, NULL); > if (r < 0) > goto fail; > @@ -822,6 +826,8 @@ void manager_free(Manager *m) { > hashmap_free(m->watch_pids2); > hashmap_free(m->watch_bus); > > + enabled_context_free(m->enabled); > + > set_free(m->startup_units); > set_free(m->failed_units); > > diff --git a/src/core/manager.h b/src/core/manager.h > index 8e3c146..3f54fe0 100644 > --- a/src/core/manager.h > +++ b/src/core/manager.h > @@ -72,6 +72,7 @@ typedef enum ManagerExitCode { > #include "unit-name.h" > #include "exit-status.h" > #include "show-status.h" > +#include "install.h" > #include "failure-action.h" > > struct Manager { > @@ -82,6 +83,7 @@ struct Manager { > /* Active jobs and units */ > Hashmap *units; /* name string => Unit object n:1 */ > Hashmap *jobs; /* job id => Job object 1:1 */ > + EnabledContext *enabled; /* name string => is enabled */ > > /* To make it easy to iterate through the units of a specific > * type we maintain a per type linked list */ > diff --git a/src/core/unit.c b/src/core/unit.c > index 0389e6e..3e29944 100644 > --- a/src/core/unit.c > +++ b/src/core/unit.c > @@ -2941,7 +2941,7 @@ UnitFileState unit_get_unit_file_state(Unit *u) { > if (u->unit_file_state < 0 && u->fragment_path) > u->unit_file_state = unit_file_get_state( > u->manager->running_as == SYSTEMD_SYSTEM ? > UNIT_FILE_SYSTEM : UNIT_FILE_USER, > - NULL, basename(u->fragment_path)); > + NULL, basename(u->fragment_path), > u->manager->enabled); > > return u->unit_file_state; > } > diff --git a/src/shared/install.c b/src/shared/install.c > index ff5dcba..dfc1944 100644 > --- a/src/shared/install.c > +++ b/src/shared/install.c > @@ -25,6 +25,7 @@ > #include <string.h> > #include <fnmatch.h> > > +#include "macro.h" > #include "util.h" > #include "mkdir.h" > #include "hashmap.h" > @@ -382,21 +383,106 @@ static int remove_marked_symlinks( > return r; > } > > -static int find_symlinks_fd( > - const char *name, > +EnabledContext *enabled_context_new(void) { > + EnabledContext *ec; > + int r; > + > + ec = new0(EnabledContext, 1); > + if (!ec) > + return NULL; > + > + r = hashmap_ensure_allocated(&ec->config_paths_forward, > &string_hash_ops); > + if (r < 0) { > + free(ec); > + return NULL; > + } > + > + r = hashmap_ensure_allocated(&ec->config_paths_reverse, > &string_hash_ops); > + if (r < 0) { > + free(ec); > + return NULL; > + } > + > + return ec; > +} > + > +void enabled_context_free(EnabledContext *ec) { > + char *key; > + Hashmap *h; if (!ec) return; > + > + while ((key = hashmap_first_key(ec->config_paths_forward))) { > + h = hashmap_steal_first(ec->config_paths_forward); > + hashmap_free_free_free(h); > + free(key); > + } > + hashmap_free_free_free(ec->config_paths_forward); > + ec->config_paths_forward = NULL; > + > + while ((key = hashmap_first_key(ec->config_paths_reverse))) { > + h = hashmap_steal_first(ec->config_paths_reverse); > + hashmap_free_free_free(h); > + free(key); > + } > + hashmap_free_free_free(ec->config_paths_reverse); > + ec->config_paths_reverse = NULL; > + > + free(ec); > +} > + > +static int insert_link_mapping(Hashmap *h, const char * key, const char > *val) { > + int r = 0; > + char *keycpy, *valcpy; assert(h); assert(key); assert(val); > + > + /* Make copies of the key and value */ > + keycpy = strdup(key); Missing OOM check > + valcpy = strdup(val); Missing OOM check > + > + r = hashmap_put(h, keycpy, valcpy); > + if (r == 1) { > + /* Our copies were inserted, we're done */ > + return 0; > + } else { > + /* Either an error or the mapping already existed, either way > + * we don't want the key and value copies any more ... > + */ > + free(keycpy); > + free(valcpy); > + return r; > + } > +} > + > +static int fill_enabled_context( > int fd, > const char *path, > const char *config_path, > - bool *same_name_link) { > - > + EnabledContext *ec) { > int r = 0; > _cleanup_closedir_ DIR *d = NULL; > - > - assert(name); > - assert(fd >= 0); > - assert(path); > - assert(config_path); Why remove these asserts ? > - assert(same_name_link); > + Hashmap *config_path_forward; > + Hashmap *config_path_reverse; > + > + config_path_forward = hashmap_get(ec->config_paths_forward, > config_path); > + config_path_reverse = hashmap_get(ec->config_paths_reverse, > config_path); > + > + /* If config_path_forward isn't cached, generate both directions */ > + if (config_path_forward == NULL) { > + /* Initialize empty forward and reverse lookups for the > + * enabled context cache */ > + r = hashmap_ensure_allocated(&config_path_forward, > &string_hash_ops); > + if (r < 0) { > + return r; > + } > + r = hashmap_put(ec->config_paths_forward, > strdup(config_path), config_path_forward); > + if (r < 0) > + return r; > + r = hashmap_ensure_allocated(&config_path_reverse, > &string_hash_ops); > + if (r < 0) { > + return r; > + } > + r = hashmap_put(ec->config_paths_reverse, > strdup(config_path), config_path_reverse); > + if (r < 0) > + return r; > + } > > d = fdopendir(fd); > if (!d) { > @@ -413,7 +499,7 @@ static int find_symlinks_fd( > return -errno; > > if (!de) > - return r; > + break; > > if (ignore_file(de->d_name)) > continue; > @@ -441,15 +527,12 @@ static int find_symlinks_fd( > } > > /* This will close nfd, regardless whether it > succeeds or not */ > - q = find_symlinks_fd(name, nfd, p, config_path, > same_name_link); > - if (q > 0) > - return 1; > + q = fill_enabled_context(nfd, p, config_path, ec); > if (r == 0) > r = q; > > } else if (de->d_type == DT_LNK) { > _cleanup_free_ char *p = NULL, *dest = NULL; > - bool found_path, found_dest, b = false; > int q; > > /* Acquire symlink name */ > @@ -468,44 +551,121 @@ static int find_symlinks_fd( > continue; > } > > - /* Check if the symlink itself matches what we > - * are looking for */ > - if (path_is_absolute(name)) > - found_path = path_equal(p, name); > - else > - found_path = streq(de->d_name, name); > + /* Insert symlink's own full path and > + * name as keys pointing to the unit's > + * full path */ > + q = insert_link_mapping(config_path_forward, p, > dest); > + if (r == 0) > + r = q; > + q = insert_link_mapping(config_path_forward, > de->d_name, dest); > + if (r == 0) > + r = q; > > - /* Check if what the symlink points to > - * matches what we are looking for */ > - if (path_is_absolute(name)) > - found_dest = path_equal(dest, name); > - else > - found_dest = streq(basename(dest), name); > + /* Insert full path and base of the > + * symlink target as keys pointing to > + * the symlink's own full path */ > + q = insert_link_mapping(config_path_reverse, dest, > p); > + if (r == 0) > + r = q; > + q = insert_link_mapping(config_path_reverse, > basename(dest), p); > + if (r == 0) > + r = q; > + } > + } > > - if (found_path && found_dest) { > - _cleanup_free_ char *t = NULL; > + return r; > +} > > - /* Filter out same name links in the main > - * config path */ > - t = path_make_absolute(name, config_path); > - if (!t) > - return -ENOMEM; > +static int find_symlinks_fd( > + const char *name, > + int fd, > + const char *path, > + const char *config_path, > + bool *same_name_link, > + EnabledContext *ec) { > > - b = path_equal(t, p); > - } > + int r = 0; > + _cleanup_closedir_ DIR *d = NULL; > + _cleanup_enabled_context_ EnabledContext *temp_ec = NULL; > + char *symlink_path; > + char *symlink_target_path; > + Hashmap *config_path_forward; > + Hashmap *config_path_reverse; > + bool found_path, found_dest, b = false; > > - if (b) > - *same_name_link = true; > - else if (found_path || found_dest) > - return 1; > - } > + assert(name); > + assert(fd >= 0); > + assert(path); > + assert(config_path); > + assert(same_name_link); > + > + /* If no ec is passed in, use a temporary one that auto-frees */ > + if (ec == NULL) { > + ec = enabled_context_new(); > + if (ec == NULL) > + return -ENOMEM; > + temp_ec = ec; > + } > + > + config_path_forward = hashmap_get(ec->config_paths_forward, > config_path); > + > + /* If config_path_forward isn't cached, generate both directions */ > + if (config_path_forward == NULL) { > + r = fill_enabled_context(fd, path, config_path, ec); > + if (r < 0) > + return r; > + } else { > + safe_close(fd); > + } > + > + /* Refetch, may have been filled. */ > + config_path_forward = hashmap_get(ec->config_paths_forward, > config_path); > + config_path_reverse = hashmap_get(ec->config_paths_reverse, > config_path); > + > + /* Use the cache to perform a reverse lookup of symlink > + * destinations to see if any one matches what we are looking > + * for. Because both full and base names are keys, there is no > + * need to check if "name" is a full path or base name. */ > + symlink_path = hashmap_get(config_path_reverse, name); > + found_dest = (symlink_path != NULL); > + > + /* Choose a strategy to determine if the unit's name matches the > + * symlink's name. If the reverse lookup succeeded, we can skip > + * the forward lookup. */ > + if (found_dest) { > + if (path_is_absolute(name)) > + found_path = path_equal(symlink_path, name); > + else > + found_path = streq(basename(symlink_path), name); > + } else { > + symlink_target_path = hashmap_get(config_path_forward, name); > + found_path = (symlink_target_path != NULL); > } > + > + if (found_path && found_dest) { > + _cleanup_free_ char *t = NULL; > + > + /* Filter out same name links in the main config path */ > + t = path_make_absolute(name, config_path); > + if (!t) > + return -ENOMEM; > + > + b = path_equal(t, symlink_path); > + } > + > + if (b) > + *same_name_link = true; > + else if (found_path || found_dest) > + return 1; > + > + return 0; > } > > static int find_symlinks( > const char *name, > const char *config_path, > - bool *same_name_link) { > + bool *same_name_link, > + EnabledContext *ec) { > > int fd; > > @@ -521,14 +681,15 @@ static int find_symlinks( > } > > /* This takes possession of fd and closes it */ > - return find_symlinks_fd(name, fd, config_path, config_path, > same_name_link); > + return find_symlinks_fd(name, fd, config_path, config_path, > same_name_link, ec); > } > > static int find_symlinks_in_scope( > UnitFileScope scope, > const char *root_dir, > const char *name, > - UnitFileState *state) { > + UnitFileState *state, > + EnabledContext *ec) { > > int r; > _cleanup_free_ char *path = NULL; > @@ -544,7 +705,7 @@ static int find_symlinks_in_scope( > if (r < 0) > return r; > > - r = find_symlinks(name, path, &same_name_link_runtime); > + r = find_symlinks(name, path, &same_name_link_runtime, ec); > if (r < 0) > return r; > else if (r > 0) { > @@ -557,7 +718,7 @@ static int find_symlinks_in_scope( > if (r < 0) > return r; > > - r = find_symlinks(name, path, &same_name_link); > + r = find_symlinks(name, path, &same_name_link, ec); > if (r < 0) > return r; > else if (r > 0) { > @@ -1504,6 +1665,7 @@ int unit_file_add_dependency( > char *target, > UnitDependency dep, > bool force, > + EnabledContext *ec, > UnitFileChange **changes, > unsigned *n_changes) { > > @@ -1528,7 +1690,7 @@ int unit_file_add_dependency( > STRV_FOREACH(i, files) { > UnitFileState state; > > - state = unit_file_get_state(scope, root_dir, *i); > + state = unit_file_get_state(scope, root_dir, *i, ec); > if (state < 0) { > log_error("Failed to get unit file state for %s: > %s", *i, strerror(-state)); > return state; > @@ -1602,7 +1764,7 @@ int unit_file_enable( > STRV_FOREACH(i, files) { > UnitFileState state; > > - state = unit_file_get_state(scope, root_dir, *i); > + state = unit_file_get_state(scope, root_dir, *i, NULL); > if (state < 0) { > log_error("Failed to get unit file state for %s: > %s", *i, strerror(-state)); > return state; > @@ -1784,7 +1946,8 @@ int unit_file_get_default( > UnitFileState unit_file_get_state( > UnitFileScope scope, > const char *root_dir, > - const char *name) { > + const char *name, > + EnabledContext *ec) { > > _cleanup_lookup_paths_free_ LookupPaths paths = {}; > UnitFileState state = _UNIT_FILE_STATE_INVALID; > @@ -1847,7 +2010,7 @@ UnitFileState unit_file_get_state( > } > } > > - r = find_symlinks_in_scope(scope, root_dir, name, &state); > + r = find_symlinks_in_scope(scope, root_dir, name, &state, > ec); > if (r < 0) > return r; > else if (r > 0) > @@ -2130,7 +2293,8 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(UnitFileList*, > unit_file_list_free_one); > int unit_file_get_list( > UnitFileScope scope, > const char *root_dir, > - Hashmap *h) { > + Hashmap *h, > + EnabledContext *ec) { > > _cleanup_lookup_paths_free_ LookupPaths paths = {}; > char **i; > @@ -2214,7 +2378,7 @@ int unit_file_get_list( > goto found; > } > > - r = find_symlinks_in_scope(scope, root_dir, > de->d_name, &f->state); > + r = find_symlinks_in_scope(scope, root_dir, > de->d_name, &f->state, ec); > if (r < 0) > return r; > else if (r > 0) { > @@ -2246,7 +2410,10 @@ int unit_file_get_list( > } > } > > - return r; > + /* The 1 that hashmap_put in the "found" section returns on > + * successful put can get here. No valid error r value can > + * get here. */ > + return 0; > } > > static const char* const unit_file_state_table[_UNIT_FILE_STATE_MAX] = { > diff --git a/src/shared/install.h b/src/shared/install.h > index c0b4df6..df7b488 100644 > --- a/src/shared/install.h > +++ b/src/shared/install.h > @@ -84,6 +84,11 @@ typedef struct { > char *default_instance; > } InstallInfo; > > +typedef struct { > + Hashmap* config_paths_forward; /* config_path string => symlink > name/path => unit_name path */ > + Hashmap* config_paths_reverse; /* config_path string => > unit_name/path string => symlink path */ > +} EnabledContext; > + > int unit_file_enable(UnitFileScope scope, bool runtime, const char > *root_dir, char **files, bool force, UnitFileChange **changes, unsigned > *n_changes); > int unit_file_disable(UnitFileScope scope, bool runtime, const char > *root_dir, char **files, UnitFileChange **changes, unsigned *n_changes); > int unit_file_reenable(UnitFileScope scope, bool runtime, const char > *root_dir, char **files, bool force, UnitFileChange **changes, unsigned > *n_changes); > @@ -94,11 +99,11 @@ int unit_file_mask(UnitFileScope scope, bool runtime, > const char *root_dir, char > int unit_file_unmask(UnitFileScope scope, bool runtime, const char > *root_dir, char **files, UnitFileChange **changes, unsigned *n_changes); > int unit_file_set_default(UnitFileScope scope, const char *root_dir, const > char *file, bool force, UnitFileChange **changes, unsigned *n_changes); > int unit_file_get_default(UnitFileScope scope, const char *root_dir, char > **name); > -int unit_file_add_dependency(UnitFileScope scope, bool runtime, const char > *root_dir, char **files, char *target, UnitDependency dep, bool force, > UnitFileChange **changes, unsigned *n_changes); > +int unit_file_add_dependency(UnitFileScope scope, bool runtime, const char > *root_dir, char **files, char *target, UnitDependency dep, bool force, > EnabledContext *ec, UnitFileChange **changes, unsigned *n_changes); > > -UnitFileState unit_file_get_state(UnitFileScope scope, const char *root_dir, > const char *filename); > +UnitFileState unit_file_get_state(UnitFileScope scope, const char *root_dir, > const char *filename, EnabledContext *ec); > > -int unit_file_get_list(UnitFileScope scope, const char *root_dir, Hashmap > *h); > +int unit_file_get_list(UnitFileScope scope, const char *root_dir, Hashmap > *h, EnabledContext *ec); > > void unit_file_list_free(Hashmap *h); > void unit_file_changes_free(UnitFileChange *changes, unsigned n_changes); > @@ -111,5 +116,12 @@ UnitFileState unit_file_state_from_string(const char *s) > _pure_; > const char *unit_file_change_type_to_string(UnitFileChangeType s) _const_; > UnitFileChangeType unit_file_change_type_from_string(const char *s) _pure_; > > +EnabledContext *enabled_context_new(void); > +void enabled_context_free(EnabledContext *ec); > + > +DEFINE_TRIVIAL_CLEANUP_FUNC(EnabledContext*, enabled_context_free); > + > +#define _cleanup_enabled_context_ _cleanup_(enabled_context_freep) > + > const char *unit_file_preset_mode_to_string(UnitFilePresetMode m) _const_; > UnitFilePresetMode unit_file_preset_mode_from_string(const char *s) _pure_; > diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c > index 28eaa6a..a42af8b 100644 > --- a/src/systemctl/systemctl.c > +++ b/src/systemctl/systemctl.c > @@ -1325,7 +1325,7 @@ static int list_unit_files(sd_bus *bus, char **args) { > if (!h) > return log_oom(); > > - r = unit_file_get_list(arg_scope, arg_root, h); > + r = unit_file_get_list(arg_scope, arg_root, h, NULL); > if (r < 0) { > unit_file_list_free(h); > log_error("Failed to get unit file list: %s", > strerror(-r)); > @@ -5420,7 +5420,7 @@ static int add_dependency(sd_bus *bus, char **args) { > UnitFileChange *changes = NULL; > unsigned n_changes = 0; > > - r = unit_file_add_dependency(arg_scope, arg_runtime, > arg_root, names, target, dep, arg_force, &changes, &n_changes); > + r = unit_file_add_dependency(arg_scope, arg_runtime, > arg_root, names, target, dep, arg_force, NULL, &changes, &n_changes); > > if (r < 0) { > log_error("Can't add dependency: %s", strerror(-r)); > @@ -5567,7 +5567,7 @@ static int unit_is_enabled(sd_bus *bus, char **args) { > STRV_FOREACH(name, names) { > UnitFileState state; > > - state = unit_file_get_state(arg_scope, arg_root, > *name); > + state = unit_file_get_state(arg_scope, arg_root, > *name, NULL); > if (state < 0) { > log_error("Failed to get unit file state for > %s: %s", *name, strerror(-state)); > return state; > diff --git a/src/test/test-enabled.c b/src/test/test-enabled.c > index 104348e..fa5d596 100644 > --- a/src/test/test-enabled.c > +++ b/src/test/test-enabled.c > @@ -76,9 +76,9 @@ > > > #define confirm_unit_state(unit, expected) \ > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, unit) == > expected) > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, unit, ec) > == expected) > > -static void test_enabled(int argc, char* argv[]) { > +static void test_enabled(int argc, char* argv[], EnabledContext *ec) { > Hashmap *h; > UnitFileList *p; > Iterator i; > @@ -87,6 +87,7 @@ static void test_enabled(int argc, char* argv[]) { > > root_dir = strappenda(TEST_DIR, "/test-enabled-root"); > > + /* Explicitly check each of the units. */ > confirm_unit_state("nonexistent.service", -ENOENT); > confirm_unit_state("invalid.service", -EBADMSG); > confirm_unit_state("disabled.service", > UNIT_FILE_DISABLED); > @@ -102,15 +103,15 @@ static void test_enabled(int argc, char* argv[]) { > confirm_unit_state("templating@three.service", UNIT_FILE_ENABLED); > confirm_unit_state("unique.service", UNIT_FILE_ENABLED); > > + /* Reconcile unit_file_get_list with the return for each unit. */ > h = hashmap_new(&string_hash_ops); > - r = unit_file_get_list(UNIT_FILE_SYSTEM, root_dir, h); > - assert_se(r >= 0); > - > + r = unit_file_get_list(UNIT_FILE_SYSTEM, root_dir, h, ec); > + assert_se(r == 0); > HASHMAP_FOREACH(p, h, i) { > UnitFileState s; > > s = unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, > - basename(p->path)); > + basename(p->path), ec); > > /* unit_file_get_list and unit_file_get_state are > * a little different in some cases. Handle these > @@ -136,6 +137,15 @@ static void test_enabled(int argc, char* argv[]) { > } > > int main(int argc, char* argv[]) { > - test_enabled(argc, argv); > + _cleanup_enabled_context_ EnabledContext *ec = NULL; > + > + /* built-in EnabledContext */ > + test_enabled(argc, argv, NULL); > + > + /* explicit EnabledContext */ > + ec = enabled_context_new(); > + assert(ec); > + test_enabled(argc, argv, ec); > + > return 0; > } > diff --git a/src/test/test-install.c b/src/test/test-install.c > index 467970b..0c8c05b 100644 > --- a/src/test/test-install.c > +++ b/src/test/test-install.c > @@ -41,24 +41,25 @@ static void dump_changes(UnitFileChange *c, unsigned n) { > } > } > > -int main(int argc, char* argv[]) { > +static void test_install(int argc, char* argv[], EnabledContext *ec) { > Hashmap *h; > UnitFileList *p; > Iterator i; > int r; > - const char *const files[] = { "avahi-daemon.service", NULL }; > + const char *root_dir = argv[1] ?: NULL; > + const char *const files[] = { "systemd-timesyncd.service", NULL }; > const char *const files2[] = { "/home/lennart/test.service", NULL }; > UnitFileChange *changes = NULL; > unsigned n_changes = 0; > > h = hashmap_new(&string_hash_ops); > - r = unit_file_get_list(UNIT_FILE_SYSTEM, NULL, h); > + r = unit_file_get_list(UNIT_FILE_SYSTEM, root_dir, h, ec); > assert_se(r == 0); > > HASHMAP_FOREACH(p, h, i) { > UnitFileState s; > > - s = unit_file_get_state(UNIT_FILE_SYSTEM, NULL, > basename(p->path)); > + s = unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, > basename(p->path), ec); > > assert_se(p->state == s); > > @@ -71,195 +72,205 @@ int main(int argc, char* argv[]) { > > log_error("enable"); > > - r = unit_file_enable(UNIT_FILE_SYSTEM, false, NULL, (char**) files, > false, &changes, &n_changes); > + r = unit_file_enable(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files, false, &changes, &n_changes); > assert_se(r >= 0); > > log_error("enable2"); > > - r = unit_file_enable(UNIT_FILE_SYSTEM, false, NULL, (char**) files, > false, &changes, &n_changes); > + r = unit_file_enable(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files, false, &changes, &n_changes); > assert_se(r >= 0); > > dump_changes(changes, n_changes); > unit_file_changes_free(changes, n_changes); > > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == > UNIT_FILE_ENABLED); > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, files[0], > ec) == UNIT_FILE_ENABLED); > > log_error("disable"); > > changes = NULL; > n_changes = 0; > > - r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, (char**) files, > &changes, &n_changes); > + r = unit_file_disable(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files, &changes, &n_changes); > assert_se(r >= 0); > > dump_changes(changes, n_changes); > unit_file_changes_free(changes, n_changes); > > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == > UNIT_FILE_DISABLED); > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, files[0], > ec) == UNIT_FILE_DISABLED); > > log_error("mask"); > changes = NULL; > n_changes = 0; > > - r = unit_file_mask(UNIT_FILE_SYSTEM, false, NULL, (char**) files, > false, &changes, &n_changes); > + r = unit_file_mask(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files, false, &changes, &n_changes); > assert_se(r >= 0); > log_error("mask2"); > - r = unit_file_mask(UNIT_FILE_SYSTEM, false, NULL, (char**) files, > false, &changes, &n_changes); > + r = unit_file_mask(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files, false, &changes, &n_changes); > assert_se(r >= 0); > > dump_changes(changes, n_changes); > unit_file_changes_free(changes, n_changes); > > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == > UNIT_FILE_MASKED); > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, files[0], > ec) == UNIT_FILE_MASKED); > > log_error("unmask"); > changes = NULL; > n_changes = 0; > > - r = unit_file_unmask(UNIT_FILE_SYSTEM, false, NULL, (char**) files, > &changes, &n_changes); > + r = unit_file_unmask(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files, &changes, &n_changes); > assert_se(r >= 0); > log_error("unmask2"); > - r = unit_file_unmask(UNIT_FILE_SYSTEM, false, NULL, (char**) files, > &changes, &n_changes); > + r = unit_file_unmask(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files, &changes, &n_changes); > assert_se(r >= 0); > > dump_changes(changes, n_changes); > unit_file_changes_free(changes, n_changes); > > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == > UNIT_FILE_DISABLED); > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, files[0], > ec) == UNIT_FILE_DISABLED); > > log_error("mask"); > changes = NULL; > n_changes = 0; > > - r = unit_file_mask(UNIT_FILE_SYSTEM, false, NULL, (char**) files, > false, &changes, &n_changes); > + r = unit_file_mask(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files, false, &changes, &n_changes); > assert_se(r >= 0); > > dump_changes(changes, n_changes); > unit_file_changes_free(changes, n_changes); > > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == > UNIT_FILE_MASKED); > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, files[0], > ec) == UNIT_FILE_MASKED); > > log_error("disable"); > changes = NULL; > n_changes = 0; > > - r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, (char**) files, > &changes, &n_changes); > + r = unit_file_disable(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files, &changes, &n_changes); > assert_se(r >= 0); > log_error("disable2"); > - r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, (char**) files, > &changes, &n_changes); > + r = unit_file_disable(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files, &changes, &n_changes); > assert_se(r >= 0); > > dump_changes(changes, n_changes); > unit_file_changes_free(changes, n_changes); > > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == > UNIT_FILE_MASKED); > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, files[0], > ec) == UNIT_FILE_MASKED); > > log_error("umask"); > changes = NULL; > n_changes = 0; > > - r = unit_file_unmask(UNIT_FILE_SYSTEM, false, NULL, (char**) files, > &changes, &n_changes); > + r = unit_file_unmask(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files, &changes, &n_changes); > assert_se(r >= 0); > > dump_changes(changes, n_changes); > unit_file_changes_free(changes, n_changes); > > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, files[0]) == > UNIT_FILE_DISABLED); > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, files[0], > ec) == UNIT_FILE_DISABLED); > > log_error("enable files2"); > changes = NULL; > n_changes = 0; > > - r = unit_file_enable(UNIT_FILE_SYSTEM, false, NULL, (char**) files2, > false, &changes, &n_changes); > + r = unit_file_enable(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files2, false, &changes, &n_changes); > assert_se(r >= 0); > > dump_changes(changes, n_changes); > unit_file_changes_free(changes, n_changes); > > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, > basename(files2[0])) == UNIT_FILE_ENABLED); > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, > basename(files2[0]), ec) == UNIT_FILE_ENABLED); > > log_error("disable files2"); > changes = NULL; > n_changes = 0; > > - r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, (char**) > files2, &changes, &n_changes); > + r = unit_file_disable(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files2, &changes, &n_changes); > assert_se(r >= 0); > > dump_changes(changes, n_changes); > unit_file_changes_free(changes, n_changes); > > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, > basename(files2[0])) == _UNIT_FILE_STATE_INVALID); > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, > basename(files2[0]), ec) == _UNIT_FILE_STATE_INVALID); > > log_error("link files2"); > changes = NULL; > n_changes = 0; > > - r = unit_file_link(UNIT_FILE_SYSTEM, false, NULL, (char**) files2, > false, &changes, &n_changes); > + r = unit_file_link(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files2, false, &changes, &n_changes); > assert_se(r >= 0); > > dump_changes(changes, n_changes); > unit_file_changes_free(changes, n_changes); > > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, > basename(files2[0])) == UNIT_FILE_LINKED); > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, > basename(files2[0]), ec) == UNIT_FILE_LINKED); > > log_error("disable files2"); > changes = NULL; > n_changes = 0; > > - r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, (char**) > files2, &changes, &n_changes); > + r = unit_file_disable(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files2, &changes, &n_changes); > assert_se(r >= 0); > > dump_changes(changes, n_changes); > unit_file_changes_free(changes, n_changes); > > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, > basename(files2[0])) == _UNIT_FILE_STATE_INVALID); > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, > basename(files2[0]), ec) == _UNIT_FILE_STATE_INVALID); > > log_error("link files2"); > changes = NULL; > n_changes = 0; > > - r = unit_file_link(UNIT_FILE_SYSTEM, false, NULL, (char**) files2, > false, &changes, &n_changes); > + r = unit_file_link(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files2, false, &changes, &n_changes); > assert_se(r >= 0); > > dump_changes(changes, n_changes); > unit_file_changes_free(changes, n_changes); > > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, > basename(files2[0])) == UNIT_FILE_LINKED); > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, > basename(files2[0]), ec) == UNIT_FILE_LINKED); > > log_error("reenable files2"); > changes = NULL; > n_changes = 0; > > - r = unit_file_reenable(UNIT_FILE_SYSTEM, false, NULL, (char**) > files2, false, &changes, &n_changes); > + r = unit_file_reenable(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files2, false, &changes, &n_changes); > assert_se(r >= 0); > > dump_changes(changes, n_changes); > unit_file_changes_free(changes, n_changes); > > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, > basename(files2[0])) == UNIT_FILE_ENABLED); > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, > basename(files2[0]), ec) == UNIT_FILE_ENABLED); > > log_error("disable files2"); > changes = NULL; > n_changes = 0; > > - r = unit_file_disable(UNIT_FILE_SYSTEM, false, NULL, (char**) > files2, &changes, &n_changes); > + r = unit_file_disable(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files2, &changes, &n_changes); > assert_se(r >= 0); > > dump_changes(changes, n_changes); > unit_file_changes_free(changes, n_changes); > > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, > basename(files2[0])) == _UNIT_FILE_STATE_INVALID); > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, > basename(files2[0]), ec) == _UNIT_FILE_STATE_INVALID); > log_error("preset files"); > changes = NULL; > n_changes = 0; > > - r = unit_file_preset(UNIT_FILE_SYSTEM, false, NULL, (char**) files, > UNIT_FILE_PRESET_FULL, false, &changes, &n_changes); > + r = unit_file_preset(UNIT_FILE_SYSTEM, false, root_dir, (char**) > files, UNIT_FILE_PRESET_FULL, false, &changes, &n_changes); > assert_se(r >= 0); > > dump_changes(changes, n_changes); > unit_file_changes_free(changes, n_changes); > > - assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, NULL, > basename(files[0])) == UNIT_FILE_ENABLED); > + assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root_dir, > basename(files[0]), ec) == UNIT_FILE_ENABLED); > +} > + > +int main(int argc, char* argv[]) { > + EnabledContext *ec; > + > + test_install(argc, argv, NULL); > + > + ec = enabled_context_new(); > + assert(ec); > + test_install(argc, argv, ec); > > return 0; > } > diff --git a/src/test/test-manyunits.c b/src/test/test-manyunits.c > index e76f04c..f43fb78 100644 > --- a/src/test/test-manyunits.c > +++ b/src/test/test-manyunits.c > @@ -93,7 +93,7 @@ static void setup_manyunits(void) { > } > } > > -static void test_manyunits(void) { > +static void test_manyunits(EnabledContext *ec) { > time_t t0, t1; > int r = 0; > int count = 0; > @@ -105,7 +105,7 @@ static void test_manyunits(void) { > > t0 = time(NULL); > h = hashmap_new(&string_hash_ops); > - r = unit_file_get_list(UNIT_FILE_SYSTEM, root_dir, h); > + r = unit_file_get_list(UNIT_FILE_SYSTEM, root_dir, h, ec); > assert_se_cleanup(r >= 0); > HASHMAP_FOREACH(p, h, i) { > ++count; > @@ -121,11 +121,16 @@ static void test_manyunits(void) { > } > > int main(int argc, char* argv[]) { > + _cleanup_enabled_context_ EnabledContext *ec = NULL; > + > root_dir = strappenda(TEST_DIR, "/test-enabled-root"); > > setup_manyunits(); > > - test_manyunits(); > + ec = enabled_context_new(); > + assert(ec); > + > + test_manyunits(ec); > > cleanup_manyunits(); > > diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c > index 03b3e25..227ec10 100644 > --- a/src/test/test-unit-file.c > +++ b/src/test/test-unit-file.c > @@ -47,7 +47,7 @@ static int test_unit_file_get_set(void) { > h = hashmap_new(&string_hash_ops); > assert_se(h); > > - r = unit_file_get_list(UNIT_FILE_SYSTEM, NULL, h); > + r = unit_file_get_list(UNIT_FILE_SYSTEM, NULL, h, NULL); > > if (r == -EPERM || r == -EACCES) { > printf("Skipping test: unit_file_get_list: %s", > strerror(-r)); > -- > 1.9.3 > > _______________________________________________ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel