[systemd-devel] [PATCH 2/2] systemctl: add "systemctl cat"
Shawn Landden
shawn at churchofgit.com
Thu Nov 28 10:30:17 PST 2013
On Wed, Nov 27, 2013 at 11:59 PM, Zbigniew Jędrzejewski-Szmek
<zbyszek at in.waw.pl> wrote:
> 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.
GREEDY_REALLOC isn't appropriate here, (or read_full_file() which this
code is from), as in general stat is going to succeed and we are going
to read
the whole file in one run.
>
>> +
>> + 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.
explained below
>
>> + 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:
So....I had this EXACT same thing in mind when first writing this
patch, same "# %s" and all,
but I had difficulty mixing buffered and unbuffered io, so just
removed it, to see what y'all would think
of it without the filenames. Anyways, I now figured out how to make it
work, which is by using fprintf()
instead of printf().
>
> $ 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
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
More information about the systemd-devel
mailing list