From 5872ea700811bf3d2b9839812736675174a9d636 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 16 Sep 2024 03:24:19 +0900 Subject: [PATCH 1/5] network: log when no matching .network file found When an interface enters unmanaged state, there are two possibilities: - no matching .network file found, - found a matching .network with Unmanaged=yes. When a matching .network file is found, networkd logs the filename. Let's also log when no matching .network file is found. This also slightly adjust the log message when a matching .network file found. Closes #34436. --- src/network/networkd-link.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index b464ea8405..234e08680b 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1293,9 +1293,9 @@ static int link_get_network(Link *link, Network **ret) { } log_link_full(link, warn ? LOG_WARNING : LOG_DEBUG, - "found matching network '%s'%s.", - network->filename, - warn ? ", based on potentially unpredictable interface name" : ""); + "Found matching .network file%s: %s", + warn ? ", based on potentially unpredictable interface name" : "", + network->filename); if (network->unmanaged) return -ENOENT; @@ -1304,7 +1304,7 @@ static int link_get_network(Link *link, Network **ret) { return 0; } - return -ENOENT; + return log_link_debug_errno(link, SYNTHETIC_ERRNO(ENOENT), "No matching .network found."); } int link_reconfigure_impl(Link *link, bool force) { From 25da422bd1050d8cebd4dc604707edc5c6948a38 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 18 Sep 2024 02:35:28 +0900 Subject: [PATCH 2/5] network: log loaded .network and .netdev files --- src/network/netdev/netdev.c | 2 +- src/network/networkd-network.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index d37476bb71..53f74f6e8b 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -973,7 +973,7 @@ int netdev_load_one(Manager *manager, const char *filename) { if (r < 0) return r; - log_netdev_debug(netdev, "loaded \"%s\"", netdev_kind_to_string(netdev->kind)); + log_syntax(/* unit = */ NULL, LOG_DEBUG, filename, /* config_line = */ 0, /* error = */ 0, "Successfully loaded."); r = netdev_request_to_create(netdev); if (r < 0) diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 8ccf215a71..5107da4783 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -590,6 +590,7 @@ int network_load_one(Manager *manager, OrderedHashmap **networks, const char *fi return log_warning_errno(r, "%s: Failed to store configuration into hashmap: %m", filename); TAKE_PTR(network); + log_syntax(/* unit = */ NULL, LOG_DEBUG, filename, /* config_line = */ 0, /* error = */ 0, "Successfully loaded."); return 0; } From 572d031eca36e8f0bddce99201617f632936eda0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 18 Sep 2024 02:50:19 +0900 Subject: [PATCH 3/5] log: introduce log_oom_full() --- src/basic/log.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/basic/log.h b/src/basic/log.h index ac9d72202b..3859a6c41d 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -300,9 +300,10 @@ int log_emergency_level(void); #define log_dump(level, buffer) \ log_dump_internal(level, 0, PROJECT_FILE, __LINE__, __func__, buffer) -#define log_oom() log_oom_internal(LOG_ERR, PROJECT_FILE, __LINE__, __func__) -#define log_oom_debug() log_oom_internal(LOG_DEBUG, PROJECT_FILE, __LINE__, __func__) -#define log_oom_warning() log_oom_internal(LOG_WARNING, PROJECT_FILE, __LINE__, __func__) +#define log_oom_full(level) log_oom_internal(level, PROJECT_FILE, __LINE__, __func__) +#define log_oom() log_oom_full(LOG_ERR) +#define log_oom_debug() log_oom_full(LOG_DEBUG) +#define log_oom_warning() log_oom_full(LOG_WARNING) bool log_on_console(void) _pure_; From be8e4b1a878757373bbd48d791412ba222133949 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 18 Sep 2024 03:05:41 +0900 Subject: [PATCH 4/5] conf-parser: log errors in config_parse_many_files() and friends Previously, if an file cannot be opened, e.g. due to its permission, config_parse_many() or so did not log the error even if CONFIG_PARSE_WARN flag is set. This makes all error paths in these functions are logged, and the log level is controlled by the flag. Prompted by #34436. --- src/shared/conf-parser.c | 49 ++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index c2a42a1ae0..c9e5b49cd4 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -502,12 +502,12 @@ static int config_parse_many_files( _cleanup_ordered_hashmap_free_ OrderedHashmap *dropins = NULL; _cleanup_set_free_ Set *inodes = NULL; struct stat st; - int r; + int r, level = FLAGS_SET(flags, CONFIG_PARSE_WARN) ? LOG_WARNING : LOG_DEBUG; if (ret_stats_by_path) { stats_by_path = hashmap_new(&path_hash_ops_free_free); if (!stats_by_path) - return -ENOMEM; + return log_oom_full(level); } STRV_FOREACH(fn, files) { @@ -518,14 +518,14 @@ static int config_parse_many_files( if (r == -ENOENT) continue; if (r < 0) - return r; + return log_full_errno(level, r, "Failed to open %s: %m", *fn); int fd = fileno(f); r = ordered_hashmap_ensure_put(&dropins, &config_file_hash_ops_fclose, *fn, f); if (r < 0) { - assert(r != -EEXIST); - return r; + assert(r == -ENOMEM); + return log_oom_full(level); } assert(r > 0); TAKE_PTR(f); @@ -535,14 +535,14 @@ static int config_parse_many_files( _cleanup_free_ struct stat *st_dropin = new(struct stat, 1); if (!st_dropin) - return -ENOMEM; + return log_oom_full(level); if (fstat(fd, st_dropin) < 0) - return -errno; + return log_full_errno(level, errno, "Failed to stat %s: %m", *fn); r = set_ensure_consume(&inodes, &inode_hash_ops, TAKE_PTR(st_dropin)); if (r < 0) - return r; + return log_oom_full(level); } /* First read the first found main config file. */ @@ -553,11 +553,11 @@ static int config_parse_many_files( if (r == -ENOENT) continue; if (r < 0) - return r; + return log_full_errno(level, r, "Failed to open %s: %m", *fn); if (inodes) { if (fstat(fileno(f), &st) < 0) - return -errno; + return log_full_errno(level, errno, "Failed to stat %s: %m", *fn); if (set_contains(inodes, &st)) { log_debug("%s: symlink to/symlinked as drop-in, will be read later.", *fn); @@ -567,13 +567,13 @@ static int config_parse_many_files( r = config_parse(/* unit= */ NULL, *fn, f, sections, lookup, table, flags, userdata, &st); if (r < 0) - return r; + return r; /* config_parse() logs internally. */ assert(r > 0); if (ret_stats_by_path) { r = hashmap_put_stats_by_path(&stats_by_path, *fn, &st); if (r < 0) - return r; + return log_full_errno(level, r, "Failed to save stats of %s: %m", *fn); } break; @@ -586,13 +586,13 @@ static int config_parse_many_files( ORDERED_HASHMAP_FOREACH_KEY(f_dropin, path_dropin, dropins) { r = config_parse(/* unit= */ NULL, path_dropin, f_dropin, sections, lookup, table, flags, userdata, &st); if (r < 0) - return r; + return r; /* config_parse() logs internally. */ assert(r > 0); if (ret_stats_by_path) { r = hashmap_put_stats_by_path(&stats_by_path, path_dropin, &st); if (r < 0) - return r; + return log_full_errno(level, r, "Failed to save stats of %s: %m", path_dropin); } } @@ -625,11 +625,12 @@ int config_parse_many( r = conf_files_list_dropins(&files, dropin_dirname, root, conf_file_dirs); if (r < 0) - return r; + return log_full_errno(FLAGS_SET(flags, CONFIG_PARSE_WARN) ? LOG_WARNING : LOG_DEBUG, r, + "Failed to list up drop-in configs in %s: %m", dropin_dirname); r = config_parse_many_files(root, conf_files, files, sections, lookup, table, flags, userdata, ret_stats_by_path); if (r < 0) - return r; + return r; /* config_parse_many_files() logs internally. */ if (ret_dropin_files) *ret_dropin_files = TAKE_PTR(files); @@ -650,22 +651,16 @@ int config_parse_standard_file_with_dropins_full( const char* const *conf_paths = (const char* const*) CONF_PATHS_STRV(""); _cleanup_strv_free_ char **configs = NULL; - int r; + int r, level = FLAGS_SET(flags, CONFIG_PARSE_WARN) ? LOG_WARNING : LOG_DEBUG; /* Build the list of main config files */ r = strv_extend_strv_biconcat(&configs, root, conf_paths, main_file); - if (r < 0) { - if (flags & CONFIG_PARSE_WARN) - log_oom(); - return r; - } + if (r < 0) + return log_oom_full(level); _cleanup_free_ char *dropin_dirname = strjoin(main_file, ".d"); - if (!dropin_dirname) { - if (flags & CONFIG_PARSE_WARN) - log_oom(); - return -ENOMEM; - } + if (!dropin_dirname) + return log_oom_full(level); return config_parse_many( (const char* const*) configs, From a95ae2d36ac4931265e9585e0b8e4eafec5887ae Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 18 Sep 2024 03:13:47 +0900 Subject: [PATCH 5/5] conf-parser: use hashmap_ensure_put() at one more place --- src/shared/conf-parser.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index c9e5b49cd4..f862d0380b 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -465,10 +465,6 @@ int hashmap_put_stats_by_path(Hashmap **stats_by_path, const char *path, const s assert(path); assert(st); - r = hashmap_ensure_allocated(stats_by_path, &path_hash_ops_free_free); - if (r < 0) - return r; - st_copy = newdup(struct stat, st, 1); if (!st_copy) return -ENOMEM; @@ -477,7 +473,7 @@ int hashmap_put_stats_by_path(Hashmap **stats_by_path, const char *path, const s if (!path_copy) return -ENOMEM; - r = hashmap_put(*stats_by_path, path_copy, st_copy); + r = hashmap_ensure_put(stats_by_path, &path_hash_ops_free_free, path_copy, st_copy); if (r < 0) return r;