[systemd-devel] [PATCH 2/2] systemctl: add "systemctl cat"

Zbigniew Jędrzejewski-Szmek zbyszek at in.waw.pl
Wed Nov 27 23:59:24 PST 2013


On Sat, Nov 23, 2013 at 07:52:53PM -0800, Shawn Landden wrote:
> ---
>  TODO                      |  2 --
>  src/shared/fileio.c       | 73 ++++++++++++++++++++++++++++++++++++-
>  src/shared/fileio.h       |  1 +
>  src/systemctl/systemctl.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 164 insertions(+), 3 deletions(-)
> 
> diff --git a/TODO b/TODO
> index 6ba4b31..7c6003b 100644
> --- a/TODO
> +++ b/TODO
> @@ -125,8 +125,6 @@ Features:
>  
>  * optimize the cgroup propagation bits, especially unit_get_members_mask(), cgroup_context_get_mask()
>  
> -* "systemctl cat" or "systemctl view" command or or so, that cats the backing unit file of a service, plus its drop-ins and shows them in a pager
> -
>  * rfkill,backlight: we probably should run the load tools inside of the udev rules so that the state is properly initialized by the time other software sees it
>  
>  * tmpfiles: when applying ownership to /run/log/journal, also do this for the journal fails contained in it
> diff --git a/src/shared/fileio.c b/src/shared/fileio.c
> index 733b320..ac1b409 100644
> --- a/src/shared/fileio.c
> +++ b/src/shared/fileio.c
> @@ -20,6 +20,7 @@
>  ***/
>  
>  #include <unistd.h>
> +#include <sys/sendfile.h>
>  #include "fileio.h"
>  #include "util.h"
>  #include "strv.h"
> @@ -117,6 +118,77 @@ int read_one_line_file(const char *fn, char **line) {
>          return 0;
>  }
>  
> +ssize_t sendfile_full(int out_fd, const char *fn) {
> +        _cleanup_fclose_ FILE *f;
> +        struct stat st;
> +        int r;
> +        ssize_t s;
> +
> +        size_t n, l;
> +        _cleanup_free_ char *buf = NULL;
> +
> +        assert(out_fd > 0);
> +        assert(fn);
> +
> +        f = fopen(fn, "r");
> +        if (!f)
> +                return -errno;
> +
> +        r = fstat(fileno(f), &st);
> +        if (r < 0)
> +                return -errno;
> +
> +        s = sendfile(out_fd, fileno(f), NULL, st.st_size);
> +        if (s < 0)
> +                if (errno == EINVAL || errno == ENOSYS) {
> +                        /* continue below */
> +                } else
> +                        return -errno;
> +        else
> +                return s;
> +
> +        /* sendfile() failed, fall back to read/write */
> +
> +        /* Safety check */
> +        if (st.st_size > 4*1024*1024)
> +                return -E2BIG;
> +
> +        n = st.st_size > 0 ? st.st_size : LINE_MAX;
> +        l = 0;
> +
> +        while (true) {
> +                char *t;
> +                size_t k;
> +
> +                t = realloc(buf, n);
> +                if (!t)
> +                        return -ENOMEM;
Can you convert this to GREEDY_REALLOC? It should be a bit simpler then.

> +
> +                buf = t;
> +                k = fread(buf + l, 1, n - l, f);
> +
> +                if (k <= 0) {
> +                        if (ferror(f))
> +                                return -errno;
> +
> +                        break;
> +                }
> +
> +                l += k;
> +                n *= 2;
> +
> +                /* Safety check */
> +                if (n > 4*1024*1024)
> +                        return -E2BIG;
> +        }
> +
> +        r = write(out_fd, buf, l);
> +        if (r < 0)
> +                return -errno;
> +
> +        return (ssize_t) l;
> +}
> +
>  int read_full_file(const char *fn, char **contents, size_t *size) {
>          _cleanup_fclose_ FILE *f = NULL;
>          size_t n, l;
> @@ -168,7 +240,6 @@ int read_full_file(const char *fn, char **contents, size_t *size) {
>  
>          buf[l] = 0;
>          *contents = buf;
> -        buf = NULL;
>  
>          if (size)
>                  *size = l;
> diff --git a/src/shared/fileio.h b/src/shared/fileio.h
> index 59e4150..06c2887 100644
> --- a/src/shared/fileio.h
> +++ b/src/shared/fileio.h
> @@ -31,6 +31,7 @@ int write_string_file_atomic(const char *fn, const char *line);
>  
>  int read_one_line_file(const char *fn, char **line);
>  int read_full_file(const char *fn, char **contents, size_t *size);
> +ssize_t sendfile_full(int out_fd, const char *fn);
>  
>  int parse_env_file(const char *fname, const char *separator, ...) _sentinel_;
>  int load_env_file(const char *fname, const char *separator, char ***l);
> diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
> index 6cb7a82..18d5e45 100644
> --- a/src/systemctl/systemctl.c
> +++ b/src/systemctl/systemctl.c
> @@ -3665,6 +3665,95 @@ static int show_all(
>          return 0;
>  }
>  
> +static int cat(sd_bus *bus, char **args) {
> +        int r = 0;
> +        char **name;
> +
> +        _cleanup_free_ char *unit = NULL, *n = NULL;
> +
> +        assert(bus);
> +        assert(args);
> +
> +        pager_open_if_enabled();
> +
> +        STRV_FOREACH(name, args+1) {
> +                _cleanup_free_ char *fragment_path = NULL;
> +                _cleanup_strv_free_ char **dropin_paths = NULL;
> +                sd_bus_error error;
> +                FILE *stdout;
This variable appears unused.

> +                char **path;
> +
> +                n = unit_name_mangle(*name);
> +                if (!n)
> +                        return log_oom();
> +
> +                unit = unit_dbus_path_from_name(n);
> +                if (!unit)
> +                        return log_oom();
> +
> +                if (need_daemon_reload(bus, n)) {
> +                        log_error("Unit file of %s changed on disk. Run 'systemctl%s daemon-reload'.",
> +                                n, arg_scope == UNIT_FILE_SYSTEM ? "" : " --user");
> +                        r = -ENODATA;
> +                        continue;
> +                }
> +
> +                r = sd_bus_get_property_string(
> +                                bus,
> +                                "org.freedesktop.systemd1",
> +                                unit,
> +                                "org.freedesktop.systemd1.Unit",
> +                                "FragmentPath",
> +                                &error,
> +                                &fragment_path);
> +                if (r < 0) {
> +                        log_warning("Failed to get FragmentPath: %s", bus_error_message(&error, r));
> +                        continue;
> +                }
> +
> +                if (isempty(fragment_path))
> +                        if (sd_bus_get_property_string(
> +                                        bus,
> +                                        "org.freedesktop.systemd1",
> +                                        unit,
> +                                        "org.freedesktop.systemd1.Unit",
> +                                        "SourcePath",
> +                                        &error,
> +                                        &fragment_path) < 0) {
> +                                log_warning("Failed to get SourcePath: %s", bus_error_message(&error, r));
> +                                continue;
> +                        }
> +
> +                r = sd_bus_get_property_strv(
> +                                bus,
> +                                "org.freedesktop.systemd1",
> +                                unit,
> +                                "org.freedesktop.systemd1.Unit",
> +                                "DropInPaths",
> +                                &error,
> +                                &dropin_paths);
> +                if (r < 0) {
> +                        log_warning("Failed to get DropInPaths: %s", bus_error_message(&error, r));
> +                        continue;
> +                }
> +
> +                r = sendfile_full(STDOUT_FILENO, fragment_path);
> +                if (r < 0) {
> +                        log_warning("Failed to cat %s: %s\n", fragment_path, strerror(-r));
> +                        continue;
> +                }
> +
> +                STRV_FOREACH(path, dropin_paths) {
> +                        if (r < 0) {
> +                                log_warning("Failed to cat %s: %s\n", *path, strerror(-r));
> +                                continue;
> +                        }
> +                }
> +        }
> +
> +        return r;
> +}
> +
>  static int show(sd_bus *bus, char **args) {
>          int r, ret = 0;
>          bool show_properties, show_status, new_line = false;
> @@ -4701,6 +4790,7 @@ static int systemctl_help(void) {
>                 "  status [NAME...|PID...]         Show runtime status of one or more units\n"
>                 "  show [NAME...|JOB...]           Show properties of one or more\n"
>                 "                                  units/jobs or the manager\n"
> +               "  cat [NAME...]                   Show files and drop-ins of one or more units\n"
>                 "  set-property [NAME] [ASSIGNMENT...]\n"
>                 "                                  Sets one or more properties of a unit\n"
>                 "  help [NAME...|PID...]           Show manual for one or more units\n"
> @@ -5678,6 +5768,7 @@ static int systemctl_main(sd_bus *bus, int argc, char *argv[], int bus_error) {
>                  { "check",                 MORE,  2, check_unit_active },
>                  { "is-failed",             MORE,  2, check_unit_failed },
>                  { "show",                  MORE,  1, show              },
> +                { "cat",                   MORE,  2, cat               },
>                  { "status",                MORE,  1, show              },
>                  { "help",                  MORE,  2, show              },
>                  { "snapshot",              LESS,  2, snapshot          },
Seems to work nicely, but I have two issues:
1. the name — it just seems to generic. Maybe something like 'show-fragment'? I don't like
'cat' and 'view' because when I hear those verbs I cannot infer the action they represent.

2. when multiple units are given as arguments, they are really concatenated together, like
the cat command would. But maybe it'd be nicer to add the file name in a comment:

$ systemctl cat dbus.service dbus.socket
# /usr/lib/systemd/system/dbus.service                            <-------------- filename here
[Unit]
Description=D-Bus System Message Bus
Requires=dbus.socket
After=syslog.target

[Service]
ExecStart=/bin/dbus-daemon --system --address=systemd: --nofork --nopidfile --systemd-activation
ExecReload=/bin/dbus-send --print-reply --system --type=method_call --dest=org.freedesktop.DBus / org.freedeskt
OOMScoreAdjust=-900
                                                                  <-------------- empty line here
# /usr/lib/systemd/system/dbus.socket                             <-------------- filename here
[Unit]
Description=D-Bus System Message Bus Socket

[Socket]
ListenStream=/var/run/dbus/system_bus_socket

I think that this will be more useful both for multiple units, and for drop-in files.
Otherwise it all runs together, and I don't see how that could be useful.

Zbyszek


More information about the systemd-devel mailing list