From 070a43ced463e9c578150a00f2390366ffc89a62 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 8 Jul 2024 13:38:09 +0200 Subject: [PATCH 1/5] mount-tool: use lowercase table column names The "systemd-mount" tool is the one outlier in our codebase to specify upper case column names. And it's quite pointless given that our table output logic uppercases this anyway on output. Hence, let's fix that. (This would be a compat break, if we'd support JSON output of this table, but we do not currently. JSON fields use the literal column name after all.) --- src/mount/mount-tool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index 8ac08e95b8..a0ca17f0f0 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -1424,7 +1424,7 @@ static int list_devices(void) { if (r < 0) return log_error_errno(r, "Failed to add property match: %m"); - table = table_new("NODE", "PATH", "MODEL", "WWN", "FSTYPE", "LABEL", "UUID"); + table = table_new("node", "path", "model", "wwn", "fstype", "label", "uuid"); if (!table) return log_oom(); From 66f4f49c8ff9d12e95914d55b936b1638e68b2f4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 8 Jul 2024 13:44:12 +0200 Subject: [PATCH 2/5] mount-tool: add support for a --json= output mode Let's add a JSON output mode, like we have it for so many of our tools. --- man/systemd-mount.xml | 1 + src/mount/mount-tool.c | 20 ++++++++++++-------- test/units/TEST-74-AUX-UTILS.mount.sh | 1 + 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/man/systemd-mount.xml b/man/systemd-mount.xml index 558fd03fc1..05357b06b1 100644 --- a/man/systemd-mount.xml +++ b/man/systemd-mount.xml @@ -121,6 +121,7 @@ + diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index a0ca17f0f0..f127168c60 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -74,6 +74,7 @@ static gid_t arg_gid = GID_INVALID; static bool arg_fsck = true; static bool arg_aggressive_gc = false; static bool arg_tmpfs = false; +static sd_json_format_flags_t arg_json_format_flags = SD_JSON_FORMAT_OFF; STATIC_DESTRUCTOR_REGISTER(arg_mount_what, freep); STATIC_DESTRUCTOR_REGISTER(arg_mount_where, freep); @@ -128,6 +129,7 @@ static int help(void) { " -l --full Do not ellipsize output\n" " --no-ask-password Do not prompt for password\n" " -q --quiet Suppress information messages during runtime\n" + " --json=pretty|short|off Generate JSON output\n" " --user Run as user unit\n" " -H --host=[USER@]HOST Operate on remote host\n" " -M --machine=CONTAINER Operate on local container\n" @@ -176,6 +178,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_AUTOMOUNT_PROPERTY, ARG_BIND_DEVICE, ARG_LIST, + ARG_JSON, }; static const struct option options[] = { @@ -207,6 +210,7 @@ static int parse_argv(int argc, char *argv[]) { { "unmount", no_argument, NULL, 'u' }, /* Compat spelling */ { "collect", no_argument, NULL, 'G' }, { "tmpfs", no_argument, NULL, 'T' }, + { "json", required_argument, NULL, ARG_JSON }, {}, }; @@ -361,6 +365,13 @@ static int parse_argv(int argc, char *argv[]) { arg_tmpfs = true; break; + case ARG_JSON: + r = parse_json_argument(optarg, &arg_json_format_flags); + if (r <= 0) + return r; + + break; + case '?': return -EINVAL; @@ -1435,7 +1446,6 @@ static int list_devices(void) { if (r < 0) return log_error_errno(r, "Failed to set sort index: %m"); - table_set_header(table, arg_legend); table_set_ersatz_string(table, TABLE_ERSATZ_DASH); FOREACH_DEVICE(e, d) { @@ -1479,13 +1489,7 @@ static int list_devices(void) { } } - pager_open(arg_pager_flags); - - r = table_print(table, NULL); - if (r < 0) - return table_log_print_error(r); - - return 0; + return table_print_with_pager(table, arg_json_format_flags, arg_pager_flags, arg_legend); } static int run(int argc, char* argv[]) { diff --git a/test/units/TEST-74-AUX-UTILS.mount.sh b/test/units/TEST-74-AUX-UTILS.mount.sh index 14253c358d..d440a7c667 100755 --- a/test/units/TEST-74-AUX-UTILS.mount.sh +++ b/test/units/TEST-74-AUX-UTILS.mount.sh @@ -23,6 +23,7 @@ systemd-mount --list --full systemd-mount --list --no-legend systemd-mount --list --no-pager systemd-mount --list --quiet +systemd-mount --list --json=pretty # tmpfs mkdir -p "$WORK_DIR/mnt/foo/bar" From 3d41dfe4474162414e1a9d7e15bc6496a2403b13 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 8 Jul 2024 14:37:58 +0200 Subject: [PATCH 3/5] mount-tool: use the usual coloring in --help text --- src/mount/mount-tool.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index f127168c60..1c11cdd5f8 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -120,7 +120,7 @@ static int help(void) { "systemd-mount [OPTIONS...] --tmpfs [NAME] WHERE\n" "systemd-mount [OPTIONS...] --list\n" "%s [OPTIONS...] %sWHAT|WHERE...\n\n" - "Establish a mount or auto-mount point transiently.\n\n" + "%sEstablish a mount or auto-mount point transiently.%s\n\n" " -h --help Show this help\n" " --version Show package version\n" " --no-block Do not wait until operation finished\n" @@ -152,6 +152,8 @@ static int help(void) { "\nSee the %s for details.\n", program_invocation_short_name, streq(program_invocation_short_name, "systemd-umount") ? "" : "--umount ", + ansi_highlight(), + ansi_normal(), link); return 0; From 5b37c4cf86db20762af9fe23224bec313a35ca43 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 8 Jul 2024 14:38:22 +0200 Subject: [PATCH 4/5] mount-tool: show diskseq in --list output Let's expose the diskseq a bit more prominently. --- src/mount/mount-tool.c | 18 +++++++++++++++++- test/units/TEST-74-AUX-UTILS.mount.sh | 2 +- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/mount/mount-tool.c b/src/mount/mount-tool.c index 1c11cdd5f8..d43db8ffd1 100644 --- a/src/mount/mount-tool.c +++ b/src/mount/mount-tool.c @@ -1412,6 +1412,7 @@ static int discover_device(void) { static int list_devices(void) { enum { COLUMN_NODE, + COLUMN_DISKSEQ, COLUMN_PATH, COLUMN_MODEL, COLUMN_WWN, @@ -1437,7 +1438,7 @@ static int list_devices(void) { if (r < 0) return log_error_errno(r, "Failed to add property match: %m"); - table = table_new("node", "path", "model", "wwn", "fstype", "label", "uuid"); + table = table_new("node", "diskseq", "path", "model", "wwn", "fstype", "label", "uuid"); if (!table) return log_oom(); @@ -1460,6 +1461,21 @@ static int list_devices(void) { (void) sd_device_get_devname(d, &x); break; + case COLUMN_DISKSEQ: { + uint64_t ds; + + r = sd_device_get_diskseq(d, &ds); + if (r < 0) { + log_debug_errno(r, "Failed to get diskseq of block device, ignoring: %m"); + r = table_add_cell(table, NULL, TABLE_EMPTY, NULL); + } else + r = table_add_cell(table, NULL, TABLE_UINT64, &ds); + if (r < 0) + return table_log_add_error(r); + + continue; + } + case COLUMN_PATH: (void) sd_device_get_property_value(d, "ID_PATH", &x); break; diff --git a/test/units/TEST-74-AUX-UTILS.mount.sh b/test/units/TEST-74-AUX-UTILS.mount.sh index d440a7c667..89a391d48e 100755 --- a/test/units/TEST-74-AUX-UTILS.mount.sh +++ b/test/units/TEST-74-AUX-UTILS.mount.sh @@ -146,7 +146,7 @@ test -e /run/media/system/simple.img/foo.bar # systemd-mount --list and systemd-umount require the loopback block device is initialized by udevd. udevadm settle --timeout 30 assert_in "/dev/loop.* ext4 +sd-mount-test" "$(systemd-mount --list --full)" -LOOP_AUTO=$(systemd-mount --list --full --no-legend | awk '$6 == "sd-mount-test" { print $1 }') +LOOP_AUTO=$(systemd-mount --list --full --no-legend | awk '$7 == "sd-mount-test" { print $1 }') LOOP_AUTO_DEVPATH=$(udevadm info --query property --property DEVPATH --value "$LOOP_AUTO") systemd-umount "$WORK_DIR/simple.img" # Wait for 'change' uevent for the device with DISK_MEDIA_CHANGE=1. From c4ecc0231f26a2e4735edcdd9bcfe0d4fa211131 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 8 Jul 2024 14:46:21 +0200 Subject: [PATCH 5/5] update TODO --- TODO | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/TODO b/TODO index aa69ab8693..5e50ea309b 100644 --- a/TODO +++ b/TODO @@ -130,6 +130,22 @@ Deprecations and removals: Features: +* go through all uses of table_new() in our codebase, and make sure we support + all three of: + 1. --no-legend properly + 2. --json= properly + 3. --no-pager properly + +* go through all --help texts in our codebases, and make sure: + 1. the one sentence description of the tool is highlighted via ANSI how we + usually do it + 2. If more than one or two commands are supported (as opposed to switches), + separate commands + switches from each other, using underlined --help sections. + 3. If there are many switches, consider adding additional --help sections. + +* go through our codebase, and convert "vertical tables" (i.e. things such as + "systemctl status") to use table_new_vertical() for output + * logind: when logging in use new tmpfs quota support to configure quota on /tmp/ + /dev/shm/. But do so only in case of tmpfs, because otherwise quota is persistent and any persistent settings mean we don#t have to reapply them.