[PATCH 0/7] btrfs-progs: implement json output for subvolume subcommands

[PATCH 0/7] btrfs-progs: implement json output for subvolume subcommands

[Christoph Heiss]

JSON as output format is already implement for some of the commands in
btrfs-progs, which is a very useful feature to have for e.g. scripting.

This series adds the same for `btrfs subvolume list`, `btrfs subvolume
get-default` and `btrfs subvolume show`.

#1-#3 are some small preparatory fixes & refactors, #4 introduces the
`struct rowspec` containing all fields needed by the series.

The actual implementation of the JSON output is mostly pretty
straight-forward, only cmd_subvolume_show() needed some refactoring
first.

I probably will go about implementing the same for more commands, but
wanted to get this out now to get some feedback.

Christoph Heiss (7):
  btrfs-progs: common: document `time-long` output format
  btrfs-progs: subvol show: remove duplicated quotas error check
  btrfs-progs: subvol show: factor out text printing to own function
  btrfs-progs: subvol: introduce rowspec definition for json output
  btrfs-progs: subvol list: implement json format output
  btrfs-progs: subvol get-default: implement json format output
  btrfs-progs: subvol show: implement json format output

 cmds/subvolume-list.c  |  91 ++++++++++-
 cmds/subvolume.c       | 345 +++++++++++++++++++++++++++++------------
 cmds/subvolume.h       |  19 +++
 common/format-output.h |   2 +
 4 files changed, 357 insertions(+), 100 deletions(-)
 create mode 100644 cmds/subvolume.h

--
2.41.0

[PATCH 1/7] btrfs-progs: common: document `time-long` output format

[Christoph Heiss]

Seems this was forgotten; rectify that.

Signed-off-by: Christoph Heiss <christoph@c8h4.io>
---
 common/format-output.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/common/format-output.h b/common/format-output.h
index 95e2a117..ab12a7f1 100644
--- a/common/format-output.h
+++ b/common/format-output.h
@@ -28,6 +28,8 @@ struct rowspec {
 	 *   (values: va_args)
 	 * - uuid: format UUID as text
 	 *   (value: u8 *uuid)
+	 * - time-long: pretty print timestamp, including timezone
+	 *   (values: time_t)
 	 * - list: print list opening bracket [
 	 *   (values printed separately)
 	 * - map:  start a new group, opens {
-- 
2.41.0

[PATCH 2/7] btrfs-progs: subvol show: remove duplicated quotas error check

[Christoph Heiss]

The exact same check is repeated here, with the second being dead code.
Keep the second instance, as that informs the user what is happening.

Signed-off-by: Christoph Heiss <christoph@c8h4.io>
---
 cmds/subvolume.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/cmds/subvolume.c b/cmds/subvolume.c
index 0691157c..a5423759 100644
--- a/cmds/subvolume.c
+++ b/cmds/subvolume.c
@@ -1466,11 +1466,6 @@ static int cmd_subvolume_show(const struct cmd_struct *cmd, int argc, char **arg
 	btrfs_util_destroy_subvolume_iterator(iter);
 
 	ret = btrfs_qgroup_query(fd, subvol.id, &stats);
-	if (ret == -ENOTTY) {
-		/* Quotas not enabled */
-		ret = 0;
-		goto out;
-	}
 	if (ret == -ENOTTY) {
 		/* Quota information not available, not fatal */
 		pr_verbose(LOG_DEFAULT, "\tQuota group:\t\tn/a\n");
-- 
2.41.0

[PATCH 3/7] btrfs-progs: subvol show: factor out text printing to own function

[Christoph Heiss]

Implements JSON-formatted output for the `subvolume list` command using
the `--format json` global option, much like it is implemented for other
commands.

Signed-off-by: Christoph Heiss <christoph@c8h4.io>
---
 cmds/subvolume.c | 190 ++++++++++++++++++++++++++---------------------
 1 file changed, 107 insertions(+), 83 deletions(-)

diff --git a/cmds/subvolume.c b/cmds/subvolume.c
index a5423759..65cff24b 100644
--- a/cmds/subvolume.c
+++ b/cmds/subvolume.c
@@ -1232,6 +1232,105 @@ static int cmd_subvolume_find_new(const struct cmd_struct *cmd, int argc, char *
 }
 static DEFINE_SIMPLE_COMMAND(subvolume_find_new, "find-new");
 
+static void print_subvolume_show_text(const struct btrfs_util_subvolume_info *subvol,
+				      const char *subvol_path, const char *subvol_name)
+{
+	char tstr[256];
+	char uuidparse[BTRFS_UUID_UNPARSED_SIZE];
+
+	/* Warn if it's a read-write subvolume with received_uuid */
+	if (!uuid_is_null(subvol->received_uuid) &&
+	    !(subvol->flags & BTRFS_ROOT_SUBVOL_RDONLY)) {
+		warning("the subvolume is read-write and has received_uuid set,\n"
+			"\t don't use it for incremental send. Please see section\n"
+			"\t 'SUBVOLUME FLAGS' in manual page btrfs-subvolume for\n"
+			"\t further information.");
+	}
+
+	/* print the info */
+	pr_verbose(LOG_DEFAULT, "%s\n",
+		   subvol->id == BTRFS_FS_TREE_OBJECTID ? "/" : subvol_path);
+	pr_verbose(LOG_DEFAULT, "\tName: \t\t\t%s\n", subvol_name);
+
+	if (uuid_is_null(subvol->uuid))
+		strcpy(uuidparse, "-");
+	else
+		uuid_unparse(subvol->uuid, uuidparse);
+	pr_verbose(LOG_DEFAULT, "\tUUID: \t\t\t%s\n", uuidparse);
+
+	if (uuid_is_null(subvol->parent_uuid))
+		strcpy(uuidparse, "-");
+	else
+		uuid_unparse(subvol->parent_uuid, uuidparse);
+	pr_verbose(LOG_DEFAULT, "\tParent UUID: \t\t%s\n", uuidparse);
+
+	if (uuid_is_null(subvol->received_uuid))
+		strcpy(uuidparse, "-");
+	else
+		uuid_unparse(subvol->received_uuid, uuidparse);
+	pr_verbose(LOG_DEFAULT, "\tReceived UUID: \t\t%s\n", uuidparse);
+
+	if (subvol->otime.tv_sec) {
+		struct tm tm;
+
+		localtime_r(&subvol->otime.tv_sec, &tm);
+		strftime(tstr, 256, "%Y-%m-%d %X %z", &tm);
+	} else
+		strcpy(tstr, "-");
+	pr_verbose(LOG_DEFAULT, "\tCreation time: \t\t%s\n", tstr);
+
+	pr_verbose(LOG_DEFAULT, "\tSubvolume ID: \t\t%" PRIu64 "\n", subvol->id);
+	pr_verbose(LOG_DEFAULT, "\tGeneration: \t\t%" PRIu64 "\n", subvol->generation);
+	pr_verbose(LOG_DEFAULT, "\tGen at creation: \t%" PRIu64 "\n", subvol->otransid);
+	pr_verbose(LOG_DEFAULT, "\tParent ID: \t\t%" PRIu64 "\n", subvol->parent_id);
+	pr_verbose(LOG_DEFAULT, "\tTop level ID: \t\t%" PRIu64 "\n", subvol->parent_id);
+
+	if (subvol->flags & BTRFS_ROOT_SUBVOL_RDONLY)
+		pr_verbose(LOG_DEFAULT, "\tFlags: \t\t\treadonly\n");
+	else
+		pr_verbose(LOG_DEFAULT, "\tFlags: \t\t\t-\n");
+
+	pr_verbose(LOG_DEFAULT, "\tSend transid: \t\t%" PRIu64 "\n", subvol->stransid);
+	pr_verbose(LOG_DEFAULT, "\tSend time: \t\t%s\n", tstr);
+	if (subvol->stime.tv_sec) {
+		struct tm tm;
+
+		localtime_r(&subvol->stime.tv_sec, &tm);
+		strftime(tstr, 256, "%Y-%m-%d %X %z", &tm);
+	} else {
+		strcpy(tstr, "-");
+	}
+	pr_verbose(LOG_DEFAULT, "\tReceive transid: \t%" PRIu64 "\n", subvol->rtransid);
+	if (subvol->rtime.tv_sec) {
+		struct tm tm;
+
+		localtime_r(&subvol->rtime.tv_sec, &tm);
+		strftime(tstr, 256, "%Y-%m-%d %X %z", &tm);
+	} else {
+		strcpy(tstr, "-");
+	}
+	pr_verbose(LOG_DEFAULT, "\tReceive time: \t\t%s\n", tstr);
+}
+
+static void print_subvolume_show_quota_text(const struct btrfs_util_subvolume_info *subvol,
+					    const struct btrfs_qgroup_stats *stats,
+					    unsigned int unit_mode)
+{
+	pr_verbose(LOG_DEFAULT, "\tQuota group:\t\t0/%" PRIu64 "\n", subvol->id);
+	fflush(stdout);
+
+	pr_verbose(LOG_DEFAULT, "\t  Limit referenced:\t%s\n",
+			stats->limit.max_referenced == 0 ? "-" :
+			pretty_size_mode(stats->limit.max_referenced, unit_mode));
+	pr_verbose(LOG_DEFAULT, "\t  Limit exclusive:\t%s\n",
+			stats->limit.max_exclusive == 0 ? "-" :
+			pretty_size_mode(stats->limit.max_exclusive, unit_mode));
+	pr_verbose(LOG_DEFAULT, "\t  Usage referenced:\t%s\n",
+			pretty_size_mode(stats->info.referenced, unit_mode));
+	pr_verbose(LOG_DEFAULT, "\t  Usage exclusive:\t%s\n",
+			pretty_size_mode(stats->info.exclusive, unit_mode));
+}
+
 static const char * const cmd_subvolume_show_usage[] = {
 	"btrfs subvolume show [options] <path>",
 	"Show more information about the subvolume (UUIDs, generations, times, snapshots)",
@@ -1247,7 +1346,6 @@ static const char * const cmd_subvolume_show_usage[] = {
 
 static int cmd_subvolume_show(const struct cmd_struct *cmd, int argc, char **argv)
 {
-	char tstr[256];
 	char uuidparse[BTRFS_UUID_UNPARSED_SIZE];
 	char *fullpath = NULL;
 	int fd = -1;
@@ -1260,6 +1358,7 @@ static int cmd_subvolume_show(const struct cmd_struct *cmd, int argc, char **arg
 	struct btrfs_util_subvolume_iterator *iter;
 	struct btrfs_util_subvolume_info subvol;
 	char *subvol_path = NULL;
+	char *subvol_name = NULL;
 	enum btrfs_util_error err;
 	struct btrfs_qgroup_stats stats;
 	unsigned int unit_mode;
@@ -1365,78 +1464,15 @@ static int cmd_subvolume_show(const struct cmd_struct *cmd, int argc, char **arg
 
 	}
 
-	/* Warn if it's a read-write subvolume with received_uuid */
-	if (!uuid_is_null(subvol.received_uuid) &&
-	    !(subvol.flags & BTRFS_ROOT_SUBVOL_RDONLY)) {
-		warning("the subvolume is read-write and has received_uuid set,\n"
-			"\t don't use it for incremental send. Please see section\n"
-			"\t 'SUBVOLUME FLAGS' in manual page btrfs-subvolume for\n"
-			"\t further information.");
-	}
-	/* print the info */
-	pr_verbose(LOG_DEFAULT, "%s\n", subvol.id == BTRFS_FS_TREE_OBJECTID ? "/" : subvol_path);
-	pr_verbose(LOG_DEFAULT, "\tName: \t\t\t%s\n",
-	       (subvol.id == BTRFS_FS_TREE_OBJECTID ? "<FS_TREE>" :
-		basename(subvol_path)));
-
-	if (uuid_is_null(subvol.uuid))
-		strcpy(uuidparse, "-");
-	else
-		uuid_unparse(subvol.uuid, uuidparse);
-	pr_verbose(LOG_DEFAULT, "\tUUID: \t\t\t%s\n", uuidparse);
-
-	if (uuid_is_null(subvol.parent_uuid))
-		strcpy(uuidparse, "-");
-	else
-		uuid_unparse(subvol.parent_uuid, uuidparse);
-	pr_verbose(LOG_DEFAULT, "\tParent UUID: \t\t%s\n", uuidparse);
-
-	if (uuid_is_null(subvol.received_uuid))
-		strcpy(uuidparse, "-");
-	else
-		uuid_unparse(subvol.received_uuid, uuidparse);
-	pr_verbose(LOG_DEFAULT, "\tReceived UUID: \t\t%s\n", uuidparse);
-
-	if (subvol.otime.tv_sec) {
-		struct tm tm;
-
-		localtime_r(&subvol.otime.tv_sec, &tm);
-		strftime(tstr, 256, "%Y-%m-%d %X %z", &tm);
-	} else
-		strcpy(tstr, "-");
-	pr_verbose(LOG_DEFAULT, "\tCreation time: \t\t%s\n", tstr);
-
-	pr_verbose(LOG_DEFAULT, "\tSubvolume ID: \t\t%" PRIu64 "\n", subvol.id);
-	pr_verbose(LOG_DEFAULT, "\tGeneration: \t\t%" PRIu64 "\n", subvol.generation);
-	pr_verbose(LOG_DEFAULT, "\tGen at creation: \t%" PRIu64 "\n", subvol.otransid);
-	pr_verbose(LOG_DEFAULT, "\tParent ID: \t\t%" PRIu64 "\n", subvol.parent_id);
-	pr_verbose(LOG_DEFAULT, "\tTop level ID: \t\t%" PRIu64 "\n", subvol.parent_id);
-
-	if (subvol.flags & BTRFS_ROOT_SUBVOL_RDONLY)
-		pr_verbose(LOG_DEFAULT, "\tFlags: \t\t\treadonly\n");
-	else
-		pr_verbose(LOG_DEFAULT, "\tFlags: \t\t\t-\n");
-
-	pr_verbose(LOG_DEFAULT, "\tSend transid: \t\t%" PRIu64 "\n", subvol.stransid);
-	pr_verbose(LOG_DEFAULT, "\tSend time: \t\t%s\n", tstr);
-	if (subvol.stime.tv_sec) {
-		struct tm tm;
-
-		localtime_r(&subvol.stime.tv_sec, &tm);
-		strftime(tstr, 256, "%Y-%m-%d %X %z", &tm);
+	if (subvol.id == BTRFS_FS_TREE_OBJECTID) {
+		free(subvol_path);
+		subvol_path = strdup("/");
+		subvol_name = "<FS_TREE>";
 	} else {
-		strcpy(tstr, "-");
+		subvol_name = basename(subvol_path);
 	}
-	pr_verbose(LOG_DEFAULT, "\tReceive transid: \t%" PRIu64 "\n", subvol.rtransid);
-	if (subvol.rtime.tv_sec) {
-		struct tm tm;
 
-		localtime_r(&subvol.rtime.tv_sec, &tm);
-		strftime(tstr, 256, "%Y-%m-%d %X %z", &tm);
-	} else {
-		strcpy(tstr, "-");
-	}
-	pr_verbose(LOG_DEFAULT, "\tReceive time: \t\t%s\n", tstr);
+	print_subvolume_show_text(&subvol, subvol_path, subvol_name);
 
 	/* print the snapshots of the given subvol if any*/
 	pr_verbose(LOG_DEFAULT, "\tSnapshot(s):\n");
@@ -1478,19 +1514,7 @@ static int cmd_subvolume_show(const struct cmd_struct *cmd, int argc, char **arg
 		goto out;
 	}
 
-	pr_verbose(LOG_DEFAULT, "\tQuota group:\t\t0/%" PRIu64 "\n", subvol.id);
-	fflush(stdout);
-
-	pr_verbose(LOG_DEFAULT, "\t  Limit referenced:\t%s\n",
-			stats.limit.max_referenced == 0 ? "-" :
-			pretty_size_mode(stats.limit.max_referenced, unit_mode));
-	pr_verbose(LOG_DEFAULT, "\t  Limit exclusive:\t%s\n",
-			stats.limit.max_exclusive == 0 ? "-" :
-			pretty_size_mode(stats.limit.max_exclusive, unit_mode));
-	pr_verbose(LOG_DEFAULT, "\t  Usage referenced:\t%s\n",
-			pretty_size_mode(stats.info.referenced, unit_mode));
-	pr_verbose(LOG_DEFAULT, "\t  Usage exclusive:\t%s\n",
-			pretty_size_mode(stats.info.exclusive, unit_mode));
+	print_subvolume_show_quota_text(&subvol, &stats, unit_mode);
 
 out:
 	free(subvol_path);
-- 
2.41.0

[PATCH 4/7] btrfs-progs: subvol: introduce rowspec definition for json output

[Christoph Heiss]

Includes all fields that are needed by the various `btrfs subvolume`
subcommands.

Signed-off-by: Christoph Heiss <christoph@c8h4.io>
---
 cmds/subvolume.c | 25 +++++++++++++++++++++++++
 cmds/subvolume.h | 19 +++++++++++++++++++
 2 files changed, 44 insertions(+)
 create mode 100644 cmds/subvolume.h

diff --git a/cmds/subvolume.c b/cmds/subvolume.c
index 65cff24b..cb863ac7 100644
--- a/cmds/subvolume.c
+++ b/cmds/subvolume.c
@@ -41,9 +41,34 @@
 #include "common/open-utils.h"
 #include "common/string-utils.h"
 #include "common/units.h"
+#include "common/format-output.h"
 #include "cmds/commands.h"
 #include "cmds/qgroup.h"
 
+const struct rowspec btrfs_subvolume_rowspec[] = {
+	{ .key = "ID", .fmt = "%llu", .out_json = "id" },
+	{ .key = "name", .fmt = "%s", .out_json = "name" },
+	{ .key = "gen", .fmt = "%llu", .out_json = "gen" },
+	{ .key = "cgen", .fmt = "%llu", .out_json = "cgen" },
+	{ .key = "parent", .fmt = "%llu", .out_json = "parent" },
+	{ .key = "top level", .fmt = "%llu", .out_json = "top_level" },
+	{ .key = "otime", .fmt = "time-long", .out_json = "otime" },
+	{ .key = "parent_uuid", .fmt = "uuid", .out_json = "parent_uuid" },
+	{ .key = "received_uuid", .fmt = "uuid", .out_json = "received_uuid" },
+	{ .key = "uuid", .fmt = "uuid", .out_json = "uuid" },
+	{ .key = "path", .fmt = "%s", .out_json = "path" },
+	{ .key = "flag-list-item", .fmt = "%s" },
+	{ .key = "stransid", .fmt = "%llu", .out_json = "stransid" },
+	{ .key = "stime", .fmt = "time-long", .out_json = "stime" },
+	{ .key = "rtransid", .fmt = "%llu", .out_json = "rtransid" },
+	{ .key = "rtime", .fmt = "time-long", .out_json = "rtime" },
+	{ .key = "snapshot-list-item", .fmt = "%s" },
+	{ .key = "quota-group", .fmt = "qgroupid", .out_json = "group" },
+	{ .key = "quota-ref", .fmt = "%llu", .out_json = "referenced" },
+	{ .key = "quota-excl", .fmt = "%llu", .out_json = "exclusive" },
+	ROWSPEC_END
+};
+
 static int wait_for_subvolume_cleaning(int fd, size_t count, uint64_t *ids,
 				       int sleep_interval)
 {
diff --git a/cmds/subvolume.h b/cmds/subvolume.h
new file mode 100644
index 00000000..041ea865
--- /dev/null
+++ b/cmds/subvolume.h
@@ -0,0 +1,19 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+struct rowspec;
+
+extern const struct rowspec btrfs_subvolume_rowspec[];
-- 
2.41.0

[PATCH 5/7] btrfs-progs: subvol list: implement json format output

[Christoph Heiss]

Implements JSON-formatted output for the `subvolume list` command using
the `--format json` global option, much like it is implemented for other
commands.

Re-uses the `btrfs_list_layout` infrastructure to nicely fit it into the
existing formatting code.

A notable difference to the normal, text-based output is that in the
JSON output, timestamps include the timezone offset as well.

Signed-off-by: Christoph Heiss <christoph@c8h4.io>
---
 cmds/subvolume-list.c | 91 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 88 insertions(+), 3 deletions(-)

diff --git a/cmds/subvolume-list.c b/cmds/subvolume-list.c
index 382b0676..be7faca6 100644
--- a/cmds/subvolume-list.c
+++ b/cmds/subvolume-list.c
@@ -35,7 +35,9 @@
 #include "common/open-utils.h"
 #include "common/string-utils.h"
 #include "common/utils.h"
+#include "common/format-output.h"
 #include "cmds/commands.h"
+#include "cmds/subvolume.h"
 
 /*
  * Naming of options:
@@ -75,6 +77,8 @@ static const char * const cmd_subvolume_list_usage[] = {
 	OPTLINE("--sort=gen,ogen,rootid,path", "list the subvolume in order of gen, ogen, rootid or path "
 		"you also can add '+' or '-' in front of each items. "
 		"(+:ascending, -:descending, ascending default)"),
+	HELPINFO_INSERT_GLOBALS,
+	HELPINFO_INSERT_FORMAT,
 	NULL,
 };
 
@@ -84,7 +88,8 @@ static const char * const cmd_subvolume_list_usage[] = {
 enum btrfs_list_layout {
 	BTRFS_LIST_LAYOUT_DEFAULT = 0,
 	BTRFS_LIST_LAYOUT_TABLE,
-	BTRFS_LIST_LAYOUT_RAW
+	BTRFS_LIST_LAYOUT_RAW,
+	BTRFS_LIST_LAYOUT_JSON
 };
 
 /*
@@ -1269,14 +1274,83 @@ static void print_all_subvol_info_tab_head(void)
 	}
 }
 
+static void print_subvol_json_key(struct format_ctx *fctx,
+				  const struct root_info *subv,
+				  const enum btrfs_list_column_enum column)
+{
+	const char *column_name;
+
+	UASSERT(0 <= column && column < BTRFS_LIST_ALL);
+
+	column_name = btrfs_list_columns[column].name;
+	switch (column) {
+	case BTRFS_LIST_OBJECTID:
+		fmt_print(fctx, column_name, subv->root_id);
+		break;
+	case BTRFS_LIST_GENERATION:
+		fmt_print(fctx, column_name, subv->gen);
+		break;
+	case BTRFS_LIST_OGENERATION:
+		fmt_print(fctx, column_name, subv->ogen);
+		break;
+	case BTRFS_LIST_PARENT:
+		fmt_print(fctx, column_name, subv->ref_tree);
+		break;
+	case BTRFS_LIST_TOP_LEVEL:
+		fmt_print(fctx, column_name, subv->top_id);
+		break;
+	case BTRFS_LIST_OTIME:
+		fmt_print(fctx, column_name, subv->otime);
+		break;
+	case BTRFS_LIST_UUID:
+		fmt_print(fctx, column_name, subv->uuid);
+		break;
+	case BTRFS_LIST_PUUID:
+		fmt_print(fctx, column_name, subv->puuid);
+		break;
+	case BTRFS_LIST_RUUID:
+		fmt_print(fctx, column_name, subv->ruuid);
+		break;
+	case BTRFS_LIST_PATH:
+		BUG_ON(!subv->full_path);
+		fmt_print(fctx, column_name, subv->full_path);
+		break;
+	default:
+		break;
+	}
+}
+
+static void print_one_subvol_info_json(struct format_ctx *fctx,
+				struct root_info *subv)
+{
+	int i;
+
+	fmt_print_start_group(fctx, NULL, JSON_TYPE_MAP);
+
+	for (i = 0; i < BTRFS_LIST_ALL; i++) {
+		if (!btrfs_list_columns[i].need_print)
+			continue;
+
+		print_subvol_json_key(fctx, subv, i);
+	}
+
+	fmt_print_end_group(fctx, NULL);
+}
+
+
 static void print_all_subvol_info(struct rb_root *sorted_tree,
 		  enum btrfs_list_layout layout, const char *raw_prefix)
 {
 	struct rb_node *n;
 	struct root_info *entry;
+	struct format_ctx fctx;
 
-	if (layout == BTRFS_LIST_LAYOUT_TABLE)
+	if (layout == BTRFS_LIST_LAYOUT_TABLE) {
 		print_all_subvol_info_tab_head();
+	} else if (layout == BTRFS_LIST_LAYOUT_JSON) {
+		fmt_start(&fctx, btrfs_subvolume_rowspec, 1, 0);
+		fmt_print_start_group(&fctx, "subvolume-list", JSON_TYPE_ARRAY);
+	}
 
 	n = rb_first(sorted_tree);
 	while (n) {
@@ -1296,10 +1370,18 @@ static void print_all_subvol_info(struct rb_root *sorted_tree,
 		case BTRFS_LIST_LAYOUT_RAW:
 			print_one_subvol_info_raw(entry, raw_prefix);
 			break;
+		case BTRFS_LIST_LAYOUT_JSON:
+			print_one_subvol_info_json(&fctx, entry);
+			break;
 		}
 next:
 		n = rb_next(n);
 	}
+
+	if (layout == BTRFS_LIST_LAYOUT_JSON) {
+		fmt_print_end_group(&fctx, "subvolume-list");
+		fmt_end(&fctx);
+	}
 }
 
 static int btrfs_list_subvols(int fd, struct rb_root *root_lookup)
@@ -1631,6 +1713,9 @@ static int cmd_subvolume_list(const struct cmd_struct *cmd, int argc, char **arg
 	btrfs_list_setup_print_column(BTRFS_LIST_TOP_LEVEL);
 	btrfs_list_setup_print_column(BTRFS_LIST_PATH);
 
+	if (bconf.output_format == CMD_FORMAT_JSON)
+		layout = BTRFS_LIST_LAYOUT_JSON;
+
 	ret = btrfs_list_subvols_print(fd, filter_set, comparer_set,
 			layout, !is_list_all && !is_only_in_path, NULL);
 
@@ -1644,4 +1729,4 @@ out:
 		usage(cmd, 1);
 	return !!ret;
 }
-DEFINE_SIMPLE_COMMAND(subvolume_list, "list");
+DEFINE_COMMAND_WITH_FLAGS(subvolume_list, "list", CMD_FORMAT_JSON);
-- 
2.41.0

[PATCH 6/7] btrfs-progs: subvol get-default: implement json format output

[Christoph Heiss]

Implements JSON-formatted output for the `subvolume get-default` command
using the `--format json` global option, much like it is implemented for
other commands.

Signed-off-by: Christoph Heiss <christoph@c8h4.io>
---
 cmds/subvolume.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/cmds/subvolume.c b/cmds/subvolume.c
index cb863ac7..f7076655 100644
--- a/cmds/subvolume.c
+++ b/cmds/subvolume.c
@@ -701,6 +701,8 @@ static DEFINE_SIMPLE_COMMAND(subvolume_snapshot, "snapshot");
 static const char * const cmd_subvolume_get_default_usage[] = {
 	"btrfs subvolume get-default <path>",
 	"Get the default subvolume of a filesystem",
+	HELPINFO_INSERT_GLOBALS,
+	HELPINFO_INSERT_FORMAT,
 	NULL
 };
 
@@ -712,6 +714,7 @@ static int cmd_subvolume_get_default(const struct cmd_struct *cmd, int argc, cha
 	DIR *dirstream = NULL;
 	enum btrfs_util_error err;
 	struct btrfs_util_subvolume_info subvol;
+	struct format_ctx fctx;
 	char *path;
 
 	clean_args_no_options(cmd, argc, argv);
@@ -731,7 +734,14 @@ static int cmd_subvolume_get_default(const struct cmd_struct *cmd, int argc, cha
 
 	/* no need to resolve roots if FS_TREE is default */
 	if (default_id == BTRFS_FS_TREE_OBJECTID) {
-		pr_verbose(LOG_DEFAULT, "ID 5 (FS_TREE)\n");
+		if (bconf.output_format == CMD_FORMAT_JSON) {
+			fmt_start(&fctx, btrfs_subvolume_rowspec, 1, 0);
+			fmt_print(&fctx, "ID", 5);
+			fmt_end(&fctx);
+		} else {
+			pr_verbose(LOG_DEFAULT, "ID 5 (FS_TREE)\n");
+		}
+
 		ret = 0;
 		goto out;
 	}
@@ -748,8 +758,17 @@ static int cmd_subvolume_get_default(const struct cmd_struct *cmd, int argc, cha
 		goto out;
 	}
 
-	pr_verbose(LOG_DEFAULT, "ID %" PRIu64 " gen %" PRIu64 " top level %" PRIu64 " path %s\n",
-	       subvol.id, subvol.generation, subvol.parent_id, path);
+	if (bconf.output_format == CMD_FORMAT_JSON) {
+		fmt_start(&fctx, btrfs_subvolume_rowspec, 1, 0);
+		fmt_print(&fctx, "ID", subvol.id);
+		fmt_print(&fctx, "gen", subvol.generation);
+		fmt_print(&fctx, "top level", subvol.parent_id);
+		fmt_print(&fctx, "path", path);
+		fmt_end(&fctx);
+	} else {
+		pr_verbose(LOG_DEFAULT, "ID %" PRIu64 " gen %" PRIu64 " top level %" PRIu64 " path %s\n",
+		       subvol.id, subvol.generation, subvol.parent_id, path);
+	}
 
 	free(path);
 
@@ -758,7 +777,7 @@ out:
 	close_file_or_dir(fd, dirstream);
 	return ret;
 }
-static DEFINE_SIMPLE_COMMAND(subvolume_get_default, "get-default");
+static DEFINE_COMMAND_WITH_FLAGS(subvolume_get_default, "get-default", CMD_FORMAT_JSON);
 
 static const char * const cmd_subvolume_set_default_usage[] = {
 	"btrfs subvolume set-default <subvolume>\n"
-- 
2.41.0

[PATCH 7/7] btrfs-progs: subvol show: implement json format output

[Christoph Heiss]

Implements JSON-formatted output for the `subvolume list` command using
the `--format json` global option, much like it is implemented for other
commands.

Signed-off-by: Christoph Heiss <christoph@c8h4.io>
---
 cmds/subvolume.c | 108 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 98 insertions(+), 10 deletions(-)

diff --git a/cmds/subvolume.c b/cmds/subvolume.c
index f7076655..1f513a4a 100644
--- a/cmds/subvolume.c
+++ b/cmds/subvolume.c
@@ -1375,6 +1375,64 @@ static void print_subvolume_show_quota_text(const struct btrfs_util_subvolume_in
 			pretty_size_mode(stats->info.exclusive, unit_mode));
 }
 
+static void print_subvolume_show_json(struct format_ctx *fctx,
+				      const struct btrfs_util_subvolume_info *subvol,
+				      const char *subvol_path, const char *subvol_name)
+{
+	fmt_print(fctx, "name", subvol_name);
+
+	if (!uuid_is_null(subvol->uuid))
+		fmt_print(fctx, "uuid", subvol->uuid);
+	if (!uuid_is_null(subvol->parent_uuid))
+		fmt_print(fctx, "parent_uuid", subvol->parent_uuid);
+	if (!uuid_is_null(subvol->received_uuid))
+		fmt_print(fctx, "received_uuid", subvol->received_uuid);
+
+	fmt_print(fctx, "otime", subvol->otime);
+	fmt_print(fctx, "ID", subvol->id);
+	fmt_print(fctx, "gen", subvol->generation);
+	fmt_print(fctx, "cgen", subvol->otransid);
+	fmt_print(fctx, "parent", subvol->parent_id);
+	fmt_print(fctx, "top level", subvol->parent_id);
+
+	fmt_print_start_group(fctx, "flags", JSON_TYPE_ARRAY);
+	if (subvol->flags & BTRFS_ROOT_SUBVOL_RDONLY)
+		fmt_print(fctx, "flag-list-item", "readonly");
+	fmt_print_end_group(fctx, "flags");
+
+	if (subvol->stransid)
+		fmt_print(fctx, "stransid", subvol->stransid);
+
+	if (subvol->stime.tv_sec)
+		fmt_print(fctx, "stime", subvol->stime);
+
+	if (subvol->rtransid)
+		fmt_print(fctx, "rtransid", subvol->rtransid);
+
+	if (subvol->rtime.tv_sec)
+		fmt_print(fctx, "rtime", subvol->rtime);
+}
+
+static void print_subvolume_show_quota_json(struct format_ctx *fctx,
+					     const struct btrfs_util_subvolume_info *subvol,
+					     const struct btrfs_qgroup_stats *stats)
+{
+	fmt_print_start_group(fctx, "quota", JSON_TYPE_MAP);
+	fmt_print(fctx, "quota-group", 0, subvol->id);
+
+	fmt_print_start_group(fctx, "limit", JSON_TYPE_MAP);
+	fmt_print(fctx, "quota-ref", stats->limit.max_referenced);
+	fmt_print(fctx, "quota-excl", stats->limit.max_exclusive);
+	fmt_print_end_group(fctx, "limit");
+
+	fmt_print_start_group(fctx, "usage", JSON_TYPE_MAP);
+	fmt_print(fctx, "quota-ref", stats->info.referenced);
+	fmt_print(fctx, "quota-excl", stats->info.exclusive);
+	fmt_print_end_group(fctx, "usage");
+
+	fmt_print_end_group(fctx, "quota");
+}
+
 static const char * const cmd_subvolume_show_usage[] = {
 	"btrfs subvolume show [options] <path>",
 	"Show more information about the subvolume (UUIDs, generations, times, snapshots)",
@@ -1385,6 +1443,8 @@ static const char * const cmd_subvolume_show_usage[] = {
 	OPTLINE("-r|--rootid ID", "root id of the subvolume"),
 	OPTLINE("-u|--uuid UUID", "UUID of the subvolum"),
 	HELPINFO_UNITS_SHORT_LONG,
+	HELPINFO_INSERT_GLOBALS,
+	HELPINFO_INSERT_FORMAT,
 	NULL
 };
 
@@ -1406,6 +1466,7 @@ static int cmd_subvolume_show(const struct cmd_struct *cmd, int argc, char **arg
 	enum btrfs_util_error err;
 	struct btrfs_qgroup_stats stats;
 	unsigned int unit_mode;
+	struct format_ctx fctx;
 
 	unit_mode = get_unit_mode_from_arg(&argc, argv, 1);
 
@@ -1516,10 +1577,19 @@ static int cmd_subvolume_show(const struct cmd_struct *cmd, int argc, char **arg
 		subvol_name = basename(subvol_path);
 	}
 
-	print_subvolume_show_text(&subvol, subvol_path, subvol_name);
+	if (bconf.output_format == CMD_FORMAT_JSON) {
+		fmt_start(&fctx, btrfs_subvolume_rowspec, 1, 0);
+		fmt_print_start_group(&fctx, subvol_path, JSON_TYPE_MAP);
+		print_subvolume_show_json(&fctx, &subvol, subvol_path, subvol_name);
+	} else {
+		print_subvolume_show_text(&subvol, subvol_path, subvol_name);
+	}
 
 	/* print the snapshots of the given subvol if any*/
-	pr_verbose(LOG_DEFAULT, "\tSnapshot(s):\n");
+	if (bconf.output_format == CMD_FORMAT_JSON)
+		fmt_print_start_group(&fctx, "snapshots", JSON_TYPE_ARRAY);
+	else
+		pr_verbose(LOG_DEFAULT, "\tSnapshot(s):\n");
 
 	err = btrfs_util_create_subvolume_iterator_fd(fd,
 						      BTRFS_FS_TREE_OBJECTID, 0,
@@ -1535,30 +1605,48 @@ static int cmd_subvolume_show(const struct cmd_struct *cmd, int argc, char **arg
 		} else if (err) {
 			error_btrfs_util(err);
 			btrfs_util_destroy_subvolume_iterator(iter);
-			goto out;
+			goto out2;
 		}
 
-		if (uuid_compare(subvol2.parent_uuid, subvol.uuid) == 0)
-			pr_verbose(LOG_DEFAULT, "\t\t\t\t%s\n", path);
+		if (uuid_compare(subvol2.parent_uuid, subvol.uuid) == 0) {
+			if (bconf.output_format == CMD_FORMAT_JSON)
+				fmt_print(&fctx, "snapshot-list-item", path);
+			else
+				pr_verbose(LOG_DEFAULT, "\t\t\t\t%s\n", path);
+		}
 
 		free(path);
 	}
+
+	if (bconf.output_format == CMD_FORMAT_JSON)
+		fmt_print_end_group(&fctx, "snapshots");
+
 	btrfs_util_destroy_subvolume_iterator(iter);
 
 	ret = btrfs_qgroup_query(fd, subvol.id, &stats);
 	if (ret == -ENOTTY) {
 		/* Quota information not available, not fatal */
-		pr_verbose(LOG_DEFAULT, "\tQuota group:\t\tn/a\n");
+		if (bconf.output_format == CMD_FORMAT_TEXT)
+			pr_verbose(LOG_DEFAULT, "\tQuota group:\t\tn/a\n");
 		ret = 0;
-		goto out;
+		goto out2;
 	}
 
 	if (ret) {
 		error("quota query failed: %m");
-		goto out;
+		goto out2;
 	}
 
-	print_subvolume_show_quota_text(&subvol, &stats, unit_mode);
+	if (bconf.output_format == CMD_FORMAT_JSON)
+		print_subvolume_show_quota_json(&fctx, &subvol, &stats);
+	else
+		print_subvolume_show_quota_text(&subvol, &stats, unit_mode);
+
+out2:
+	if (bconf.output_format == CMD_FORMAT_JSON) {
+		fmt_print_end_group(&fctx, subvol_path);
+		fmt_end(&fctx);
+	}
 
 out:
 	free(subvol_path);
@@ -1566,7 +1654,7 @@ out:
 	free(fullpath);
 	return !!ret;
 }
-static DEFINE_SIMPLE_COMMAND(subvolume_show, "show");
+static DEFINE_COMMAND_WITH_FLAGS(subvolume_show, "show", CMD_FORMAT_JSON);
 
 static const char * const cmd_subvolume_sync_usage[] = {
 	"btrfs subvolume sync <path> [<subvolid>...]",
-- 
2.41.0

Re: [PATCH 0/7] btrfs-progs: implement json output for subvolume subcommands

[David Sterba]

On Sun, Aug 13, 2023 at 11:44:55AM +0200, Christoph Heiss wrote:
> JSON as output format is already implement for some of the commands in
> btrfs-progs, which is a very useful feature to have for e.g. scripting.
> 
> This series adds the same for `btrfs subvolume list`, `btrfs subvolume
> get-default` and `btrfs subvolume show`.
> 
> #1-#3 are some small preparatory fixes & refactors, #4 introduces the
> `struct rowspec` containing all fields needed by the series.
> 
> The actual implementation of the JSON output is mostly pretty
> straight-forward, only cmd_subvolume_show() needed some refactoring
> first.
> 
> I probably will go about implementing the same for more commands, but
> wanted to get this out now to get some feedback.

Thanks. There are a few things regarding the json output that are still
to be figured out and to set examples to follow. The plain and json
output does not match 1:1 in the printed information, here the
'top level' does not need to be in the json output or there could be
more subvolume related info in the map. The textual output is
unfortunatelly parsed by many tools nowadays so we can't change the
format. With json it's easier to filter out the interesting data so
"more is better" in this case.

The formatter is designed in a way to allow plain text and json to be
printed by the same lines of code but this is namely for line oriented
output, like 'subvolume show'.

For the 'subvolume list' this may not be that easy so two separate
printing functions make more sense, like you did it in the series. So
that's fine.

I'll put some guidelines to the documentation, the key naming must be
unified, e.g. 'gen' or 'generation' but there's also 'transid' used in
some cases etc.

As the json format is also an ABI we need to get it finalized first so
I'll merge the series but put the actual support for --json option under
experimental build.

Re: [PATCH 5/7] btrfs-progs: subvol list: implement json format output

[David Sterba]

On Sun, Aug 13, 2023 at 11:45:00AM +0200, Christoph Heiss wrote:
> Implements JSON-formatted output for the `subvolume list` command using
> the `--format json` global option, much like it is implemented for other
> commands.
> 
> Re-uses the `btrfs_list_layout` infrastructure to nicely fit it into the
> existing formatting code.
> 
> A notable difference to the normal, text-based output is that in the
> JSON output, timestamps include the timezone offset as well.
> 
> Signed-off-by: Christoph Heiss <christoph@c8h4.io>
> ---
>  cmds/subvolume-list.c | 91 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 88 insertions(+), 3 deletions(-)
> 
> diff --git a/cmds/subvolume-list.c b/cmds/subvolume-list.c
> index 382b0676..be7faca6 100644
> --- a/cmds/subvolume-list.c
> +++ b/cmds/subvolume-list.c
> @@ -35,7 +35,9 @@
>  #include "common/open-utils.h"
>  #include "common/string-utils.h"
>  #include "common/utils.h"
> +#include "common/format-output.h"
>  #include "cmds/commands.h"
> +#include "cmds/subvolume.h"
>  
>  /*
>   * Naming of options:
> @@ -75,6 +77,8 @@ static const char * const cmd_subvolume_list_usage[] = {
>  	OPTLINE("--sort=gen,ogen,rootid,path", "list the subvolume in order of gen, ogen, rootid or path "
>  		"you also can add '+' or '-' in front of each items. "
>  		"(+:ascending, -:descending, ascending default)"),
> +	HELPINFO_INSERT_GLOBALS,
> +	HELPINFO_INSERT_FORMAT,
>  	NULL,
>  };
>  
> @@ -84,7 +88,8 @@ static const char * const cmd_subvolume_list_usage[] = {
>  enum btrfs_list_layout {
>  	BTRFS_LIST_LAYOUT_DEFAULT = 0,
>  	BTRFS_LIST_LAYOUT_TABLE,
> -	BTRFS_LIST_LAYOUT_RAW
> +	BTRFS_LIST_LAYOUT_RAW,
> +	BTRFS_LIST_LAYOUT_JSON

Json should not be a layout, that's relevant only for plain text output.

Re: [PATCH 7/7] btrfs-progs: subvol show: implement json format output

[David Sterba]

On Sun, Aug 13, 2023 at 11:45:02AM +0200, Christoph Heiss wrote:
> Implements JSON-formatted output for the `subvolume list` command using
> the `--format json` global option, much like it is implemented for other
> commands.
> 
> Signed-off-by: Christoph Heiss <christoph@c8h4.io>
> ---
>  cmds/subvolume.c | 108 ++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 98 insertions(+), 10 deletions(-)
> 
> diff --git a/cmds/subvolume.c b/cmds/subvolume.c
> index f7076655..1f513a4a 100644
> --- a/cmds/subvolume.c
> +++ b/cmds/subvolume.c
> @@ -1375,6 +1375,64 @@ static void print_subvolume_show_quota_text(const struct btrfs_util_subvolume_in
>  			pretty_size_mode(stats->info.exclusive, unit_mode));
>  }
>  
> +static void print_subvolume_show_json(struct format_ctx *fctx,
> +				      const struct btrfs_util_subvolume_info *subvol,
> +				      const char *subvol_path, const char *subvol_name)
> +{
> +	fmt_print(fctx, "name", subvol_name);
> +
> +	if (!uuid_is_null(subvol->uuid))
> +		fmt_print(fctx, "uuid", subvol->uuid);

Regarding the "more is better" approach, no conditionals for similar
data.

> +	if (!uuid_is_null(subvol->parent_uuid))
> +		fmt_print(fctx, "parent_uuid", subvol->parent_uuid);
> +	if (!uuid_is_null(subvol->received_uuid))
> +		fmt_print(fctx, "received_uuid", subvol->received_uuid);
> +
> +	fmt_print(fctx, "otime", subvol->otime);
> +	fmt_print(fctx, "ID", subvol->id);
> +	fmt_print(fctx, "gen", subvol->generation);
> +	fmt_print(fctx, "cgen", subvol->otransid);
> +	fmt_print(fctx, "parent", subvol->parent_id);
> +	fmt_print(fctx, "top level", subvol->parent_id);
> +
> +	fmt_print_start_group(fctx, "flags", JSON_TYPE_ARRAY);
> +	if (subvol->flags & BTRFS_ROOT_SUBVOL_RDONLY)
> +		fmt_print(fctx, "flag-list-item", "readonly");
> +	fmt_print_end_group(fctx, "flags");
> +
> +	if (subvol->stransid)
> +		fmt_print(fctx, "stransid", subvol->stransid);
> +
> +	if (subvol->stime.tv_sec)
> +		fmt_print(fctx, "stime", subvol->stime);
> +
> +	if (subvol->rtransid)
> +		fmt_print(fctx, "rtransid", subvol->rtransid);
> +
> +	if (subvol->rtime.tv_sec)
> +		fmt_print(fctx, "rtime", subvol->rtime);
> +}

Re: [PATCH 6/7] btrfs-progs: subvol get-default: implement json format output

[David Sterba]

On Sun, Aug 13, 2023 at 11:45:01AM +0200, Christoph Heiss wrote:
> Implements JSON-formatted output for the `subvolume get-default` command
> using the `--format json` global option, much like it is implemented for
> other commands.
> 
> Signed-off-by: Christoph Heiss <christoph@c8h4.io>
> ---
>  cmds/subvolume.c | 27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/cmds/subvolume.c b/cmds/subvolume.c
> index cb863ac7..f7076655 100644
> --- a/cmds/subvolume.c
> +++ b/cmds/subvolume.c
> @@ -701,6 +701,8 @@ static DEFINE_SIMPLE_COMMAND(subvolume_snapshot, "snapshot");
>  static const char * const cmd_subvolume_get_default_usage[] = {
>  	"btrfs subvolume get-default <path>",
>  	"Get the default subvolume of a filesystem",
> +	HELPINFO_INSERT_GLOBALS,
> +	HELPINFO_INSERT_FORMAT,
>  	NULL
>  };
>  
> @@ -712,6 +714,7 @@ static int cmd_subvolume_get_default(const struct cmd_struct *cmd, int argc, cha
>  	DIR *dirstream = NULL;
>  	enum btrfs_util_error err;
>  	struct btrfs_util_subvolume_info subvol;
> +	struct format_ctx fctx;
>  	char *path;
>  
>  	clean_args_no_options(cmd, argc, argv);
> @@ -731,7 +734,14 @@ static int cmd_subvolume_get_default(const struct cmd_struct *cmd, int argc, cha
>  
>  	/* no need to resolve roots if FS_TREE is default */
>  	if (default_id == BTRFS_FS_TREE_OBJECTID) {
> -		pr_verbose(LOG_DEFAULT, "ID 5 (FS_TREE)\n");
> +		if (bconf.output_format == CMD_FORMAT_JSON) {
> +			fmt_start(&fctx, btrfs_subvolume_rowspec, 1, 0);
> +			fmt_print(&fctx, "ID", 5);
> +			fmt_end(&fctx);
> +		} else {
> +			pr_verbose(LOG_DEFAULT, "ID 5 (FS_TREE)\n");
> +		}
> +
>  		ret = 0;
>  		goto out;
>  	}
> @@ -748,8 +758,17 @@ static int cmd_subvolume_get_default(const struct cmd_struct *cmd, int argc, cha
>  		goto out;
>  	}
>  
> -	pr_verbose(LOG_DEFAULT, "ID %" PRIu64 " gen %" PRIu64 " top level %" PRIu64 " path %s\n",
> -	       subvol.id, subvol.generation, subvol.parent_id, path);
> +	if (bconf.output_format == CMD_FORMAT_JSON) {
> +		fmt_start(&fctx, btrfs_subvolume_rowspec, 1, 0);
> +		fmt_print(&fctx, "ID", subvol.id);
> +		fmt_print(&fctx, "gen", subvol.generation);
> +		fmt_print(&fctx, "top level", subvol.parent_id);
> +		fmt_print(&fctx, "path", path);
> +		fmt_end(&fctx);

Such block can be in a helper and used for 'list' and 'get-default' so
it's unified.

> +	} else {
> +		pr_verbose(LOG_DEFAULT, "ID %" PRIu64 " gen %" PRIu64 " top level %" PRIu64 " path %s\n",
> +		       subvol.id, subvol.generation, subvol.parent_id, path);

The formatter always prints '\n' at the end of the plain text values, so
with a minor update the same helper can be used to produce the plain
output.

Re: [PATCH 0/7] btrfs-progs: implement json output for subvolume subcommands

[Christoph Heiss]

First of, thanks for the review & merging right away!

On Thu, Aug 17, 2023 at 09:34:37PM +0200, David Sterba wrote:
>
> On Sun, Aug 13, 2023 at 11:44:55AM +0200, Christoph Heiss wrote:
> > [..]
>
> Thanks. There are a few things regarding the json output that are still
> to be figured out and to set examples to follow. The plain and json
> output does not match 1:1 in the printed information, here the
> 'top level' does not need to be in the json output or there could be
> more subvolume related info in the map.

> The textual output is unfortunatelly parsed by many tools nowadays so
> we can't change the format. With json it's easier to filter out the
> interesting data so "more is better" in this case.
Right, makes sense. I skimmed through your additional commits on top,
e.g. the null uuid thing. So all "optional" fields should rather be
`null` than missing.

>
> The formatter is designed in a way to allow plain text and json to be
> printed by the same lines of code but this is namely for line oriented
> output, like 'subvolume show'.
Yeah, I figured that after looking at it a bit more - that's why I
decided to leave most of the stuff as-is for now.

> [..]
>
> I'll put some guidelines to the documentation, the key naming must be
> unified, e.g. 'gen' or 'generation' but there's also 'transid' used in
> some cases etc.
>
> As the json format is also an ABI we need to get it finalized first
Does it make sense to explicitly document all the possible json outputs
with all their fields, i.e provide example outputs?

> so I'll merge the series but put the actual support for --json option
> under experimental build.
Thanks! Makes it easier to improve on it gradually in any case. I will
send some more patches your way rectifying these things as soon as I get
to it.

Re: [PATCH 6/7] btrfs-progs: subvol get-default: implement json format output

[Christoph Heiss]

Thanks for the feedback.

On Thu, Aug 17, 2023 at 10:04:49PM +0200, David Sterba wrote:
>
> On Sun, Aug 13, 2023 at 11:45:01AM +0200, Christoph Heiss wrote:
> > Implements JSON-formatted output for the `subvolume get-default` command
> > using the `--format json` global option, much like it is implemented for
> > other commands.
> >
> > Signed-off-by: Christoph Heiss <christoph@c8h4.io>
> > ---
> >  cmds/subvolume.c | 27 +++++++++++++++++++++++----
> >  1 file changed, 23 insertions(+), 4 deletions(-)
> >
> > diff --git a/cmds/subvolume.c b/cmds/subvolume.c
> > index cb863ac7..f7076655 100644
> > --- a/cmds/subvolume.c
> > +++ b/cmds/subvolume.c
> > [..]
> > @@ -748,8 +758,17 @@ static int cmd_subvolume_get_default(const struct cmd_struct *cmd, int argc, cha
> >  		goto out;
> >  	}
> >
> > -	pr_verbose(LOG_DEFAULT, "ID %" PRIu64 " gen %" PRIu64 " top level %" PRIu64 " path %s\n",
> > -	       subvol.id, subvol.generation, subvol.parent_id, path);
> > +	if (bconf.output_format == CMD_FORMAT_JSON) {
> > +		fmt_start(&fctx, btrfs_subvolume_rowspec, 1, 0);
> > +		fmt_print(&fctx, "ID", subvol.id);
> > +		fmt_print(&fctx, "gen", subvol.generation);
> > +		fmt_print(&fctx, "top level", subvol.parent_id);
> > +		fmt_print(&fctx, "path", path);
> > +		fmt_end(&fctx);
>
> Such block can be in a helper and used for 'list' and 'get-default' so
> it's unified.
Looks easy enough, I'll get on it.

>
> > +	} else {
> > +		pr_verbose(LOG_DEFAULT, "ID %" PRIu64 " gen %" PRIu64 " top level %" PRIu64 " path %s\n",
> > +		       subvol.id, subvol.generation, subvol.parent_id, path);
>
> The formatter always prints '\n' at the end of the plain text values, so
> with a minor update the same helper can be used to produce the plain
> output.
Ditto.

Re: [PATCH 0/7] btrfs-progs: implement json output for subvolume subcommands

[David Sterba]

On Fri, Aug 18, 2023 at 08:39:58PM +0200, Christoph Heiss wrote:
> First of, thanks for the review & merging right away!
> On Thu, Aug 17, 2023 at 09:34:37PM +0200, David Sterba wrote:
> >
> > On Sun, Aug 13, 2023 at 11:44:55AM +0200, Christoph Heiss wrote:
> > > [..]
> >
> > Thanks. There are a few things regarding the json output that are still
> > to be figured out and to set examples to follow. The plain and json
> > output does not match 1:1 in the printed information, here the
> > 'top level' does not need to be in the json output or there could be
> > more subvolume related info in the map.
> 
> > The textual output is unfortunatelly parsed by many tools nowadays so
> > we can't change the format. With json it's easier to filter out the
> > interesting data so "more is better" in this case.
> Right, makes sense. I skimmed through your additional commits on top,
> e.g. the null uuid thing. So all "optional" fields should rather be
> `null` than missing.
> 
> >
> > The formatter is designed in a way to allow plain text and json to be
> > printed by the same lines of code but this is namely for line oriented
> > output, like 'subvolume show'.
> Yeah, I figured that after looking at it a bit more - that's why I
> decided to leave most of the stuff as-is for now.
> 
> > [..]
> >
> > I'll put some guidelines to the documentation, the key naming must be
> > unified, e.g. 'gen' or 'generation' but there's also 'transid' used in
> > some cases etc.
> >
> > As the json format is also an ABI we need to get it finalized first
> Does it make sense to explicitly document all the possible json outputs
> with all their fields, i.e provide example outputs?

Yes, examples are very useful, all of the commands supporting json
should have coverage in the tests. For startes a command that does not
change the state could just duplicate the one done in test but with
`--format=json'. Then it'll appear in the test logs and is easy to
review or copy elsewhere.

The tests/json-formatter-test.c should cover all the basic stuff, it's
also not complete so it can serve as example provider.

We may want to put the examples into the documentation then it's also
good for understanding, though this coud get out of sync over time.

> > so I'll merge the series but put the actual support for --json option
> > under experimental build.
> Thanks! Makes it easier to improve on it gradually in any case. I will
> send some more patches your way rectifying these things as soon as I get
> to it.

Thanks.