Several fixlets for issues found by Coverity (#40765)

The tool should not be considered stable, and those things we usually
place in /usr/lib/systemd, and not in $PATH.

We can move that to $PATH once we are confident it's gonna stay the way
it is.

TODO

@@ -134,17 +134,12 @@ Features:
 * oci: support "data" in any OCI descriptor, not just manifest config.
 
 * report:
-  - should the list of metrics use JSON-SEQ? or maybe be wrapped in a json
-    array (the latter might be necessary, once we sign the combination)
+  - plug "facts" into systemd-report too, i.e. stuff that is more static, such as hostnames, ssh keys and so on.
+  - pass filtering hints to services, so that they can also be applied server-side, not just client side
   - 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,6 +37,47 @@
     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>
 
@@ -54,7 +95,10 @@
         </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,6 +295,12 @@ 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,7 +101,10 @@ static OciLayerState* oci_layer_state_free(OciLayerState *st) {
         pidref_done_sigkill_wait(&st->tar_pid);
 
         if (st->temp_path) {
-                import_remove_tree(st->temp_path, st->pull ? &st->pull->userns_fd : NULL, st->pull->flags);
+                import_remove_tree(
+                                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,3 +743,105 @@ 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,3 +273,5 @@ 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_inode_on_mount(struct inode *inode, struct vfsmount *v) {
+static int validate_mount(struct vfsmount *v) {
         struct user_namespace *mount_userns, *task_userns, *p;
         unsigned task_userns_inode;
         struct task_struct *task;
@@ -124,10 +124,9 @@ 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_inode_on_mount(inode, v);
+        return validate_mount(v);
 }
 
 SEC("lsm/path_chown")

src/nsresourced/test-userns-restrict.c

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

src/report/meson.build

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

src/report/report.c

@@ -8,27 +8,45 @@
 #include "alloc-util.h"
 #include "ansi-color.h"
 #include "build.h"
+#include "chase.h"
 #include "dirent-util.h"
-#include "fd-util.h"
+#include "format-table.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_PRETTY_AUTO|SD_JSON_FORMAT_COLOR_AUTO;
+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 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 */
@@ -38,7 +56,7 @@ typedef struct Context {
 typedef struct LinkInfo {
         Context *context;
         sd_varlink *link;
-        char *metric_prefix;
+        char *name;
 } LinkInfo;
 
 static LinkInfo* link_info_free(LinkInfo *li) {
@@ -46,7 +64,7 @@ static LinkInfo* link_info_free(LinkInfo *li) {
                 return NULL;
 
         sd_varlink_close_unref(li->link);
-        free(li->metric_prefix);
+        free(li->name);
         return mfree(li);
 }
 
@@ -61,12 +79,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,
-        void,
-        trivial_hash_func,
-        trivial_compare_func,
-        LinkInfo,
-        link_info_free);
+                link_info_hash_ops,
+                void,
+                trivial_hash_func,
+                trivial_compare_func,
+                LinkInfo,
+                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;
@@ -99,15 +117,47 @@ static int metric_compare(sd_json_variant *const *a, sd_json_variant *const *b)
         return strcmp_ptr(fields_str_a, fields_str_b);
 }
 
-static inline bool metric_startswith_prefix(const char *metric_name, const char *prefix) {
+static int metrics_name_valid(const char *metric_name) {
+
+        /* 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] == '.';
 }
 
-static bool metrics_validate_one(LinkInfo *li, sd_json_variant *metric) {
+typedef enum {
+        VERDICT_INVALID,
+        VERDICT_MATCH,
+        VERDICT_MISMATCH,
+} Verdict;
+
+static Verdict metrics_verdict(LinkInfo *li, sd_json_variant *metric) {
         int r;
 
         assert(li);
@@ -122,10 +172,48 @@ static bool metrics_validate_one(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 false;
+                return VERDICT_INVALID;
         }
 
-        return metric_startswith_prefix(metric_name, li->metric_prefix);
+        /* Validate metric name is generally valid */
+        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(
@@ -142,11 +230,11 @@ static int metrics_on_query_reply(
 
         if (error_id) {
                 if (streq(error_id, SD_VARLINK_ERROR_DISCONNECTED))
-                        log_info("Varlink disconnected");
+                        log_warning("Varlink connection to '%s' disconnected prematurely, ignoring.", li->name);
                 else if (streq(error_id, SD_VARLINK_ERROR_TIMEOUT))
-                        log_info("Varlink timed out");
+                        log_warning("Varlink connection to '%s' timed out, ignoring.", li->name);
                 else
-                        log_error("Varlink error: %s", error_id);
+                        log_warning("Varlink error from '%s', ignoring: %s", li->name, error_id);
 
                 goto finish;
         }
@@ -156,10 +244,13 @@ static int metrics_on_query_reply(
                 goto finish;
         }
 
-        if (!metrics_validate_one(li, parameters)) {
+        Verdict v = metrics_verdict(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))
@@ -178,7 +269,7 @@ finish:
         return 0;
 }
 
-static int metrics_call(Context *context, const char *path) {
+static int metrics_call(Context *context, const char *name, const char *path) {
         _cleanup_(sd_varlink_unrefp) sd_varlink *vl = NULL;
         int r;
 
@@ -201,9 +292,12 @@ static int metrics_call(Context *context, const char *path) {
         if (r < 0)
                 return log_error_errno(r, "Failed to bind reply callback: %m");
 
-        r = sd_varlink_observe(vl, "io.systemd.Metrics.List", /* parameters= */ NULL);
+        const char *method = context->action == ACTION_LIST ? "io.systemd.Metrics.List" : "io.systemd.Metrics.Describe";
+        r = sd_varlink_observe(vl,
+                               method,
+                               /* parameters= */ NULL);
         if (r < 0)
-                return log_error_errno(r, "Failed to issue io.systemd.Metrics.List call: %m");
+                return log_error_errno(r, "Failed to issue %s() call: %m", method);
 
         _cleanup_(link_info_freep) LinkInfo *li = new(LinkInfo, 1);
         if (!li)
@@ -212,95 +306,346 @@ static int metrics_call(Context *context, const char *path) {
         *li = (LinkInfo) {
                 .context = context,
                 .link = sd_varlink_ref(vl),
+                .name = strdup(name),
         };
 
-        r = path_extract_filename(path, &li->metric_prefix);
-        if (r < 0)
-                return log_error_errno(r, "Failed to extract metric name from path %s: %m", path);
+        if (!li->name)
+                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();
 
+        (void) sd_varlink_set_userdata(vl, li);
+
         TAKE_PTR(li);
         return 0;
 }
 
-static int metrics_output_sorted(Context *context) {
+static int metrics_output_list(Context *context, Table **ret) {
+        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);
 
-        FOREACH_ARRAY(m, context->metrics, context->n_metrics) {
-                r = sd_json_variant_dump(
-                                *m,
-                                arg_json_format_flags | SD_JSON_FORMAT_FLUSH,
-                                stdout,
-                                /* prefix= */ NULL);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to write JSON: %m");
+        if (sd_json_format_enabled(arg_json_format_flags)) {
+                FOREACH_ARRAY(m, context->metrics, context->n_metrics) {
+                        r = sd_json_variant_dump(
+                                        *m,
+                                        arg_json_format_flags | SD_JSON_FORMAT_FLUSH,
+                                        stdout,
+                                        /* 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;
         }
 
-        if (context->n_metrics == 0)
-                log_info("No metrics collected.");
+        _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;
+        }
+
+        if (arg_legend && !sd_json_format_enabled(arg_json_format_flags)) {
+                if (table_isempty(table))
+                        printf("No metrics available.\n");
+                else
+                        printf("\n%zu metrics listed.\n", table_get_rows(table) - 1);
+        }
 
         return 0;
 }
 
-static int metrics_query(void) {
+static int parse_metrics_matches(char **matches) {
         int r;
 
-        _cleanup_free_ char *metrics_path = NULL;
-        r = runtime_directory_generic(arg_runtime_scope, "systemd/report", &metrics_path);
+        STRV_FOREACH(i, matches) {
+                r = metrics_name_valid(*i);
+                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 metrics directory path: %m");
+                return log_error_errno(r, "Failed to determine sources directory path: %m");
 
-        log_debug("Looking for reports in %s/", metrics_path);
+        log_debug("Looking for metrics in '%s'.", sources_path);
 
-        _cleanup_(context_done) Context context = {};
+        size_t m = 0;
 
-        r = sd_event_default(&context.event);
-        if (r < 0)
-                return log_error_errno(r, "Failed to get event loop: %m");
+        _cleanup_free_ DirectoryEntries *de = NULL;
+        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;
 
-        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");
-
-        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)) {
-
-                        if (!IN_SET(de->d_type, DT_SOCK, DT_UNKNOWN))
+                        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;
+
+        _cleanup_(context_done) Context context = {
+                .action = action,
+        };
+        size_t n_skipped_sources = 0;
+
+        _cleanup_free_ DirectoryEntries *de = NULL;
+        _cleanup_free_ char *sources_path = NULL;
+        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(metrics_path, de->d_name);
+                        _cleanup_free_ char *p = path_join(sources_path, d->d_name);
                         if (!p)
                                 return log_oom();
 
-                        (void) metrics_call(&context, p);
+                        (void) metrics_call(&context, d->d_name, 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_sorted(&context);
+                r = metrics_output(&context);
                 if (r < 0)
                         return r;
         }
@@ -320,7 +665,62 @@ static int metrics_query(void) {
         return 0;
 }
 
-static int help(void) {
+static int verb_list_sources(int argc, char *argv[], void *userdata) {
+        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;
 
@@ -328,19 +728,31 @@ static int help(void) {
         if (r < 0)
                 return log_oom();
 
-        printf("%s [OPTIONS...] \n\n"
-               "%sPrint metrics for all system components.%s\n\n"
+        printf("%1$s [OPTIONS...] COMMAND ...\n"
+               "\n%5$sAcquire metrics from local sources.%6$s\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"
-               "\nSee the %s for details.\n",
+               "  -j                    Equivalent to --json=pretty (on TTY) or --json=short\n"
+               "                        (otherwise)\n"
+               "\nSee the %2$s for details.\n",
                program_invocation_short_name,
-               ansi_highlight(),
+               link,
+               ansi_underline(),
                ansi_normal(),
-               link);
+               ansi_highlight(),
+               ansi_normal());
 
         return 0;
 }
@@ -348,17 +760,21 @@ static int help(void) {
 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'         },
-                { "version", no_argument,       NULL, ARG_VERSION },
-                { "user",    no_argument,       NULL, ARG_USER    },
-                { "system",  no_argument,       NULL, ARG_SYSTEM  },
-                { "json",    required_argument, NULL, ARG_JSON    },
+                { "help",      no_argument,       NULL, 'h'           },
+                { "version",   no_argument,       NULL, ARG_VERSION   },
+                { "no-pager",  no_argument,       NULL, ARG_NO_PAGER  },
+                { "no-legend", no_argument,       NULL, ARG_NO_LEGEND },
+                { "user",      no_argument,       NULL, ARG_USER      },
+                { "system",    no_argument,       NULL, ARG_SYSTEM    },
+                { "json",      required_argument, NULL, ARG_JSON      },
                 {}
         };
 
@@ -367,14 +783,22 @@ static int parse_argv(int argc, char *argv[]) {
         assert(argc >= 0);
         assert(argv);
 
-        while ((c = getopt_long(argc, argv, "h", options, NULL)) >= 0)
+        while ((c = getopt_long(argc, argv, "hj", options, NULL)) >= 0)
                 switch (c) {
                 case 'h':
-                        return help();
+                        return verb_help(/* argc= */ 0, /* argv= */ NULL, /* userdata= */ NULL);
 
                 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;
@@ -390,6 +814,10 @@ 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;
 
@@ -397,15 +825,22 @@ 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;
 
@@ -415,7 +850,7 @@ static int run(int argc, char *argv[]) {
         if (r <= 0)
                 return r;
 
-        return metrics_query();
+        return report_main(argc, argv);
 }
 
 DEFINE_MAIN_FUNCTION(run);

src/shared/format-table.c

@@ -14,6 +14,7 @@
 #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"
@@ -110,6 +111,7 @@ 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;
@@ -249,6 +251,9 @@ 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);
 }
 
@@ -363,6 +368,9 @@ 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();
         }
@@ -445,12 +453,22 @@ static TableData *table_data_new(
         d->ellipsize_percent = ellipsize_percent;
         d->uppercase = uppercase;
 
-        if (IN_SET(type, TABLE_STRV, TABLE_STRV_WRAPPED)) {
+        switch (type) {
+
+        case TABLE_STRV:
+        case TABLE_STRV_WRAPPED:
                 d->strv = strv_copy(data);
                 if (!d->strv)
                         return NULL;
-        } else
+                break;
+
+        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);
 }
@@ -1096,6 +1114,10 @@ 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);
 
@@ -1506,6 +1528,9 @@ 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:
                         ;
                 }
@@ -2071,6 +2096,18 @@ 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();
         }
@@ -2284,6 +2321,9 @@ 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;
 }
@@ -2991,6 +3031,15 @@ 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,6 +58,7 @@ 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,19 +1120,18 @@ 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 dir_fd, MStack **ret) {
         int r;
 
+        assert(ret);
+
         /* Well-known errors:
          *
          *     -ENOTUNIQ → Multiple conflicting layers for the same path defined
          *     -EBADMSG  → Bad file suffix, inode type for layer, or unrecognized entry
          */
 
-        MStack *mstack = new(MStack, 1);
+        _cleanup_(mstack_freep) MStack *mstack = new(MStack, 1);
         if (!mstack)
                 return -ENOMEM;
 
@@ -1142,9 +1141,7 @@ int mstack_load(const char *dir,
         if (r < 0)
                 return r;
 
-        if (ret)
-                *ret = TAKE_PTR(mstack);
-
+        *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,7 +56,17 @@ 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(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_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);
 
 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 name, e.g. io.systemd.Manager.unitsByTypeTotal or io.systemd.Manager.unitActiveState"),
+                SD_VARLINK_FIELD_COMMENT("Metric family 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,4 +1523,69 @@ 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

@@ -0,0 +1,37 @@
+#!/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,12 +242,3 @@ 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 {}