TODO

@@ -134,12 +134,17 @@ Features:
 * oci: support "data" in any OCI descriptor, not just manifest config.
 
 * report:
-  - plug "facts" into systemd-report too, i.e. stuff that is more static, such as hostnames, ssh keys and so on.
+  - should the list of metrics use JSON-SEQ? or maybe be wrapped in a json
-  - pass filtering hints to services, so that they can also be applied server-side, not just client side
+    array (the latter might be necessary, once we sign the combination)
   - metrics from pid1: suppress metrics form units that are inactive and have nothing to report
+  - how to plug facts into this? i.e. hostname, ssh keys, and so on
+  - switch to daan's suggested hierarchy?
+  - enforce naming rules: a backend can only report metrics with the prefix of its service name
+  - use that to optimize sorted report generation
+  - figure out report vs. metrics
+  - systemd-report: add prefix matching of metrics
   - add "hint-suppress-zero" flag (which suppresses all metrics which are zero)
   - add "hint-object" parameter (which only queries info about certain object)
-  - make systemd-report a varlink service
 
 * implement a varlink registry service, similar to the one of the reference
   implementation, backed by /run/varlink/registry/. Then, also implement

man/systemd-report.xml

@@ -37,47 +37,6 @@
     standard output.</para>
   </refsect1>
 
-  <refsect1>
-    <title>Commands</title>
-
-    <para>The following commands are understood:</para>
-
-    <variablelist>
-      <varlistentry>
-        <term><command>metrics</command> <arg choice="opt" rep="repeat">MATCH</arg></term>
-
-        <listitem><para>Acquire a list of metrics values from all local services providing them, and write
-        them to standard output. Optionally takes one or more match expressions for filtering the metrics to
-        show. The expression either may be a literal metric family name to search for, or a prefix of one
-        (which will be matched only at dot boundaries). If multiple matches are specified as multiple
-        paramaters, any metric matching <emphasis>any</emphasis> of the specified matches are shown.</para>
-
-        <xi:include href="version-info.xml" xpointer="v260"/></listitem>
-      </varlistentry>
-
-      <varlistentry>
-        <term><command>describe-metrics</command> <arg choice="opt" rep="repeat">MATCH</arg></term>
-
-        <listitem><para>Acquire a list of metric families from all local services providing them, and write
-        them to standard output. This returns primarily static information about metrics, their data types
-        and human readable description, without values.</para>
-
-        <para>Match expressions similar to those supported by <command>metrics</command> are supported for
-        <command>describe-metrics</command>, too.</para>
-
-        <xi:include href="version-info.xml" xpointer="v260"/></listitem>
-      </varlistentry>
-
-      <varlistentry>
-        <term><command>list-sources</command></term>
-
-        <listitem><para>Show list of known metrics sources.</para>
-
-        <xi:include href="version-info.xml" xpointer="v260"/></listitem>
-      </varlistentry>
-    </variablelist>
-  </refsect1>
-
   <refsect1>
     <title>Options</title>
 
@@ -95,10 +54,7 @@
         </listitem>
       </varlistentry>
 
-      <xi:include href="standard-options.xml" xpointer="no-pager" />
       <xi:include href="standard-options.xml" xpointer="json" />
-      <xi:include href="standard-options.xml" xpointer="j" />
-      <xi:include href="standard-options.xml" xpointer="no-legend" />
       <xi:include href="standard-options.xml" xpointer="help" />
       <xi:include href="standard-options.xml" xpointer="version" />
     </variablelist>

src/basic/uid-range.c

@@ -295,12 +295,6 @@ int uid_range_load_userns_by_fd(int userns_fd, UIDRangeUsernsMode mode, UIDRange
         assert(mode < _UID_RANGE_USERNS_MODE_MAX);
         assert(ret);
 
-        r = is_our_namespace(userns_fd, NAMESPACE_USER);
-        if (r < 0)
-                return r;
-        if (r > 0)
-                return uid_range_load_userns(/* path= */ NULL, mode, ret);
-
         r = userns_enter_and_pin(userns_fd, &pidref);
         if (r < 0)
                 return r;

src/import/pull-oci.c

@@ -101,10 +101,7 @@ static OciLayerState* oci_layer_state_free(OciLayerState *st) {
         pidref_done_sigkill_wait(&st->tar_pid);
 
         if (st->temp_path) {
-                import_remove_tree(
+                import_remove_tree(st->temp_path, st->pull ? &st->pull->userns_fd : NULL, st->pull->flags);
-                                st->temp_path,
-                                st->pull ? &st->pull->userns_fd : NULL,
-                                st->pull ? st->pull->flags : 0);
                 free(st->temp_path);
         }
         free(st->final_path);

src/libsystemd/sd-json/json-util.c

@@ -743,105 +743,3 @@ int json_dispatch_access_mode(const char *name, sd_json_variant *variant, sd_jso
 
         return 0;
 }
-
-int json_variant_compare(sd_json_variant *a, sd_json_variant *b) {
-        int r;
-
-        r = CMP(!!a, !!b);
-        if (r != 0)
-                return r;
-
-        if (sd_json_variant_equal(a, b))
-                return 0;
-
-        if (sd_json_variant_is_null(a))
-                return -1;
-        if (sd_json_variant_is_null(b))
-                return 1;
-
-        if (sd_json_variant_is_string(a) &&
-            sd_json_variant_is_string(b))
-                return strcmp(sd_json_variant_string(a), sd_json_variant_string(b));
-
-        if (sd_json_variant_is_integer(a) &&
-            sd_json_variant_is_integer(b))
-                return CMP(sd_json_variant_integer(a), sd_json_variant_integer(b));
-
-        if (sd_json_variant_is_unsigned(a) &&
-            sd_json_variant_is_unsigned(b))
-                return CMP(sd_json_variant_unsigned(a), sd_json_variant_unsigned(b));
-
-        /* We cannot necessarily compare 64bit signed with unsigned, hence we go via sign checking instead */
-        if (sd_json_variant_is_number(a) && sd_json_variant_is_number(b)) {
-                if (sd_json_variant_is_negative(a) &&
-                    !sd_json_variant_is_negative(b))
-                        return -1;
-
-                if (!sd_json_variant_is_negative(a) &&
-                    sd_json_variant_is_negative(b))
-                        return 1;
-        }
-
-        if (sd_json_variant_is_real(a) &&
-            sd_json_variant_is_real(b))
-                return CMP(sd_json_variant_real(a), sd_json_variant_real(b));
-
-        if (sd_json_variant_is_boolean(a) &&
-            sd_json_variant_is_boolean(b))
-                return CMP(sd_json_variant_boolean(a), sd_json_variant_boolean(b));
-
-        if (sd_json_variant_is_array(a) &&
-            sd_json_variant_is_array(b)) {
-
-                size_t n = sd_json_variant_elements(a),
-                        m = sd_json_variant_elements(b);
-                for (size_t i = 0; i < n || i < m; i++) {
-
-                        if (i >= n)
-                                return -1;
-                        if (i >= m)
-                                return 1;
-
-                        r = json_variant_compare(
-                                        sd_json_variant_by_index(a, i),
-                                        sd_json_variant_by_index(b, i));
-                        if (r != 0)
-                                return r;
-                }
-
-                return 0;
-        }
-
-        if (sd_json_variant_is_object(a) &&
-            sd_json_variant_is_object(b)) {
-                const char *k, *lowest = NULL;
-                sd_json_variant *v;
-                int result = 0;
-
-                JSON_VARIANT_OBJECT_FOREACH(k, v, a) {
-                        if (lowest && strcmp(k, lowest) >= 0)
-                                continue;
-
-                        r = json_variant_compare(v, sd_json_variant_by_key(b, k));
-                        if (r != 0) {
-                                lowest = k;
-                                result = r;
-                        }
-                }
-
-                JSON_VARIANT_OBJECT_FOREACH(k, v, b) {
-                        if (lowest && strcmp(k, lowest) >= 0)
-                                continue;
-
-                        r = json_variant_compare(v, sd_json_variant_by_key(a, k));
-                        if (r != 0) {
-                                lowest = k;
-                                result = -r;
-                        }
-                }
-
-                return result;
-        }
-
-        return CMP(sd_json_variant_type(a), sd_json_variant_type(b));
-}

src/libsystemd/sd-json/json-util.h

@@ -273,5 +273,3 @@ int json_variant_new_fd_info(sd_json_variant **ret, int fd);
 
 char *json_underscorify(char *p);
 char *json_dashify(char *p);
-
-int json_variant_compare(sd_json_variant *a, sd_json_variant *b);

src/nsresourced/bpf/userns-restrict/userns-restrict.bpf.c

@@ -68,7 +68,7 @@ static inline struct mount *real_mount(struct vfsmount *mnt) {
         return container_of(mnt, struct mount, mnt);
 }
 
-static int validate_mount(struct vfsmount *v) {
+static int validate_inode_on_mount(struct inode *inode, struct vfsmount *v) {
         struct user_namespace *mount_userns, *task_userns, *p;
         unsigned task_userns_inode;
         struct task_struct *task;
@@ -124,9 +124,10 @@ static int validate_path(const struct path *path, int ret) {
         if (ret != 0) /* propagate earlier error */
                 return ret;
 
+        inode = path->dentry->d_inode;
         v = path->mnt;
 
-        return validate_mount(v);
+        return validate_inode_on_mount(inode, v);
 }
 
 SEC("lsm/path_chown")

src/nsresourced/test-userns-restrict.c

@@ -8,136 +8,180 @@
 
 #include "errno-util.h"
 #include "fd-util.h"
+#include "log.h"
+#include "main-func.h"
 #include "namespace-util.h"
 #include "pidref.h"
 #include "process-util.h"
 #include "rm-rf.h"
-#include "tests.h"
 #include "tmpfile-util.h"
 #include "userns-restrict.h"
 
 static int make_tmpfs_fsmount(void) {
         _cleanup_close_ int fsfd = -EBADF, mntfd = -EBADF;
 
-        fsfd = ASSERT_OK_ERRNO(fsopen("tmpfs", FSOPEN_CLOEXEC));
+        fsfd = fsopen("tmpfs", FSOPEN_CLOEXEC);
-        ASSERT_OK_ERRNO(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+        assert_se(fsfd >= 0);
+        assert_se(fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0) >= 0);
 
-        mntfd = ASSERT_OK_ERRNO(fsmount(fsfd, FSMOUNT_CLOEXEC, 0));
+        mntfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0);
+        assert_se(mntfd >= 0);
 
         return TAKE_FD(mntfd);
 }
 
-static struct userns_restrict_bpf *bpf_obj = NULL;
+static void test_works_reg(int parent_fd, const char *fname) {
-STATIC_DESTRUCTOR_REGISTER(bpf_obj, userns_restrict_bpf_freep);
+        _cleanup_close_ int fd = -EBADF;
 
-static int intro(void) {
+        fd = openat(parent_fd, fname, O_RDWR|O_CREAT|O_CLOEXEC, 0666);
-        int r;
+        assert_se(fd >= 0);
-
-        r = userns_restrict_install(/* pin= */ false, &bpf_obj);
-        if (ERRNO_IS_NOT_SUPPORTED(r))
-                return log_tests_skipped("LSM-BPF logic not supported");
-        if (ERRNO_IS_PRIVILEGE(r))
-                return log_tests_skipped("Lacking privileges");
-        ASSERT_OK(r);
-
-        return 0;
 }
 
-TEST(userns_restrict) {
+static void test_fails_reg(int parent_fd, const char *fname) {
+        errno = 0;
+        assert_se(openat(parent_fd, fname, O_RDWR|O_CREAT|O_CLOEXEC, 0666) < 0);
+        assert_se(errno == EPERM);
+}
+
+static void test_works_dir(int parent_fd, const char *fname) {
+        assert_se(mkdirat(parent_fd, fname, 0666) >= 0);
+}
+
+static void test_fails_dir(int parent_fd, const char *fname) {
+        errno = 0;
+        assert_se(mkdirat(parent_fd, fname, 0666) < 0);
+        assert_se(errno == EPERM);
+}
+
+static int run(int argc, char *argv[]) {
+        _cleanup_(userns_restrict_bpf_freep) struct userns_restrict_bpf *obj = NULL;
         _cleanup_close_ int userns_fd = -EBADF, host_fd1 = -EBADF, host_tmpfs = -EBADF, afd = -EBADF, bfd = -EBADF;
         _cleanup_(rm_rf_physical_and_freep) char *t = NULL;
         _cleanup_(pidref_done_sigkill_wait) PidRef pidref = PIDREF_NULL;
         int r;
 
-        ASSERT_OK(mkdtemp_malloc(NULL, &t));
+        log_set_max_level(LOG_DEBUG);
+        log_setup();
 
-        host_fd1 = ASSERT_OK_ERRNO(open(t, O_DIRECTORY|O_CLOEXEC));
+        r = userns_restrict_install(/* pin= */ false, &obj);
-        host_tmpfs = ASSERT_OK(make_tmpfs_fsmount());
+        if (ERRNO_IS_NOT_SUPPORTED(r)) {
-        userns_fd = ASSERT_OK(userns_acquire("0 0 1", "0 0 1", /* setgroups_deny= */ true));
+                log_notice("Skipping test, LSM-BPF logic not supported.");
+                return EXIT_TEST_SKIP;
+        }
+        if (ERRNO_IS_PRIVILEGE(r)) {
+                log_notice("Skipping test, lacking privileges.");
+                return EXIT_TEST_SKIP;
+        }
+        if (r < 0)
+                return r;
 
-        ASSERT_OK(userns_restrict_put_by_fd(
+        assert_se(mkdtemp_malloc(NULL, &t) >= 0);
-                        bpf_obj,
+
+        host_fd1 = open(t, O_DIRECTORY|O_CLOEXEC);
+        assert_se(host_fd1 >= 0);
+
+        host_tmpfs = make_tmpfs_fsmount();
+        assert_se(host_tmpfs >= 0);
+
+        userns_fd = userns_acquire("0 0 1", "0 0 1", /* setgroups_deny= */ true);
+        if (userns_fd < 0)
+                return log_error_errno(userns_fd, "Failed to make user namespace: %m");
+
+        r = userns_restrict_put_by_fd(
+                        obj,
                         userns_fd,
                         /* replace= */ true,
                         /* mount_fds= */ NULL,
-                        /* n_mount_fds= */ 0));
+                        /* n_mount_fds= */ 0);
+        if (r < 0)
+                return log_error_errno(r, "Failed to restrict user namespace: %m");
 
-        afd = ASSERT_OK_ERRNO(eventfd(0, EFD_CLOEXEC));
+        afd = eventfd(0, EFD_CLOEXEC);
-        bfd = ASSERT_OK_ERRNO(eventfd(0, EFD_CLOEXEC));
+        bfd = eventfd(0, EFD_CLOEXEC);
 
-        r = ASSERT_OK(pidref_safe_fork("(test)", FORK_DEATHSIG_SIGKILL, &pidref));
+        assert_se(afd >= 0 && bfd >= 0);
+
+        r = pidref_safe_fork("(test)", FORK_DEATHSIG_SIGKILL, &pidref);
+        assert_se(r >= 0);
         if (r == 0) {
                 _cleanup_close_ int private_tmpfs = -EBADF;
 
-                ASSERT_OK_ERRNO(setns(userns_fd, CLONE_NEWUSER));
+                assert_se(setns(userns_fd, CLONE_NEWUSER) >= 0);
-                ASSERT_OK_ERRNO(unshare(CLONE_NEWNS));
+                assert_se(unshare(CLONE_NEWNS) >= 0);
 
                 /* Allocate tmpfs locally */
                 private_tmpfs = make_tmpfs_fsmount();
 
                 /* These two host mounts should be inaccessible */
-                ASSERT_ERROR_ERRNO(openat(host_fd1, "test", O_RDWR|O_CREAT|O_CLOEXEC, 0666), EPERM);
+                test_fails_reg(host_fd1, "test");
-                ASSERT_ERROR_ERRNO(openat(host_tmpfs, "xxx", O_RDWR|O_CREAT|O_CLOEXEC, 0666), EPERM);
+                test_fails_reg(host_tmpfs, "xxx");
-                ASSERT_ERROR_ERRNO(mkdirat(host_fd1, "test2", 0666), EPERM);
+                test_fails_dir(host_fd1, "test2");
-                ASSERT_ERROR_ERRNO(mkdirat(host_tmpfs, "xxx2", 0666), EPERM);
+                test_fails_dir(host_tmpfs, "xxx2");
 
                 /* But this mount created locally should be fine */
-                safe_close(ASSERT_OK_ERRNO(openat(private_tmpfs, "yyy", O_RDWR|O_CREAT|O_CLOEXEC, 0666)));
+                test_works_reg(private_tmpfs, "yyy");
-                ASSERT_OK_ERRNO(mkdirat(private_tmpfs, "yyy2", 0666));
+                test_works_dir(private_tmpfs, "yyy2");
 
                 /* Let's sync with the parent, so that it allowlists more stuff for us */
-                ASSERT_OK_ERRNO(eventfd_write(afd, 1));
+                assert_se(eventfd_write(afd, 1) >= 0);
                 uint64_t x;
-                ASSERT_OK_ERRNO(eventfd_read(bfd, &x));
+                assert_se(eventfd_read(bfd, &x) >= 0);
 
                 /* And now we should also have access to the host tmpfs */
-                safe_close(ASSERT_OK_ERRNO(openat(host_tmpfs, "zzz", O_RDWR|O_CREAT|O_CLOEXEC, 0666)));
+                test_works_reg(host_tmpfs, "zzz");
-                safe_close(ASSERT_OK_ERRNO(openat(private_tmpfs, "aaa", O_RDWR|O_CREAT|O_CLOEXEC, 0666)));
+                test_works_reg(private_tmpfs, "aaa");
-                ASSERT_OK_ERRNO(mkdirat(host_tmpfs, "zzz2", 0666));
+                test_works_dir(host_tmpfs, "zzz2");
-                ASSERT_OK_ERRNO(mkdirat(private_tmpfs, "aaa2", 0666));
+                test_works_dir(private_tmpfs, "aaa2");
 
                 /* But this one should still fail */
-                ASSERT_ERROR_ERRNO(openat(host_fd1, "bbb", O_RDWR|O_CREAT|O_CLOEXEC, 0666), EPERM);
+                test_fails_reg(host_fd1, "bbb");
-                ASSERT_ERROR_ERRNO(mkdirat(host_fd1, "bbb2", 0666), EPERM);
+                test_fails_dir(host_fd1, "bbb2");
 
                 /* Sync again, to get more stuff allowlisted */
-                ASSERT_OK_ERRNO(eventfd_write(afd, 1));
+                assert_se(eventfd_write(afd, 1) >= 0);
-                ASSERT_OK_ERRNO(eventfd_read(bfd, &x));
+                assert_se(eventfd_read(bfd, &x) >= 0);
 
                 /* Everything should now be allowed */
-                safe_close(ASSERT_OK_ERRNO(openat(host_tmpfs, "ccc", O_RDWR|O_CREAT|O_CLOEXEC, 0666)));
+                test_works_reg(host_tmpfs, "ccc");
-                safe_close(ASSERT_OK_ERRNO(openat(host_fd1, "ddd", O_RDWR|O_CREAT|O_CLOEXEC, 0666)));
+                test_works_reg(host_fd1, "ddd");
-                safe_close(ASSERT_OK_ERRNO(openat(private_tmpfs, "eee", O_RDWR|O_CREAT|O_CLOEXEC, 0666)));
+                test_works_reg(private_tmpfs, "eee");
-                ASSERT_OK_ERRNO(mkdirat(host_tmpfs, "ccc2", 0666));
+                test_works_dir(host_tmpfs, "ccc2");
-                safe_close(ASSERT_OK_ERRNO(openat(host_fd1, "ddd2", O_RDWR|O_CREAT|O_CLOEXEC, 0666)));
+                test_works_reg(host_fd1, "ddd2");
-                ASSERT_OK_ERRNO(mkdirat(private_tmpfs, "eee2", 0666));
+                test_works_dir(private_tmpfs, "eee2");
 
                 _exit(EXIT_SUCCESS);
         }
 
         uint64_t x;
-        ASSERT_OK_ERRNO(eventfd_read(afd, &x));
+        assert_se(eventfd_read(afd, &x) >= 0);
 
-        ASSERT_OK(userns_restrict_put_by_fd(
+        r = userns_restrict_put_by_fd(
-                        bpf_obj,
+                        obj,
                         userns_fd,
                         /* replace= */ false,
                         &host_tmpfs,
-                        1));
+                        1);
+        if (r < 0)
+                return log_error_errno(r, "Failed to loosen user namespace: %m");
 
-        ASSERT_OK_ERRNO(eventfd_write(bfd, 1));
+        assert_se(eventfd_write(bfd, 1) >= 0);
-        ASSERT_OK_ERRNO(eventfd_read(afd, &x));
 
-        ASSERT_OK(userns_restrict_put_by_fd(
+        assert_se(eventfd_read(afd, &x) >= 0);
-                        bpf_obj,
+
+        r = userns_restrict_put_by_fd(
+                        obj,
                         userns_fd,
                         /* replace= */ false,
                         &host_fd1,
-                        1));
+                        1);
+        if (r < 0)
+                return log_error_errno(r, "Failed to loosen user namespace: %m");
 
-        ASSERT_OK_ERRNO(eventfd_write(bfd, 1));
+        assert_se(eventfd_write(bfd, 1) >= 0);
 
-        ASSERT_OK(pidref_wait_for_terminate_and_check("(test)", &pidref, WAIT_LOG));
+        assert_se(pidref_wait_for_terminate_and_check("(test)", &pidref, WAIT_LOG) >= 0);
+
+        return 0;
 }
 
-DEFINE_TEST_MAIN_WITH_INTRO(LOG_DEBUG, intro);
+DEFINE_MAIN_FUNCTION(run);

src/report/meson.build

@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: LGPL-2.1-or-later
 
 executables += [
-        libexec_template + {
+        executable_template + {
                 'name' : 'systemd-report',
                 'public' : true,
                 'sources' : files('report.c'),

src/report/report.c

@@ -8,45 +8,27 @@
 #include "alloc-util.h"
 #include "ansi-color.h"
 #include "build.h"
-#include "chase.h"
 #include "dirent-util.h"
-#include "format-table.h"
+#include "fd-util.h"
 #include "log.h"
 #include "main-func.h"
 #include "parse-argument.h"
 #include "path-lookup.h"
 #include "pretty-print.h"
-#include "recurse-dir.h"
 #include "runtime-scope.h"
 #include "set.h"
 #include "sort-util.h"
 #include "string-util.h"
-#include "strv.h"
 #include "time-util.h"
-#include "varlink-idl-util.h"
-#include "verbs.h"
 
 #define METRICS_MAX 1024U
 #define METRICS_LINKS_MAX 128U
 #define TIMEOUT_USEC (30 * USEC_PER_SEC) /* 30 seconds */
 
-static PagerFlags arg_pager_flags = 0;
-static bool arg_legend = true;
 static RuntimeScope arg_runtime_scope = RUNTIME_SCOPE_SYSTEM;
-static sd_json_format_flags_t arg_json_format_flags = SD_JSON_FORMAT_OFF|SD_JSON_FORMAT_PRETTY_AUTO|SD_JSON_FORMAT_COLOR_AUTO;
+static sd_json_format_flags_t arg_json_format_flags = SD_JSON_FORMAT_PRETTY_AUTO|SD_JSON_FORMAT_COLOR_AUTO;
-static char **arg_matches = NULL;
-
-STATIC_DESTRUCTOR_REGISTER(arg_matches, strv_freep);
-
-typedef enum Action {
-        ACTION_LIST,
-        ACTION_DESCRIBE,
-        _ACTION_MAX,
-        _ACTION_INVALID = -EINVAL,
-} Action;
 
 typedef struct Context {
-        Action action;
         sd_event *event;
         Set *link_infos;
         sd_json_variant **metrics;  /* Collected metrics for sorting */
@@ -56,7 +38,7 @@ typedef struct Context {
 typedef struct LinkInfo {
         Context *context;
         sd_varlink *link;
-        char *name;
+        char *metric_prefix;
 } LinkInfo;
 
 static LinkInfo* link_info_free(LinkInfo *li) {
@@ -64,7 +46,7 @@ static LinkInfo* link_info_free(LinkInfo *li) {
                 return NULL;
 
         sd_varlink_close_unref(li->link);
-        free(li->name);
+        free(li->metric_prefix);
         return mfree(li);
 }
 
@@ -79,12 +61,12 @@ static void context_done(Context *context) {
 
 DEFINE_TRIVIAL_CLEANUP_FUNC(LinkInfo*, link_info_free);
 DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(
-                link_info_hash_ops,
+        link_info_hash_ops,
-                void,
+        void,
-                trivial_hash_func,
+        trivial_hash_func,
-                trivial_compare_func,
+        trivial_compare_func,
-                LinkInfo,
+        LinkInfo,
-                link_info_free);
+        link_info_free);
 
 static int metric_compare(sd_json_variant *const *a, sd_json_variant *const *b) {
         const char *name_a, *name_b, *object_a, *object_b;
@@ -117,47 +99,15 @@ static int metric_compare(sd_json_variant *const *a, sd_json_variant *const *b)
         return strcmp_ptr(fields_str_a, fields_str_b);
 }
 
-static int metrics_name_valid(const char *metric_name) {
+static inline bool metric_startswith_prefix(const char *metric_name, const char *prefix) {
-
-        /* Validates a metrics family name. Since the prefix shall match the Varlink service name, we'll
-         * enforce Varlink interface naming rules on it. Given how close we are to Varlink let's also enforce
-         * rules on metrics names similar to those of Varlink field names. */
-
-        const char *e = strrchr(metric_name, '.');
-        if (!e)
-                return false;
-
-        _cleanup_free_ char *j = strndup(metric_name, e - metric_name);
-        if (!j)
-                return -ENOMEM;
-
-        if (!varlink_idl_interface_name_is_valid(j))
-                return false;
-
-        if (!varlink_idl_field_name_is_valid(e+1))
-                return false;
-
-        return true;
-}
-
-static bool metric_startswith_prefix(const char *metric_name, const char *prefix) {
         if (isempty(metric_name) || isempty(prefix))
                 return false;
 
-        /* NB: this checks for a *true* prefix, i.e. insists on the dot separator after the prefix. Or in
-         * other words, "foo" is not going to be considered a prefix of "foo", but of "foo.bar" it will. */
-
         const char *m = startswith(metric_name, prefix);
         return !isempty(m) && m[0] == '.';
 }
 
-typedef enum {
+static bool metrics_validate_one(LinkInfo *li, sd_json_variant *metric) {
-        VERDICT_INVALID,
-        VERDICT_MATCH,
-        VERDICT_MISMATCH,
-} Verdict;
-
-static Verdict metrics_verdict(LinkInfo *li, sd_json_variant *metric) {
         int r;
 
         assert(li);
@@ -172,48 +122,10 @@ static Verdict metrics_verdict(LinkInfo *li, sd_json_variant *metric) {
         r = sd_json_dispatch(metric, dispatch_table, SD_JSON_ALLOW_EXTENSIONS, &metric_name);
         if (r < 0) {
                 log_debug_errno(r, "Failed to get metric name, assuming name is not valid: %m");
-                return VERDICT_INVALID;
+                return false;
         }
 
-        /* Validate metric name is generally valid */
+        return metric_startswith_prefix(metric_name, li->metric_prefix);
-        r = metrics_name_valid(metric_name);
-        if (r < 0) {
-                log_debug_errno(r, "Failed to determine if '%s' is a valid metric name: %m", metric_name);
-                return VERDICT_INVALID;
-        }
-        if (!r) {
-                log_debug("Metric name '%s' is not valid, skipping.", metric_name);
-                return VERDICT_INVALID;
-        }
-
-        /* Validate metric name matches the Varlink service it was found on */
-        if (!metric_startswith_prefix(metric_name, li->name)) {
-                log_debug("Metric name '%s' does not match service name '%s', skipping.", metric_name, li->name);
-                return VERDICT_INVALID;
-        }
-
-        /* Check it against any specified matches */
-        bool matches;
-        if (strv_isempty(arg_matches))
-                matches = true;
-        else {
-                matches = false;
-
-                /* Allow exact matches or prefix matches */
-                STRV_FOREACH(i, arg_matches)
-                        if (streq(metric_name, *i) ||
-                            metric_startswith_prefix(metric_name, *i)) {
-                                matches = true;
-                                break;
-                        }
-        }
-
-        if (!matches) {
-                log_debug("Metric '%s' does not match search, ignoring.", metric_name);
-                return VERDICT_MISMATCH;
-        }
-
-        return VERDICT_MATCH;
 }
 
 static int metrics_on_query_reply(
@@ -230,11 +142,11 @@ static int metrics_on_query_reply(
 
         if (error_id) {
                 if (streq(error_id, SD_VARLINK_ERROR_DISCONNECTED))
-                        log_warning("Varlink connection to '%s' disconnected prematurely, ignoring.", li->name);
+                        log_info("Varlink disconnected");
                 else if (streq(error_id, SD_VARLINK_ERROR_TIMEOUT))
-                        log_warning("Varlink connection to '%s' timed out, ignoring.", li->name);
+                        log_info("Varlink timed out");
                 else
-                        log_warning("Varlink error from '%s', ignoring: %s", li->name, error_id);
+                        log_error("Varlink error: %s", error_id);
 
                 goto finish;
         }
@@ -244,13 +156,10 @@ static int metrics_on_query_reply(
                 goto finish;
         }
 
-        Verdict v = metrics_verdict(li, parameters);
+        if (!metrics_validate_one(li, parameters)) {
-        if (v == VERDICT_INVALID) {
                 context->n_invalid_metrics++;
                 goto finish;
         }
-        if (v == VERDICT_MISMATCH)
-                goto finish;
 
         /* Collect metrics for later sorting */
         if (!GREEDY_REALLOC(context->metrics, context->n_metrics + 1))
@@ -269,7 +178,7 @@ finish:
         return 0;
 }
 
-static int metrics_call(Context *context, const char *name, const char *path) {
+static int metrics_call(Context *context, const char *path) {
         _cleanup_(sd_varlink_unrefp) sd_varlink *vl = NULL;
         int r;
 
@@ -292,12 +201,9 @@ static int metrics_call(Context *context, const char *name, const char *path) {
         if (r < 0)
                 return log_error_errno(r, "Failed to bind reply callback: %m");
 
-        const char *method = context->action == ACTION_LIST ? "io.systemd.Metrics.List" : "io.systemd.Metrics.Describe";
+        r = sd_varlink_observe(vl, "io.systemd.Metrics.List", /* parameters= */ NULL);
-        r = sd_varlink_observe(vl,
-                               method,
-                               /* parameters= */ NULL);
         if (r < 0)
-                return log_error_errno(r, "Failed to issue %s() call: %m", method);
+                return log_error_errno(r, "Failed to issue io.systemd.Metrics.List call: %m");
 
         _cleanup_(link_info_freep) LinkInfo *li = new(LinkInfo, 1);
         if (!li)
@@ -306,346 +212,95 @@ static int metrics_call(Context *context, const char *name, const char *path) {
         *li = (LinkInfo) {
                 .context = context,
                 .link = sd_varlink_ref(vl),
-                .name = strdup(name),
         };
 
-        if (!li->name)
+        r = path_extract_filename(path, &li->metric_prefix);
-                return log_oom();
+        if (r < 0)
-
+                return log_error_errno(r, "Failed to extract metric name from path %s: %m", path);
-        if (set_ensure_put(&context->link_infos, &link_info_hash_ops, li) < 0)
-                return log_oom();
 
         (void) sd_varlink_set_userdata(vl, li);
+        if (set_ensure_put(&context->link_infos, &link_info_hash_ops, li) < 0)
+                return log_oom();
 
         TAKE_PTR(li);
         return 0;
 }
 
-static int metrics_output_list(Context *context, Table **ret) {
+static int metrics_output_sorted(Context *context) {
-        int r;
-
-        assert(context);
-
-        _cleanup_(table_unrefp) Table *table = table_new("family", "object", "fields", "value");
-        if (!table)
-                return log_oom();
-
-        table_set_ersatz_string(table, TABLE_ERSATZ_DASH);
-        table_set_sort(table, (size_t) 0, (size_t) 1, (size_t) 2, (size_t) 3);
-
-        FOREACH_ARRAY(m, context->metrics, context->n_metrics) {
-                struct {
-                        const char *name;
-                        const char *object;
-                        sd_json_variant *fields, *value;
-                } d = {};
-
-                static const sd_json_dispatch_field dispatch_table[] = {
-                        { "name",   SD_JSON_VARIANT_STRING,        sd_json_dispatch_const_string,  voffsetof(d, name),   SD_JSON_MANDATORY },
-                        { "object", SD_JSON_VARIANT_STRING,        sd_json_dispatch_const_string,  voffsetof(d, object), 0                 },
-                        { "fields", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_variant_noref, voffsetof(d, fields), 0                 },
-                        { "value",  _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_variant_noref, voffsetof(d, value),  SD_JSON_MANDATORY },
-                        {}
-                };
-
-                r = sd_json_dispatch(*m, dispatch_table, SD_JSON_ALLOW_EXTENSIONS, &d);
-                if (r < 0) {
-                        _cleanup_free_ char *t = NULL;
-                        int k = sd_json_variant_format(*m, /* flags= */ 0, &t);
-                        if (k < 0)
-                                return log_error_errno(k, "Failed to format JSON: %m");
-
-                        log_warning_errno(r, "Cannot parse metric, skipping: %s", t);
-                        continue;
-                }
-
-                r = table_add_many(
-                                table,
-                                TABLE_STRING,     d.name,
-                                TABLE_STRING,     d.object,
-                                TABLE_JSON,       d.fields,
-                                TABLE_SET_WEIGHT, 50U,
-                                TABLE_JSON,       d.value,
-                                TABLE_SET_WEIGHT, 50U);
-                if (r < 0)
-                        return table_log_add_error(r);
-        }
-
-        *ret = TAKE_PTR(table);
-        return 0;
-}
-
-static int metrics_output_describe(Context *context, Table **ret) {
-        int r;
-
-        assert(context);
-
-        _cleanup_(table_unrefp) Table *table = table_new("family", "type", "description");
-        if (!table)
-                return log_oom();
-
-        table_set_ersatz_string(table, TABLE_ERSATZ_DASH);
-        table_set_sort(table, (size_t) 0, (size_t) 1, (size_t) 2);
-
-        FOREACH_ARRAY(m, context->metrics, context->n_metrics) {
-                struct {
-                        const char *name;
-                        const char *type;
-                        const char *description;
-                } d = {};
-
-                static const sd_json_dispatch_field dispatch_table[] = {
-                        { "name",        SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, voffsetof(d, name),        SD_JSON_MANDATORY },
-                        { "type",        SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, voffsetof(d, type),        SD_JSON_MANDATORY },
-                        { "description", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, voffsetof(d, description), 0                 },
-                        {}
-                };
-
-                r = sd_json_dispatch(*m, dispatch_table, SD_JSON_ALLOW_EXTENSIONS, &d);
-                if (r < 0) {
-                        _cleanup_free_ char *t = NULL;
-                        int k = sd_json_variant_format(*m, /* flags= */ 0, &t);
-                        if (k < 0)
-                                return log_error_errno(k, "Failed to format JSON: %m");
-
-                        log_warning_errno(r, "Cannot parse metric description, skipping: %s", t);
-                        continue;
-                }
-
-                r = table_add_many(
-                                table,
-                                TABLE_STRING,     d.name,
-                                TABLE_STRING,     d.type,
-                                TABLE_STRING,     d.description,
-                                TABLE_SET_WEIGHT, 50U);
-                if (r < 0)
-                        return table_log_add_error(r);
-        }
-
-        *ret = TAKE_PTR(table);
-        return 0;
-}
-
-static int metrics_output(Context *context) {
         int r;
 
         assert(context);
 
         typesafe_qsort(context->metrics, context->n_metrics, metric_compare);
 
-        if (sd_json_format_enabled(arg_json_format_flags)) {
+        FOREACH_ARRAY(m, context->metrics, context->n_metrics) {
-                FOREACH_ARRAY(m, context->metrics, context->n_metrics) {
+                r = sd_json_variant_dump(
-                        r = sd_json_variant_dump(
+                                *m,
-                                        *m,
+                                arg_json_format_flags | SD_JSON_FORMAT_FLUSH,
-                                        arg_json_format_flags | SD_JSON_FORMAT_FLUSH,
+                                stdout,
-                                        stdout,
+                                /* prefix= */ NULL);
-                                        /* prefix= */ NULL);
-                        if (r < 0)
-                                return log_error_errno(r, "Failed to write JSON: %m");
-                }
-
-                if (context->n_metrics == 0 && arg_legend)
-                        log_info("No metrics collected.");
-
-                return 0;
-        }
-
-        _cleanup_(table_unrefp) Table *table = NULL;
-        switch(context->action) {
-
-        case ACTION_LIST:
-                r = metrics_output_list(context, &table);
-                break;
-
-        case ACTION_DESCRIBE:
-                r = metrics_output_describe(context, &table);
-                break;
-
-        default:
-                assert_not_reached();
-        }
-        if (r < 0)
-                return r;
-
-        if (!table_isempty(table) || sd_json_format_enabled(arg_json_format_flags)) {
-                r = table_print_with_pager(table, arg_json_format_flags, arg_pager_flags, arg_legend);
                 if (r < 0)
-                        return r;
+                        return log_error_errno(r, "Failed to write JSON: %m");
         }
 
-        if (arg_legend && !sd_json_format_enabled(arg_json_format_flags)) {
+        if (context->n_metrics == 0)
-                if (table_isempty(table))
+                log_info("No metrics collected.");
-                        printf("No metrics available.\n");
-                else
-                        printf("\n%zu metrics listed.\n", table_get_rows(table) - 1);
-        }
 
         return 0;
 }
 
-static int parse_metrics_matches(char **matches) {
+static int metrics_query(void) {
         int r;
 
-        STRV_FOREACH(i, matches) {
+        _cleanup_free_ char *metrics_path = NULL;
-                r = metrics_name_valid(*i);
+        r = runtime_directory_generic(arg_runtime_scope, "systemd/report", &metrics_path);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to determine if '%s' is a valid metric name: %m", *i);
-                if (!r && !varlink_idl_interface_name_is_valid(*i))
-                        return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Match is not a valid family name or prefix: %s", *i);
-
-                if (strv_extend(&arg_matches, *i) < 0)
-                        return log_oom();
-        }
-
-        strv_sort_uniq(arg_matches);
-        return 0;
-}
-
-static bool test_service_matches(const char *service) {
-        assert(service);
-
-        if (strv_isempty(arg_matches))
-                return true;
-
-        /* Only contact services whose name is either a prefix of any of the specified metrics families, or
-         * if the specified metric families are a prefix of the service.
-         *
-         * Example: if user specifies "foo" we want to match sockets "foo" and "foo.bar".
-         *          if user specifies "foo.waldo" we want to match sockets "foo" and "foo.waldo" as well as "foo.waldo.quux".
-         *
-         *          in other words: it should be fine to specify any prefix of a service name to get all metrics from all matching services.
-         *                          it should also be fine to specify a full metric name, and then go directly to the relevant services, and ask for matching metrics.
-         */
-
-        STRV_FOREACH(i, arg_matches) {
-                if (streq(service, *i))
-                        return true;
-
-                if (metric_startswith_prefix(*i, service) ||
-                    metric_startswith_prefix(service, *i))
-                        return true;
-        }
-
-        return false;
-}
-
-static int readdir_sources(char **ret_directory, DirectoryEntries **ret) {
-        int r;
-
-        assert(ret_directory);
-        assert(ret);
-
-        _cleanup_free_ char *sources_path = NULL;
-        r = runtime_directory_generic(arg_runtime_scope, "systemd/report", &sources_path);
         if (r < 0)
-                return log_error_errno(r, "Failed to determine sources directory path: %m");
+                return log_error_errno(r, "Failed to determine metrics directory path: %m");
 
-        log_debug("Looking for metrics in '%s'.", sources_path);
+        log_debug("Looking for reports in %s/", metrics_path);
 
-        size_t m = 0;
+        _cleanup_(context_done) Context context = {};
 
-        _cleanup_free_ DirectoryEntries *de = NULL;
+        r = sd_event_default(&context.event);
-        r = readdir_all_at(
-                        AT_FDCWD,
-                        sources_path,
-                        RECURSE_DIR_IGNORE_DOT|RECURSE_DIR_ENSURE_TYPE,
-                        &de);
-        if (r == -ENOENT)
-                *ret = NULL;
-        else if (r < 0)
-                return log_error_errno(r, "Failed to enumerate '%s': %m", sources_path);
-        else {
-                /* Filter out non-sockets/non-symlinks and badly named entries */
-                FOREACH_ARRAY(i, de->entries, de->n_entries) {
-                        struct dirent *d = *i;
-
-                        if (!IN_SET(d->d_type, DT_SOCK, DT_LNK))
-                                continue;
-
-                        if (!varlink_idl_interface_name_is_valid(d->d_name))
-                                continue;
-
-                        if (!test_service_matches(d->d_name))
-                                continue;
-
-                        de->entries[m++] = *i;
-                }
-
-                de->n_entries = m;
-                *ret = TAKE_PTR(de);
-        }
-
-        *ret_directory = TAKE_PTR(sources_path);
-        return m > 0;
-}
-
-static int verb_metrics(int argc, char *argv[], void *userdata) {
-        Action action;
-        int r;
-
-        assert(argc >= 1);
-        assert(argv);
-
-        /* Enable JSON-SEQ mode here, since we'll dump a large series of JSON objects */
-        arg_json_format_flags |= SD_JSON_FORMAT_SEQ;
-
-        if (streq_ptr(argv[0], "metrics"))
-                action = ACTION_LIST;
-        else {
-                assert(streq_ptr(argv[0], "describe-metrics"));
-                action = ACTION_DESCRIBE;
-        }
-
-        r = parse_metrics_matches(argv + 1);
         if (r < 0)
-                return r;
+                return log_error_errno(r, "Failed to get event loop: %m");
+
+        r = sd_event_set_signal_exit(context.event, true);
+        if (r < 0)
+                return log_error_errno(r, "Failed to enable exit on SIGINT/SIGTERM: %m");
 
-        _cleanup_(context_done) Context context = {
-                .action = action,
-        };
         size_t n_skipped_sources = 0;
+        _cleanup_closedir_ DIR *d = opendir(metrics_path);
+        if (!d) {
+                if (errno != ENOENT)
+                        return log_error_errno(errno, "Failed to open metrics directory %s: %m", metrics_path);
+        } else
+                FOREACH_DIRENT(de, d,
+                               return log_warning_errno(errno, "Failed to read %s: %m", metrics_path)) {
 
-        _cleanup_free_ DirectoryEntries *de = NULL;
+                        if (!IN_SET(de->d_type, DT_SOCK, DT_UNKNOWN))
-        _cleanup_free_ char *sources_path = NULL;
+                                continue;
-        r = readdir_sources(&sources_path, &de);
-        if (r < 0)
-                return r;
-        if (r > 0) {
-                r = sd_event_default(&context.event);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to get event loop: %m");
-
-                r = sd_event_set_signal_exit(context.event, true);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to enable exit on SIGINT/SIGTERM: %m");
-
-                FOREACH_ARRAY(i, de->entries, de->n_entries) {
-                        struct dirent *d = *i;
 
                         if (set_size(context.link_infos) >= METRICS_LINKS_MAX) {
                                 n_skipped_sources++;
                                 break;
                         }
 
-                        _cleanup_free_ char *p = path_join(sources_path, d->d_name);
+                        _cleanup_free_ char *p = path_join(metrics_path, de->d_name);
                         if (!p)
                                 return log_oom();
 
-                        (void) metrics_call(&context, d->d_name, p);
+                        (void) metrics_call(&context, p);
                 }
-        }
-
-        if (set_isempty(context.link_infos)) {
-                if (arg_legend)
-                        log_info("No metrics sources found.");
-        } else {
-                assert(context.event);
 
+        if (set_isempty(context.link_infos))
+                log_info("No metrics sources found.");
+        else {
                 r = sd_event_loop(context.event);
                 if (r < 0)
                         return log_error_errno(r, "Failed to run event loop: %m");
 
-                r = metrics_output(&context);
+                r = metrics_output_sorted(&context);
                 if (r < 0)
                         return r;
         }
@@ -665,62 +320,7 @@ static int verb_metrics(int argc, char *argv[], void *userdata) {
         return 0;
 }
 
-static int verb_list_sources(int argc, char *argv[], void *userdata) {
+static int help(void) {
-        int r;
-
-        _cleanup_(table_unrefp) Table *table = table_new("source", "address");
-        if (!table)
-                return log_oom();
-
-        _cleanup_free_ char *sources_path = NULL;
-        _cleanup_free_ DirectoryEntries *de = NULL;
-        r = readdir_sources(&sources_path, &de);
-        if (r < 0)
-                return r;
-        if (r > 0)
-                FOREACH_ARRAY(i, de->entries, de->n_entries) {
-                        struct dirent *d = *i;
-
-                        _cleanup_free_ char *k = path_join(sources_path, d->d_name);
-                        if (!k)
-                                return log_oom();
-
-                        _cleanup_free_ char *resolved = NULL;
-                        r = chase(k, /* root= */ NULL, CHASE_MUST_BE_SOCKET, &resolved, /* ret_fd= */ NULL);
-                        if (r < 0) {
-                                log_warning_errno(r, "Failed to resolve '%s', skipping: %m", k);
-                                continue;
-                        }
-
-                        _cleanup_free_ char *j = strjoin("unix:", resolved);
-                        if (!j)
-                                return log_oom();
-
-                        r = table_add_many(
-                                        table,
-                                        TABLE_STRING, d->d_name,
-                                        TABLE_STRING, j);
-                        if (r < 0)
-                                return table_log_add_error(r);
-                }
-
-        if (!table_isempty(table) || sd_json_format_enabled(arg_json_format_flags)) {
-                r = table_print_with_pager(table, arg_json_format_flags, arg_pager_flags, arg_legend);
-                if (r < 0)
-                        return r;
-        }
-
-        if (arg_legend && !sd_json_format_enabled(arg_json_format_flags)) {
-                if (table_isempty(table))
-                        printf("No sources available.\n");
-                else
-                        printf("\n%zu sources listed.\n", table_get_rows(table) - 1);
-        }
-
-        return 0;
-}
-
-static int verb_help(int argc, char *argv[], void *userdata) {
         _cleanup_free_ char *link = NULL;
         int r;
 
@@ -728,31 +328,19 @@ static int verb_help(int argc, char *argv[], void *userdata) {
         if (r < 0)
                 return log_oom();
 
-        printf("%1$s [OPTIONS...] COMMAND ...\n"
+        printf("%s [OPTIONS...] \n\n"
-               "\n%5$sAcquire metrics from local sources.%6$s\n"
+               "%sPrint metrics for all system components.%s\n\n"
-               "\n%3$sCommands:%4$s\n"
-               "  metrics [MATCH...]    Acquire list of metrics and their values\n"
-               "  describe-metrics [MATCH...]\n"
-               "                        Describe available metrics\n"
-               "  list-sources          Show list of known metrics sources\n"
-               "\n%3$sOptions:%4$s\n"
                "  -h --help             Show this help\n"
                "     --version          Show package version\n"
-               "     --no-pager         Do not pipe output into a pager\n"
-               "     --no-legend        Do not show the headers and footers\n"
                "     --user             Connect to user service manager\n"
                "     --system           Connect to system service manager (default)\n"
                "     --json=pretty|short\n"
                "                        Configure JSON output\n"
-               "  -j                    Equivalent to --json=pretty (on TTY) or --json=short\n"
+               "\nSee the %s for details.\n",
-               "                        (otherwise)\n"
-               "\nSee the %2$s for details.\n",
                program_invocation_short_name,
-               link,
-               ansi_underline(),
-               ansi_normal(),
                ansi_highlight(),
-               ansi_normal());
+               ansi_normal(),
+               link);
 
         return 0;
 }
@@ -760,21 +348,17 @@ static int verb_help(int argc, char *argv[], void *userdata) {
 static int parse_argv(int argc, char *argv[]) {
         enum {
                 ARG_VERSION = 0x100,
-                ARG_NO_PAGER,
-                ARG_NO_LEGEND,
                 ARG_USER,
                 ARG_SYSTEM,
                 ARG_JSON,
         };
 
         static const struct option options[] = {
-                { "help",      no_argument,       NULL, 'h'           },
+                { "help",    no_argument,       NULL, 'h'         },
-                { "version",   no_argument,       NULL, ARG_VERSION   },
+                { "version", no_argument,       NULL, ARG_VERSION },
-                { "no-pager",  no_argument,       NULL, ARG_NO_PAGER  },
+                { "user",    no_argument,       NULL, ARG_USER    },
-                { "no-legend", no_argument,       NULL, ARG_NO_LEGEND },
+                { "system",  no_argument,       NULL, ARG_SYSTEM  },
-                { "user",      no_argument,       NULL, ARG_USER      },
+                { "json",    required_argument, NULL, ARG_JSON    },
-                { "system",    no_argument,       NULL, ARG_SYSTEM    },
-                { "json",      required_argument, NULL, ARG_JSON      },
                 {}
         };
 
@@ -783,22 +367,14 @@ static int parse_argv(int argc, char *argv[]) {
         assert(argc >= 0);
         assert(argv);
 
-        while ((c = getopt_long(argc, argv, "hj", options, NULL)) >= 0)
+        while ((c = getopt_long(argc, argv, "h", options, NULL)) >= 0)
                 switch (c) {
                 case 'h':
-                        return verb_help(/* argc= */ 0, /* argv= */ NULL, /* userdata= */ NULL);
+                        return help();
 
                 case ARG_VERSION:
                         return version();
 
-                case ARG_NO_PAGER:
-                        arg_pager_flags |= PAGER_DISABLE;
-                        break;
-
-                case ARG_NO_LEGEND:
-                        arg_legend = false;
-                        break;
-
                 case ARG_USER:
                         arg_runtime_scope = RUNTIME_SCOPE_USER;
                         break;
@@ -814,10 +390,6 @@ static int parse_argv(int argc, char *argv[]) {
 
                         break;
 
-                case 'j':
-                        arg_json_format_flags = SD_JSON_FORMAT_PRETTY_AUTO|SD_JSON_FORMAT_COLOR_AUTO;
-                        break;
-
                 case '?':
                         return -EINVAL;
 
@@ -825,22 +397,15 @@ static int parse_argv(int argc, char *argv[]) {
                         assert_not_reached();
                 }
 
+        if (optind < argc)
+                return log_error_errno(
+                                SYNTHETIC_ERRNO(EINVAL),
+                                "%s takes no arguments.",
+                                program_invocation_short_name);
+
         return 1;
 }
 
-static int report_main(int argc, char *argv[]) {
-
-        static const Verb verbs[] = {
-                { "help",             VERB_ANY, 1,        0, verb_help         },
-                { "metrics",          VERB_ANY, VERB_ANY, 0, verb_metrics      },
-                { "describe-metrics", VERB_ANY, VERB_ANY, 0, verb_metrics      },
-                { "list-sources",     VERB_ANY, 1,        0, verb_list_sources },
-                {}
-        };
-
-        return dispatch_verb(argc, argv, verbs, NULL);
-}
-
 static int run(int argc, char *argv[]) {
         int r;
 
@@ -850,7 +415,7 @@ static int run(int argc, char *argv[]) {
         if (r <= 0)
                 return r;
 
-        return report_main(argc, argv);
+        return metrics_query();
 }
 
 DEFINE_MAIN_FUNCTION(run);

src/shared/format-table.c

@@ -14,7 +14,6 @@
 #include "glyph-util.h"
 #include "gunicode.h"
 #include "in-addr-util.h"
-#include "json-util.h"
 #include "memory-util.h"
 #include "memstream-util.h"
 #include "pager.h"
@@ -111,7 +110,6 @@ typedef struct TableData {
                 pid_t pid;
                 mode_t mode;
                 dev_t devnum;
-                sd_json_variant *json;
                 /* … add more here as we start supporting more cell data types … */
         };
 } TableData;
@@ -251,9 +249,6 @@ static TableData *table_data_free(TableData *d) {
         if (IN_SET(d->type, TABLE_STRV, TABLE_STRV_WRAPPED))
                 strv_free(d->strv);
 
-        if (d->type == TABLE_JSON)
-                sd_json_variant_unref(d->json);
-
         return mfree(d);
 }
 
@@ -368,9 +363,6 @@ static size_t table_data_size(TableDataType type, const void *data) {
         case TABLE_DEVNUM:
                 return sizeof(dev_t);
 
-        case TABLE_JSON:
-                return sizeof(sd_json_variant*);
-
         default:
                 assert_not_reached();
         }
@@ -453,22 +445,12 @@ static TableData *table_data_new(
         d->ellipsize_percent = ellipsize_percent;
         d->uppercase = uppercase;
 
-        switch (type) {
+        if (IN_SET(type, TABLE_STRV, TABLE_STRV_WRAPPED)) {
-
-        case TABLE_STRV:
-        case TABLE_STRV_WRAPPED:
                 d->strv = strv_copy(data);
                 if (!d->strv)
                         return NULL;
-                break;
+        } else
-
-        case TABLE_JSON:
-                d->json = sd_json_variant_ref((sd_json_variant*) data);
-                break;
-
-        default:
                 memcpy_safe(d->data, data, data_size);
-        }
 
         return TAKE_PTR(d);
 }
@@ -1114,10 +1096,6 @@ int table_add_many_internal(Table *t, TableDataType first_type, ...) {
                         data = &buffer.devnum;
                         break;
 
-                case TABLE_JSON:
-                        data = va_arg(ap, sd_json_variant*);
-                        break;
-
                 case TABLE_SET_MINIMUM_WIDTH: {
                         size_t w = va_arg(ap, size_t);
 
@@ -1528,9 +1506,6 @@ static int cell_data_compare(TableData *a, size_t index_a, TableData *b, size_t
 
                         return CMP(minor(a->devnum), minor(b->devnum));
 
-                case TABLE_JSON:
-                        return json_variant_compare(a->json, b->json);
-
                 default:
                         ;
                 }
@@ -2096,18 +2071,6 @@ static const char *table_data_format(
 
                 break;
 
-        case TABLE_JSON: {
-                if (!d->json)
-                        return table_ersatz_string(t);
-
-                char *p;
-                if (sd_json_variant_format(d->json, /* flags= */ 0, &p) < 0)
-                        return NULL;
-
-                d->formatted = p;
-                break;
-        }
-
         default:
                 assert_not_reached();
         }
@@ -2321,9 +2284,6 @@ static bool table_data_isempty(const TableData *d) {
         if (IN_SET(d->type, TABLE_STRV, TABLE_STRV_WRAPPED))
                 return strv_isempty(d->strv);
 
-        if (d->type == TABLE_JSON)
-                return sd_json_variant_is_null(d->json);
-
         /* Note that an empty string we do not consider empty here! */
         return false;
 }
@@ -3031,15 +2991,6 @@ static int table_data_to_json(TableData *d, sd_json_variant **ret) {
                                                   SD_JSON_BUILD_UNSIGNED(major(d->devnum)),
                                                   SD_JSON_BUILD_UNSIGNED(minor(d->devnum))));
 
-        case TABLE_JSON:
-                if (!d->json)
-                        return sd_json_variant_new_null(ret);
-
-                if (ret)
-                        *ret = sd_json_variant_ref(d->json);
-
-                return 0;
-
         case TABLE_STRING_WITH_ANSI: {
                 _cleanup_free_ char *s = strdup(d->string);
                 if (!s)

src/shared/format-table.h

@@ -58,7 +58,6 @@ typedef enum TableDataType {
         TABLE_MODE,            /* as in UNIX file mode (mode_t), in typical octal output */
         TABLE_MODE_INODE_TYPE, /* also mode_t, but displays only the inode type as string */
         TABLE_DEVNUM,          /* a dev_t, displayed in the usual major:minor way */
-        TABLE_JSON,            /* an sd_json_variant, for output formatted into text */
         _TABLE_DATA_TYPE_MAX,
 
         /* The following are not really data types, but commands for table_add_cell_many() to make changes to

src/shared/mstack.c

@@ -56,7 +56,7 @@ static void mstack_done(MStack *mstack) {
         safe_close(mstack->usr_mount_fd);
 }
 
-MStack* mstack_free(MStack *mstack) {
+MStack *mstack_free(MStack *mstack) {
         if (!mstack)
                 return NULL;
 
@@ -501,7 +501,7 @@ static bool mount_is_ro(MStackMount *m, MStackFlags flags) {
                 IN_SET(m->mount_type, MSTACK_LAYER, MSTACK_ROBIND);
 }
 
-static const char* mount_name(MStackMount *m) {
+static const char *mount_name(MStackMount *m) {
         assert(m);
 
         /* Returns some vaguely useful identifier for this layer, for showing in debug output */
@@ -1120,10 +1120,11 @@ int mstack_apply(
         return mstack_bind_mounts(&mstack, where, /* where_fd= */ -EBADF, flags, ret_root_fd);
 }
 
-int mstack_load(const char *dir, int dir_fd, MStack **ret) {
+int mstack_load(const char *dir,
-        int r;
+                int dir_fd,
+                MStack **ret) {
 
-        assert(ret);
+        int r;
 
         /* Well-known errors:
          *
@@ -1131,7 +1132,7 @@ int mstack_load(const char *dir, int dir_fd, MStack **ret) {
          *     -EBADMSG  → Bad file suffix, inode type for layer, or unrecognized entry
          */
 
-        _cleanup_(mstack_freep) MStack *mstack = new(MStack, 1);
+        MStack *mstack = new(MStack, 1);
         if (!mstack)
                 return -ENOMEM;
 
@@ -1141,7 +1142,9 @@ int mstack_load(const char *dir, int dir_fd, MStack **ret) {
         if (r < 0)
                 return r;
 
-        *ret = TAKE_PTR(mstack);
+        if (ret)
+                *ret = TAKE_PTR(mstack);
+
         return 0;
 }
 

src/shared/mstack.h

@@ -47,7 +47,7 @@ typedef struct MStack {
                 .usr_mount_fd = -EBADF,         \
         }
 
-MStack* mstack_free(MStack *mstack);
+MStack *mstack_free(MStack *mstack);
 DEFINE_TRIVIAL_CLEANUP_FUNC(MStack*, mstack_free);
 
 int mstack_load(const char *dir, int dir_fd, MStack **ret);
@@ -56,17 +56,7 @@ int mstack_make_mounts(MStack *mstack, const char *temp_mount_dir, MStackFlags f
 int mstack_bind_mounts(MStack *mstack, const char *where, int where_fd, MStackFlags flags, int *ret_root_fd);
 
 /* The four calls above in one */
-int mstack_apply(
+int mstack_apply(const char *dir, int dir_fd, const char *where, const char *temp_mount_dir, sd_varlink *mountfsd_link, int userns_fd, const ImagePolicy *image_policy, const ImageFilter *image_filter, MStackFlags flags, int *ret_root_fd);
-                const char *dir,
-                int dir_fd,
-                const char *where,
-                const char *temp_mount_dir,
-                sd_varlink *mountfsd_link,
-                int userns_fd,
-                const ImagePolicy *image_policy,
-                const ImageFilter *image_filter,
-                MStackFlags flags,
-                int *ret_root_fd);
 
 int mstack_is_read_only(MStack *mstack);
 int mstack_is_foreign_uid_owned(MStack *mstack);

src/shared/varlink-io.systemd.Metrics.c

@@ -18,15 +18,15 @@ static SD_VARLINK_DEFINE_ERROR(NoSuchMetric);
 static SD_VARLINK_DEFINE_METHOD_FULL(
                 List,
                 SD_VARLINK_REQUIRES_MORE,
-                SD_VARLINK_FIELD_COMMENT("Metric family name, e.g. io.systemd.Manager.unitsByTypeTotal or io.systemd.Manager.unitActiveState"),
+                SD_VARLINK_FIELD_COMMENT("Metric name, e.g. io.systemd.Manager.unitsByTypeTotal or io.systemd.Manager.unitActiveState"),
                 SD_VARLINK_DEFINE_OUTPUT(name, SD_VARLINK_STRING, 0),
                 /* metric value has various types depending on MetricFamilyType and actual data double/int/uint */
+                SD_VARLINK_FIELD_COMMENT("Metric value"),
+                SD_VARLINK_DEFINE_OUTPUT(value, SD_VARLINK_ANY, 0),
                 SD_VARLINK_FIELD_COMMENT("Metric object name can be unit name, process name, etc, e.g. dev-hvc0.device"),
                 SD_VARLINK_DEFINE_OUTPUT(object, SD_VARLINK_STRING, SD_VARLINK_NULLABLE),
                 SD_VARLINK_FIELD_COMMENT("Metric fields are values to differentiate between different metrics in the same metric family"),
-                SD_VARLINK_DEFINE_OUTPUT(fields, SD_VARLINK_OBJECT, SD_VARLINK_NULLABLE),
+                SD_VARLINK_DEFINE_OUTPUT(fields, SD_VARLINK_OBJECT, SD_VARLINK_NULLABLE));
-                SD_VARLINK_FIELD_COMMENT("Metric value"),
-                SD_VARLINK_DEFINE_OUTPUT(value, SD_VARLINK_ANY, 0));
 
 static SD_VARLINK_DEFINE_METHOD_FULL(
                 Describe,

src/test/test-json.c

@@ -1523,69 +1523,4 @@ TEST(access_mode) {
                                   &mm), ERANGE);
 }
 
-static void test_json_variant_compare_one(const char *a, const char *b, int expected) {
-        int r;
-
-        _cleanup_(sd_json_variant_unrefp) sd_json_variant *aa = NULL;
-        if (!isempty(a))
-                ASSERT_OK(sd_json_parse(a, /* flags= */ 0, &aa, /* reterr_line= */ NULL, /* reterr_column= */ NULL));
-
-        _cleanup_(sd_json_variant_unrefp) sd_json_variant *bb = NULL;
-        if (!isempty(b))
-                ASSERT_OK(sd_json_parse(b, /* flags= */ 0, &bb, /* reterr_line= */ NULL, /* reterr_column= */ NULL));
-
-        r = json_variant_compare(aa, bb);
-
-        log_debug("%s vs %s → %i (expected %i)", a, b, r, expected);
-
-        if (expected < 0)
-                ASSERT_LT(r, 0);
-        else if (expected > 0)
-                ASSERT_GT(r, 0);
-        else
-                ASSERT_EQ(r, 0);
-
-        r = json_variant_compare(bb, aa);
-
-        if (expected < 0)
-                ASSERT_GT(r, 0);
-        else if (expected > 0)
-                ASSERT_LT(r, 0);
-        else
-                ASSERT_EQ(r, 0);
-}
-
-TEST(json_variant_compare) {
-        test_json_variant_compare_one("null", "\"a\"", -1);
-        test_json_variant_compare_one(NULL, "\"a\"", -1);
-        test_json_variant_compare_one("0", "1", -1);
-        test_json_variant_compare_one("1", "0", 1);
-        test_json_variant_compare_one("0", "0", 0);
-        test_json_variant_compare_one("1", "1", 0);
-        test_json_variant_compare_one("1", "null", 1);
-        test_json_variant_compare_one("null", "1", -1);
-        test_json_variant_compare_one("null", "null", 0);
-        test_json_variant_compare_one("false", "true", -1);
-        test_json_variant_compare_one("true", "false", 1);
-        test_json_variant_compare_one("true", "true", 0);
-        test_json_variant_compare_one("false", "false", 0);
-        test_json_variant_compare_one("\"a\"", "\"b\"", -1);
-        test_json_variant_compare_one("\"b\"", "\"a\"", 1);
-        test_json_variant_compare_one("18446744073709551615", "0", 1);
-        test_json_variant_compare_one("0", "18446744073709551615", -1);
-        test_json_variant_compare_one("18446744073709551615", "18446744073709551615", 0);
-        test_json_variant_compare_one("-9223372036854775808", "18446744073709551615", -1);
-        test_json_variant_compare_one("18446744073709551615", "-9223372036854775808", 1);
-        test_json_variant_compare_one("1.1", "3.4", -1);
-        test_json_variant_compare_one("1", "3.4", -1);
-        test_json_variant_compare_one("[1,2]", "[1,2]", 0);
-        test_json_variant_compare_one("[1,2]", "[2,1]", -1);
-        test_json_variant_compare_one("[1,2]", "[1,2,3]", -1);
-        test_json_variant_compare_one("{}", "{\"a\":\"b\"}", -1);
-        test_json_variant_compare_one("{\"a\":\"b\"}", "{\"a\":\"b\"}", 0);
-        test_json_variant_compare_one("{\"a\":\"b\"}", "{\"b\":\"c\"}", 1);
-        test_json_variant_compare_one("{\"a\":\"b\",\"b\":\"c\"}", "{\"b\":\"c\",\"a\":\"b\"}", 0);
-        test_json_variant_compare_one("{\"a\":\"b\",\"b\":\"c\"}", "{\"a\":\"b\"}", 1);
-}
-
 DEFINE_TEST_MAIN(LOG_DEBUG);

test/units/TEST-74-AUX-UTILS.report.sh

@@ -1,37 +0,0 @@
-#!/usr/bin/env bash
-# SPDX-License-Identifier: LGPL-2.1-or-later
-set -eux
-set -o pipefail
-
-# Unset $PAGER so we don't have to use --no-pager everywhere
-export PAGER=
-
-REPORT=/usr/lib/systemd/systemd-report
-
-"$REPORT" --help
-"$REPORT" help
-"$REPORT" --version
-"$REPORT" --json=help
-
-"$REPORT" metrics
-"$REPORT" metrics -j
-"$REPORT" metrics --no-legend
-"$REPORT" describe-metrics
-"$REPORT" describe-metrics -j
-"$REPORT" describe-metrics --no-legend
-"$REPORT" list-sources
-"$REPORT" list-sources -j
-"$REPORT" list-sources --no-legend
-
-"$REPORT" metrics io
-"$REPORT" metrics io.systemd piff
-"$REPORT" metrics piff
-"$REPORT" describe-metrics io
-"$REPORT" describe-metrics io.systemd piff
-"$REPORT" describe-metrics piff
-
-# test io.systemd.Network Metrics
-varlinkctl info /run/systemd/report/io.systemd.Network
-varlinkctl list-methods /run/systemd/report/io.systemd.Network
-varlinkctl --more call /run/systemd/report/io.systemd.Network io.systemd.Metrics.List {}
-varlinkctl --more call /run/systemd/report/io.systemd.Network io.systemd.Metrics.Describe {}

test/units/TEST-74-AUX-UTILS.varlinkctl.sh

@@ -242,3 +242,12 @@ systemd-run --wait --pipe --user --machine testuser@ \
 # test io.systemd.Unit in user manager
 systemd-run --wait --pipe --user --machine testuser@ \
         varlinkctl --more call "/run/user/$testuser_uid/systemd/io.systemd.Manager" io.systemd.Unit.List '{}'
+
+# test report
+systemd-report
+
+# test io.systemd.Network Metrics
+varlinkctl info /run/systemd/report/io.systemd.Network
+varlinkctl list-methods /run/systemd/report/io.systemd.Network
+varlinkctl --more call /run/systemd/report/io.systemd.Network io.systemd.Metrics.List {}
+varlinkctl --more call /run/systemd/report/io.systemd.Network io.systemd.Metrics.Describe {}