From: Mike Yuan Date: Fri, 13 Feb 2026 16:37:10 +0000 (+0100) Subject: core/varlink: several cleanups for metrics varlink server X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9676845efa8b3d4140ab7bb8e567d22717c66b9e;p=thirdparty%2Fsystemd.git core/varlink: several cleanups for metrics varlink server Follow-up for bb1ef2edf7d62de35291702635067ee85f09bad5 The commit introduced a new "metrics" varlink server, but for user scope stuff it is not bound anywhere. The copy-pasted "fresh" handling for deserialization is also essentially meaningless as metrics_setup_varlink_server() doesn't even report whether the varlink server is fresh (let alone that no serialization is being done at all right now). Moreover, currently the event priority is hardcoded, while event loop and associated priority assignment ought to be subject to each daemon. While fixing the mentioned issues I took the chance to restructure the existing code a bit for readability. Note that serialization for the metrics server is still missing - it will be tackled in subsequent commits. --- diff --git a/src/core/varlink.c b/src/core/varlink.c index b5b00cdf740..76beeba4346 100644 --- a/src/core/varlink.c +++ b/src/core/varlink.c @@ -9,7 +9,6 @@ #include "path-util.h" #include "pidref.h" #include "string-util.h" -#include "strv.h" #include "unit.h" #include "varlink.h" #include "varlink-dynamic-user.h" @@ -375,6 +374,8 @@ int manager_setup_varlink_server(Manager *m) { if (r < 0) return log_debug_errno(r, "Failed to allocate Varlink server: %m"); + (void) sd_varlink_server_set_description(s, "varlink-api"); + r = sd_varlink_server_add_interface_many( s, &vl_interface_io_systemd_Manager, @@ -425,26 +426,64 @@ int manager_setup_varlink_server(Manager *m) { return 1; } -static int manager_setup_varlink_metrics_server(Manager *m) { - sd_varlink_server_flags_t flags = SD_VARLINK_SERVER_INHERIT_USERDATA; - int r; - +int manager_setup_varlink_metrics_server(Manager *m) { assert(m); + sd_varlink_server_flags_t flags = SD_VARLINK_SERVER_INHERIT_USERDATA; if (MANAGER_IS_SYSTEM(m)) flags |= SD_VARLINK_SERVER_ACCOUNT_UID; - r = metrics_setup_varlink_server( - &m->metrics_varlink_server, flags, m->event, vl_method_list, vl_method_describe, m); - if (r < 0) - return r; + return metrics_setup_varlink_server(&m->metrics_varlink_server, flags, + m->event, EVENT_PRIORITY_IPC, + vl_method_list_metrics, vl_method_describe_metrics, + m); +} - return 0; +static int varlink_server_listen_many_idempotent_sentinel( + sd_varlink_server *s, + bool known_fresh, + const char *prefix, + ...) { + + va_list ap; + int r = 0; + + assert(s); + + va_start(ap, prefix); + for (const char *address; (address = va_arg(ap, const char*)); ) { + _cleanup_free_ char *p = NULL; + + if (prefix) { + p = path_join(prefix, address); + if (!p) { + r = log_oom(); + break; + } + + address = p; + } + + /* We might have got sockets through deserialization. Do not bind to them twice. */ + if (!known_fresh && varlink_server_contains_socket(s, address)) + continue; + + r = sd_varlink_server_listen_address(s, address, 0666 | SD_VARLINK_SERVER_MODE_MKDIR_0755); + if (r < 0) { + log_error_errno(r, "Failed to bind to varlink socket '%s': %m", address); + break; + } + } + va_end(ap); + + return r; } -static int manager_varlink_init_system(Manager *m) { +#define varlink_server_listen_many_idempotent(s, known_fresh, prefix, ...) \ + varlink_server_listen_many_idempotent_sentinel((s), (known_fresh), (prefix), __VA_ARGS__, NULL) + +static int manager_varlink_init_system_api(Manager *m) { int r; - _cleanup_free_ char *metrics_address = NULL; assert(m); @@ -453,38 +492,21 @@ static int manager_varlink_init_system(Manager *m) { return log_error_errno(r, "Failed to set up varlink server: %m"); bool fresh = r > 0; - r = manager_setup_varlink_metrics_server(m); - if (r < 0) - return log_error_errno(r, "Failed to set up metrics varlink server: %m"); - bool metrics_fresh = r > 0; - - r = runtime_directory_generic(m->runtime_scope, "systemd/report/io.systemd.Manager", &metrics_address); - if (r < 0) - return r; - if (!MANAGER_IS_TEST_RUN(m)) { - FOREACH_STRING(address, - "/run/systemd/userdb/io.systemd.DynamicUser", - VARLINK_PATH_MANAGED_OOM_SYSTEM, - "/run/systemd/io.systemd.Manager", - metrics_address) { - - sd_varlink_server *server = streq(address, metrics_address) ? m->metrics_varlink_server : m->varlink_server; - fresh = streq(address, metrics_address) ? metrics_fresh : fresh; - /* We might have got sockets through deserialization. Do not bind to them twice. */ - if (!fresh && varlink_server_contains_socket(server, address)) - continue; - - r = sd_varlink_server_listen_address(server, address, 0666 | SD_VARLINK_SERVER_MODE_MKDIR_0755); - if (r < 0) - return log_error_errno(r, "Failed to bind to varlink socket '%s': %m", address); - } + r = varlink_server_listen_many_idempotent( + m->varlink_server, fresh, + /* prefix = */ NULL, + "/run/systemd/io.systemd.Manager", + "/run/systemd/userdb/io.systemd.DynamicUser", + VARLINK_PATH_MANAGED_OOM_SYSTEM); + if (r < 0) + return r; } - return 1; + return 0; } -static int manager_varlink_init_user(Manager *m) { +static int manager_varlink_init_user_api(Manager *m) { int r; assert(m); @@ -497,30 +519,46 @@ static int manager_varlink_init_user(Manager *m) { return log_error_errno(r, "Failed to set up varlink server: %m"); bool fresh = r > 0; - FOREACH_STRING(a, - "systemd/io.systemd.Manager") { - _cleanup_free_ char *address = NULL; - address = path_join(m->prefix[EXEC_DIRECTORY_RUNTIME], a); - if (!address) - return -ENOMEM; - /* We might have got sockets through deserialization. Do not bind to them twice. */ - if (!fresh && varlink_server_contains_socket(m->varlink_server, address)) - continue; + r = varlink_server_listen_many_idempotent( + m->varlink_server, fresh, + m->prefix[EXEC_DIRECTORY_RUNTIME], + "systemd/io.systemd.Manager"); + if (r < 0) + return r; - r = sd_varlink_server_listen_address(m->varlink_server, address, 0666 | SD_VARLINK_SERVER_MODE_MKDIR_0755); - if (r < 0) - return log_error_errno(r, "Failed to bind to varlink socket '%s': %m", address); - } + return manager_varlink_managed_oom_connect(m); +} + +static int manager_varlink_init_metrics(Manager *m) { + int r; + + assert(m); + + if (MANAGER_IS_TEST_RUN(m)) + return 0; r = manager_setup_varlink_metrics_server(m); if (r < 0) return log_error_errno(r, "Failed to set up metrics varlink server: %m"); + bool fresh = r > 0; - return manager_varlink_managed_oom_connect(m); + return varlink_server_listen_many_idempotent( + m->metrics_varlink_server, fresh, + m->prefix[EXEC_DIRECTORY_RUNTIME], + "systemd/report/io.systemd.Manager"); } int manager_varlink_init(Manager *m) { - return MANAGER_IS_SYSTEM(m) ? manager_varlink_init_system(m) : manager_varlink_init_user(m); + int r; + + if (MANAGER_IS_SYSTEM(m)) + r = manager_varlink_init_system_api(m); + else + r = manager_varlink_init_user_api(m); + if (r < 0) + return r; + + return manager_varlink_init_metrics(m); } void manager_varlink_done(Manager *m) { @@ -534,6 +572,7 @@ void manager_varlink_done(Manager *m) { m->varlink_server = sd_varlink_server_unref(m->varlink_server); m->managed_oom_varlink = sd_varlink_close_unref(m->managed_oom_varlink); + m->metrics_varlink_server = sd_varlink_server_unref(m->metrics_varlink_server); } diff --git a/src/core/varlink.h b/src/core/varlink.h index f959cf8a670..119394386b1 100644 --- a/src/core/varlink.h +++ b/src/core/varlink.h @@ -4,6 +4,7 @@ #include "core-forward.h" int manager_setup_varlink_server(Manager *m); +int manager_setup_varlink_metrics_server(Manager *m); int manager_varlink_init(Manager *m); void manager_varlink_done(Manager *m); diff --git a/src/shared/metrics.c b/src/shared/metrics.c index 5e0b0a11588..3b8965dbdc2 100644 --- a/src/shared/metrics.c +++ b/src/shared/metrics.c @@ -11,6 +11,7 @@ int metrics_setup_varlink_server( sd_varlink_server **server, /* in and out param */ sd_varlink_server_flags_t flags, sd_event *event, + int64_t priority, sd_varlink_method_t vl_method_list_cb, sd_varlink_method_t vl_method_describe_cb, void *userdata) { @@ -45,13 +46,12 @@ int metrics_setup_varlink_server( if (r < 0) return log_debug_errno(r, "Failed to set varlink metrics server description: %m"); - r = sd_varlink_server_attach_event(s, event, SD_EVENT_PRIORITY_NORMAL); + r = sd_varlink_server_attach_event(s, event, priority); if (r < 0) return log_debug_errno(r, "Failed to attach varlink metrics server to event loop: %m"); *server = TAKE_PTR(s); - - return 0; + return 1; } static const char * const metric_family_type_table[_METRIC_FAMILY_TYPE_MAX] = { diff --git a/src/shared/metrics.h b/src/shared/metrics.h index cec0153e680..ffc28cb0a9d 100644 --- a/src/shared/metrics.h +++ b/src/shared/metrics.h @@ -31,6 +31,7 @@ int metrics_setup_varlink_server( sd_varlink_server **server, /* in and out param */ sd_varlink_server_flags_t flags, sd_event *event, + int64_t priority, sd_varlink_method_t vl_method_list_cb, sd_varlink_method_t vl_method_describe_cb, void *userdata);