machinectl: split OS field in two; print ip addresses (#4058)

This splits the OS field in two : one for the distribution name
and one for the the version id.
Dashes are written for missing fields.
This also prints ip addresses of known machines. The `--max-addresses`
option specifies how much ip addresses we want to see. The default is 1.
When more than one address is written for a machine, a `,` follows it.
If there are more ips than `--max-addresses`, `...` follows the last
address.

4 files changed, 141 insertions, 31 deletions

diff --git a/TODO b/TODO
index 0ce9a1fd30..88bf48d0d9 100644
--- a/TODO
+++ b/TODO
@@ -642,8 +642,6 @@ Features:
     is used
 
 * machined:
-  - "machinectl list" should probably show columns for OS version and IP
-    addresses
   - add an API so that libvirt-lxc can inform us about network interfaces being
     removed or added to an existing machine
   - "machinectl migrate" or similar to copy a container from or to a
diff --git a/man/machinectl.xml b/man/machinectl.xml
index 597a5cc583..7056fd4204 100644
--- a/man/machinectl.xml
+++ b/man/machinectl.xml
@@ -285,6 +285,20 @@
         name passed.</para></listitem>
       </varlistentry>
 
+      <varlistentry>
+        <term><option>--max-addresses=</option></term>
+
+        <listitem><para>When used with the <option>list-machines</option>
+        command, limits the number of ip addresses output for every machine.
+        Defaults to 1. All addresses can be requested with <literal>all</literal>
+        as argument to <option>--max-addresses</option> . If the argument to
+        <option>--max-addresses</option> is less than the actual number
+        of addresses,<literal>...</literal>follows the last address.
+        If multiple addresses are to be written for a given machine, every
+        address except the first one is on a new line and is followed by
+        <literal>,</literal> if another address will be output afterwards. </para></listitem>
+      </varlistentry>
+
       <xi:include href="user-system-options.xml" xpointer="host" />
       <xi:include href="user-system-options.xml" xpointer="machine" />
 
diff --git a/src/basic/string-util.h b/src/basic/string-util.h
index b75aba63c2..d029d538bd 100644
--- a/src/basic/string-util.h
+++ b/src/basic/string-util.h
@@ -70,6 +70,10 @@ static inline const char *empty_to_null(const char *p) {
         return isempty(p) ? NULL : p;
 }
 
+static inline const char *strdash_if_empty(const char *str) {
+        return isempty(str) ? "-" : str;
+}
+
 static inline char *startswith(const char *s, const char *prefix) {
         size_t l;
 
diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c
index 4acce4bea7..74e1a349bc 100644
--- a/src/machine/machinectl.c
+++ b/src/machine/machinectl.c
@@ -60,6 +60,9 @@
 #include "util.h"
 #include "verbs.h"
 #include "web-util.h"
+#include "stdio-util.h"
+
+#define ALL_IP_ADDRESSES -1
 
 static char **arg_property = NULL;
 static bool arg_all = false;
@@ -82,6 +85,9 @@ static ImportVerify arg_verify = IMPORT_VERIFY_SIGNATURE;
 static const char* arg_format = NULL;
 static const char *arg_uid = NULL;
 static char **arg_setenv = NULL;
+static int arg_addrs = 1;
+
+static int print_addresses(sd_bus *bus, const char *name, int, const char *pr1, const char *pr2, int n_addr);
 
 static void polkit_agent_open_if_enabled(void) {
 
@@ -110,6 +116,7 @@ typedef struct MachineInfo {
         const char *class;
         const char *service;
         char *os;
+        char *version_id;
 } MachineInfo;
 
 static int compare_machine_info(const void *a, const void *b) {
@@ -124,20 +131,27 @@ static void clean_machine_info(MachineInfo *machines, size_t n_machines) {
         if (!machines || n_machines == 0)
                 return;
 
-        for (i = 0; i < n_machines; i++)
+        for (i = 0; i < n_machines; i++) {
                 free(machines[i].os);
+                free(machines[i].version_id);
+        }
         free(machines);
 }
 
-static int get_os_name(sd_bus *bus, const char *name, char **out) {
+static int get_os_release_property(sd_bus *bus, const char *name, const char *query, ...) {
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
-        const char *k, *v, *os;
-        char *str;
+        const char *k, *v, *iter, **query_res = NULL;
+        size_t count = 0, awaited_args = 0;
+        va_list ap;
         int r;
 
         assert(bus);
         assert(name);
-        assert(out);
+        assert(query);
+
+        NULSTR_FOREACH(iter, query)
+                awaited_args++;
+        query_res = newa0(const char *, awaited_args);
 
         r = sd_bus_call_method(bus,
                 "org.freedesktop.machine1",
@@ -153,8 +167,14 @@ static int get_os_name(sd_bus *bus, const char *name, char **out) {
                 return bus_log_parse_error(r);
 
         while ((r = sd_bus_message_read(reply, "{ss}", &k, &v)) > 0) {
-                if (streq(k, "PRETTY_NAME"))
-                        os = v;
+                count = 0;
+                NULSTR_FOREACH(iter, query) {
+                        if (streq(k, iter)) {
+                                query_res[count] = v;
+                                break;
+                        }
+                        count++;
+                }
         }
         if (r < 0)
                 return bus_log_parse_error(r);
@@ -163,10 +183,22 @@ static int get_os_name(sd_bus *bus, const char *name, char **out) {
         if (r < 0)
                 return bus_log_parse_error(r);
 
-        str = strdup(os);
-        if (!str)
-                return log_oom();
-        *out = str;
+        va_start(ap, query);
+        for (count = 0; count < awaited_args; count++) {
+                char *val, **out;
+
+                out = va_arg(ap, char **);
+                assert(out);
+                if (query_res[count]) {
+                        val = strdup(query_res[count]);
+                        if (!val) {
+                                va_end(ap);
+                                return log_oom();
+                        }
+                        *out = val;
+                }
+        }
+        va_end(ap);
 
         return 0;
 }
@@ -174,9 +206,10 @@ static int get_os_name(sd_bus *bus, const char *name, char **out) {
 static int list_machines(int argc, char *argv[], void *userdata) {
 
         size_t max_name = strlen("MACHINE"), max_class = strlen("CLASS"),
-               max_service = strlen("SERVICE"), max_os = strlen("OS");
+               max_service = strlen("SERVICE"), max_os = strlen("OS"), max_version_id = strlen("VERSION");
         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
+        _cleanup_free_ char *prefix = NULL;
         MachineInfo *machines = NULL;
         const char *name, *class, *service, *object;
         size_t n_machines = 0, n_allocated = 0, j;
@@ -215,7 +248,11 @@ static int list_machines(int argc, char *argv[], void *userdata) {
                 }
 
                 machines[n_machines].os = NULL;
-                r = get_os_name(bus, name, &machines[n_machines].os);
+                machines[n_machines].version_id = NULL;
+                r = get_os_release_property(bus, name,
+                                "ID\0" "VERSION_ID\0",
+                                &machines[n_machines].os,
+                                &machines[n_machines].version_id);
                 if (r < 0)
                         goto out;
 
@@ -235,10 +272,14 @@ static int list_machines(int argc, char *argv[], void *userdata) {
                 if (l > max_service)
                         max_service = l;
 
-                l = machines[n_machines].os ? strlen(machines[n_machines].os) : 0;
+                l = machines[n_machines].os ? strlen(machines[n_machines].os) : 1;
                 if (l > max_os)
                         max_os = l;
 
+                l = machines[n_machines].version_id ? strlen(machines[n_machines].version_id) : 1;
+                if (l > max_version_id)
+                        max_version_id = l;
+
                 n_machines++;
         }
         if (r < 0) {
@@ -254,19 +295,40 @@ static int list_machines(int argc, char *argv[], void *userdata) {
 
         qsort_safe(machines, n_machines, sizeof(MachineInfo), compare_machine_info);
 
+        /* Allocate for prefix max characters for all fields + spaces between them + strlen(",\n") */
+        r = asprintf(&prefix, "%-*s",
+                        (int) (max_name +
+                        max_class +
+                        max_service +
+                        max_os +
+                        max_version_id + 5 + strlen(",\n")),
+                        ",\n");
+        if (r < 0) {
+                r = log_oom();
+                goto out;
+        }
+
         if (arg_legend && n_machines > 0)
-                printf("%-*s %-*s %-*s %-*s\n",
+                printf("%-*s %-*s %-*s %-*s %-*s %s\n",
                        (int) max_name, "MACHINE",
                        (int) max_class, "CLASS",
                        (int) max_service, "SERVICE",
-                       (int) max_os, "OS");
+                       (int) max_os, "OS",
+                       (int) max_version_id, "VERSION",
+                       "ADDRESSES");
 
-        for (j = 0; j < n_machines; j++)
-                printf("%-*s %-*s %-*s %-*s\n",
+        for (j = 0; j < n_machines; j++) {
+                printf("%-*s %-*s %-*s %-*s %-*s ",
                        (int) max_name, machines[j].name,
                        (int) max_class, machines[j].class,
-                       (int) max_service, machines[j].service,
-                       (int) max_os, machines[j].os ? machines[j].os : "");
+                       (int) max_service, strdash_if_empty(machines[j].service),
+                       (int) max_os, strdash_if_empty(machines[j].os),
+                       (int) max_version_id, strdash_if_empty(machines[j].version_id));
+
+                r = print_addresses(bus, machines[j].name, 0, "", prefix, arg_addrs);
+                if (r == -ENOSYS)
+                        printf("-\n");
+        }
 
         if (arg_legend && n_machines > 0)
                 printf("\n%zu machines listed.\n", n_machines);
@@ -467,8 +529,10 @@ static int show_unit_cgroup(sd_bus *bus, const char *unit, pid_t leader) {
         return 0;
 }
 
-static int print_addresses(sd_bus *bus, const char *name, int ifi, const char *prefix, const char *prefix2) {
+static int print_addresses(sd_bus *bus, const char *name, int ifi, const char *prefix, const char *prefix2, int n_addr) {
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
+        _cleanup_free_ char *addresses = NULL;
+        bool truncate = false;
         int r;
 
         assert(bus);
@@ -487,6 +551,11 @@ static int print_addresses(sd_bus *bus, const char *name, int ifi, const char *p
         if (r < 0)
                 return r;
 
+        addresses = strdup(prefix);
+        if (!addresses)
+                return log_oom();
+        prefix = "";
+
         r = sd_bus_message_enter_container(reply, 'a', "(iay)");
         if (r < 0)
                 return bus_log_parse_error(r);
@@ -495,7 +564,7 @@ static int print_addresses(sd_bus *bus, const char *name, int ifi, const char *p
                 int family;
                 const void *a;
                 size_t sz;
-                char buffer[MAX(INET6_ADDRSTRLEN, INET_ADDRSTRLEN)];
+                char buf_ifi[DECIMAL_STR_MAX(int) + 2], buffer[MAX(INET6_ADDRSTRLEN, INET_ADDRSTRLEN)];
 
                 r = sd_bus_message_read(reply, "i", &family);
                 if (r < 0)
@@ -505,11 +574,16 @@ static int print_addresses(sd_bus *bus, const char *name, int ifi, const char *p
                 if (r < 0)
                         return bus_log_parse_error(r);
 
-                fputs(prefix, stdout);
-                fputs(inet_ntop(family, a, buffer, sizeof(buffer)), stdout);
-                if (family == AF_INET6 && ifi > 0)
-                        printf("%%%i", ifi);
-                fputc('\n', stdout);
+                if (n_addr != 0) {
+                        if (family == AF_INET6 && ifi > 0)
+                                xsprintf(buf_ifi, "%%%i", ifi);
+                        else
+                                strcpy(buf_ifi, "");
+
+                        if(!strextend(&addresses, prefix, inet_ntop(family, a, buffer, sizeof(buffer)), buf_ifi, NULL))
+                                return log_oom();
+                } else
+                        truncate = true;
 
                 r = sd_bus_message_exit_container(reply);
                 if (r < 0)
@@ -517,6 +591,9 @@ static int print_addresses(sd_bus *bus, const char *name, int ifi, const char *p
 
                 if (prefix != prefix2)
                         prefix = prefix2;
+
+                if (n_addr > 0)
+                        n_addr -= 1;
         }
         if (r < 0)
                 return bus_log_parse_error(r);
@@ -525,6 +602,7 @@ static int print_addresses(sd_bus *bus, const char *name, int ifi, const char *p
         if (r < 0)
                 return bus_log_parse_error(r);
 
+        fprintf(stdout, "%s%s\n", addresses, truncate ? "..." : "");
         return 0;
 }
 
@@ -536,7 +614,7 @@ static int print_os_release(sd_bus *bus, const char *name, const char *prefix) {
         assert(name);
         assert(prefix);
 
-        r = get_os_name(bus, name, &pretty);
+        r = get_os_release_property(bus, name, "PRETTY_NAME\0", &pretty, NULL);
         if (r < 0)
                 return r;
 
@@ -644,7 +722,8 @@ static void print_machine_status_info(sd_bus *bus, MachineStatusInfo *i) {
 
         print_addresses(bus, i->name, ifi,
                        "\t Address: ",
-                       "\t          ");
+                       "\n\t          ",
+                       ALL_IP_ADDRESSES);
 
         print_os_release(bus, i->name, "\t      OS: ");
 
@@ -2546,6 +2625,7 @@ static int help(int argc, char *argv[], void *userdata) {
                "     --read-only              Create read-only bind mount\n"
                "     --mkdir                  Create directory before bind mounting, if missing\n"
                "  -n --lines=INTEGER          Number of journal entries to show\n"
+               "     --max-addresses=INTEGER  Number of internet addresses to show at most\n"
                "  -o --output=STRING          Change journal output mode (short,\n"
                "                              short-monotonic, verbose, export, json,\n"
                "                              json-pretty, json-sse, cat)\n"
@@ -2610,6 +2690,7 @@ static int parse_argv(int argc, char *argv[]) {
                 ARG_FORCE,
                 ARG_FORMAT,
                 ARG_UID,
+                ARG_NUMBER_IPS,
         };
 
         static const struct option options[] = {
@@ -2636,6 +2717,7 @@ static int parse_argv(int argc, char *argv[]) {
                 { "format",          required_argument, NULL, ARG_FORMAT          },
                 { "uid",             required_argument, NULL, ARG_UID             },
                 { "setenv",          required_argument, NULL, 'E'                 },
+                { "max-addresses",   required_argument, NULL, ARG_NUMBER_IPS      },
                 {}
         };
 
@@ -2826,6 +2908,18 @@ static int parse_argv(int argc, char *argv[]) {
                                 return log_oom();
                         break;
 
+                case ARG_NUMBER_IPS:
+                        if (streq(optarg, "all"))
+                                arg_addrs = ALL_IP_ADDRESSES;
+                        else if (safe_atoi(optarg, &arg_addrs) < 0) {
+                                log_error("Invalid number of IPs");
+                                return -EINVAL;
+                        } else if (arg_addrs < 0) {
+                                log_error("Number of IPs cannot be negative");
+                                return -EINVAL;
+                        }
+                        break;
+
                 case '?':
                         return -EINVAL;