[PATCH v2] btrfs-progs: device stats: add json output format

[PATCH v2] btrfs-progs: device stats: add json output format

[Sidong Yang]

Add supports for json formatting, this patch changes hard coded printing
code to formatted print with output formatter. Json output would be
useful for other programs that parse output of the command. but it
changes the text format.

Example text format:

device:                 /dev/vdb
write_io_errs:          0
read_io_errs:           0
flush_io_errs:          0
corruption_errs:        0
generation_errs:        0

Example json format:

{
  "__header": {
    "version": "1"
  },
  "device-stats": {
    "/dev/vdb": {
      "device": "/dev/vdb",
      "write_io_errs": "0",
      "read_io_errs": "0",
      "flush_io_errs": "0",
      "corruption_errs": "0",
      "generation_errs": "0"
    }
  },
}

Cc: David Sterba <dsterba@suse.cz>

Signed-off-by: Sidong Yang <realwakka@gmail.com>
---
v2:
 - use output formatter
---
 cmds/device.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/cmds/device.c b/cmds/device.c
index d72881f8..91b45d9f 100644
--- a/cmds/device.c
+++ b/cmds/device.c
@@ -36,6 +36,7 @@
 #include "common/path-utils.h"
 #include "common/device-utils.h"
 #include "common/device-scan.h"
+#include "common/format-output.h"
 #include "mkfs/common.h"
 
 static const char * const device_cmd_group_usage[] = {
@@ -459,6 +460,16 @@ out:
 }
 static DEFINE_SIMPLE_COMMAND(device_ready, "ready");
 
+static const struct rowspec device_stats_rowspec[] = {
+	{ .key = "device", .fmt = "%s", .out_text = "device", .out_json = "device" },
+	{ .key = "write_io_errs", .fmt = "%llu", .out_text = "write_io_errs", .out_json = "write_io_errs" },
+	{ .key = "read_io_errs", .fmt = "%llu", .out_text = "read_io_errs", .out_json = "read_io_errs" },
+	{ .key = "flush_io_errs", .fmt = "%llu", .out_text = "flush_io_errs", .out_json = "flush_io_errs" },
+	{ .key = "corruption_errs", .fmt = "%llu", .out_text = "corruption_errs", .out_json = "corruption_errs" },
+	{ .key = "generation_errs", .fmt = "%llu", .out_text = "generation_errs", .out_json = "generation_errs" },
+	ROWSPEC_END
+};
+
 static const char * const cmd_device_stats_usage[] = {
 	"btrfs device stats [options] <path>|<device>",
 	"Show device IO error statistics",
@@ -482,6 +493,7 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
 	int check = 0;
 	__u64 flags = 0;
 	DIR *dirstream = NULL;
+	struct format_ctx fctx;
 
 	optind = 0;
 	while (1) {
@@ -530,6 +542,8 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
 		goto out;
 	}
 
+	fmt_start(&fctx, device_stats_rowspec, 24, 0);
+	fmt_print_start_group(&fctx, "device-stats", JSON_TYPE_MAP);
 	for (i = 0; i < fi_args.num_devices; i++) {
 		struct btrfs_ioctl_get_dev_stats args = {0};
 		char path[BTRFS_DEVICE_PATH_NAME_MAX + 1];
@@ -574,31 +588,34 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
 				snprintf(canonical_path, 32,
 					 "devid:%llu", args.devid);
 			}
+			fmt_print_start_group(&fctx, canonical_path, JSON_TYPE_MAP);
+			fmt_print(&fctx, "device", canonical_path);
 
 			for (j = 0; j < ARRAY_SIZE(dev_stats); j++) {
 				/* We got fewer items than we know */
 				if (args.nr_items < dev_stats[j].num + 1)
 					continue;
-				printf("[%s].%-16s %llu\n", canonical_path,
-					dev_stats[j].name,
-					(unsigned long long)
-					 args.values[dev_stats[j].num]);
+
+				fmt_print(&fctx, dev_stats[j].name, args.values[dev_stats[j].num]);
 				if ((check == 1)
 				    && (args.values[dev_stats[j].num] > 0))
 					err |= 64;
 			}
-
+			fmt_print_end_group(&fctx, "device");
 			free(canonical_path);
 		}
 	}
 
+	fmt_print_end_group(&fctx, "device-stats");
+	fmt_end(&fctx);
+
 out:
 	free(di_args);
 	close_file_or_dir(fdmnt, dirstream);
 
 	return err;
 }
-static DEFINE_SIMPLE_COMMAND(device_stats, "stats");
+static DEFINE_COMMAND_WITH_FLAGS(device_stats, "stats", CMD_FORMAT_JSON);
 
 static const char * const cmd_device_usage_usage[] = {
 	"btrfs device usage [options] <path> [<path>..]",
-- 
2.25.1

Re: [PATCH v2] btrfs-progs: device stats: add json output format

[David Sterba]

On Sun, Oct 04, 2020 at 11:25:57AM +0000, Sidong Yang wrote:
> Add supports for json formatting, this patch changes hard coded printing
> code to formatted print with output formatter. Json output would be
> useful for other programs that parse output of the command. but it
> changes the text format.
> 
> Example text format:
> 
> device:                 /dev/vdb
> write_io_errs:          0
> read_io_errs:           0
> flush_io_errs:          0
> corruption_errs:        0
> generation_errs:        0
> 
> Example json format:
> 
> {
>   "__header": {
>     "version": "1"
>   },
>   "device-stats": {
>     "/dev/vdb": {
>       "device": "/dev/vdb",
>       "write_io_errs": "0",
>       "read_io_errs": "0",
>       "flush_io_errs": "0",
>       "corruption_errs": "0",
>       "generation_errs": "0"
>     }
>   },
> }

The overall structure looks good, ie. the separate object 'device-stats'
and then the contents. For that the device id should be either key to a
map, or we can put it into an array (where device id must be present
too).

A check if the format is usable you can try to write a sample tool that
parses some of the data and prints them. So eg. using python or jq and
print stats of device 1. Which points out that device id is missing for
example.

Re: [PATCH v2] btrfs-progs: device stats: add json output format

[Sidong Yang]

On Fri, Oct 30, 2020 at 06:55:25PM +0100, David Sterba wrote:
> On Sun, Oct 04, 2020 at 11:25:57AM +0000, Sidong Yang wrote:
> > Add supports for json formatting, this patch changes hard coded printing
> > code to formatted print with output formatter. Json output would be
> > useful for other programs that parse output of the command. but it
> > changes the text format.
> > 
> > Example text format:
> > 
> > device:                 /dev/vdb
> > write_io_errs:          0
> > read_io_errs:           0
> > flush_io_errs:          0
> > corruption_errs:        0
> > generation_errs:        0
> > 
> > Example json format:
> > 
> > {
> >   "__header": {
> >     "version": "1"
> >   },
> >   "device-stats": {
> >     "/dev/vdb": {
> >       "device": "/dev/vdb",
> >       "write_io_errs": "0",
> >       "read_io_errs": "0",
> >       "flush_io_errs": "0",
> >       "corruption_errs": "0",
> >       "generation_errs": "0"
> >     }
> >   },
> > }

Hi David!
Thanks for review.

> 
> The overall structure looks good, ie. the separate object 'device-stats'
> and then the contents. For that the device id should be either key to a
> map, or we can put it into an array (where device id must be present
> too).

Thanks, You mean that devid should be key to json map? And I think that 
using as key is better than using array. Example should be like this.

{
  "__header": {
    "version": "1"
  },
  "device-stats": {
    "1": {
      "devid": "1",
      "device": "/dev/vdb",
      "write_io_errs": "0",
      "read_io_errs": "0",
      "flush_io_errs": "0",
      "corruption_errs": "0",
      "generation_errs": "0"
    }
  },
}

If so, I'll write a new patch for this.

Thanks,
Sidong

> 
> A check if the format is usable you can try to write a sample tool that
> parses some of the data and prints them. So eg. using python or jq and
> print stats of device 1. Which points out that device id is missing for
> example.

Re: [PATCH v2] btrfs-progs: device stats: add json output format

[Sidong Yang]

On Fri, Oct 30, 2020 at 06:55:25PM +0100, David Sterba wrote:
> On Sun, Oct 04, 2020 at 11:25:57AM +0000, Sidong Yang wrote:
> > Add supports for json formatting, this patch changes hard coded printing
> > code to formatted print with output formatter. Json output would be
> > useful for other programs that parse output of the command. but it
> > changes the text format.
> > 
> > Example text format:
> > 
> > device:                 /dev/vdb
> > write_io_errs:          0
> > read_io_errs:           0
> > flush_io_errs:          0
> > corruption_errs:        0
> > generation_errs:        0
> > 
> > Example json format:
> > 
> > {
> >   "__header": {
> >     "version": "1"
> >   },
> >   "device-stats": {
> >     "/dev/vdb": {
> >       "device": "/dev/vdb",
> >       "write_io_errs": "0",
> >       "read_io_errs": "0",
> >       "flush_io_errs": "0",
> >       "corruption_errs": "0",
> >       "generation_errs": "0"
> >     }
> >   },
> > }
> 
> The overall structure looks good, ie. the separate object 'device-stats'
> and then the contents. For that the device id should be either key to a
> map, or we can put it into an array (where device id must be present
> too).

IMHO, It's okay to put 'device-stats' object into an array like below.

{
  "__header": {
    "version": "1"
  },
  "device-stats": [
    {
      "devid": "1",
      "device": "/dev/vdb",
      "write_io_errs": "0",
      "read_io_errs": "0",
      "flush_io_errs": "0",
      "corruption_errs": "0",
      "generation_errs": "0"
    }
  ],
}

but I can't find the way to insert an object like 'fmt_start_object'.
I think we need it for this.

Thanks,
Sidong


> 
> A check if the format is usable you can try to write a sample tool that
> parses some of the data and prints them. So eg. using python or jq and
> print stats of device 1. Which points out that device id is missing for
> example.