All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] btrfs-progs: common: introduce fmt_print_start_object
@ 2020-11-11 16:39 Sidong Yang
  2020-11-11 16:39 ` [PATCH v2 2/2] btrfs-progs: device stats: add json output format Sidong Yang
  2020-12-10 20:28 ` [PATCH v2 1/2] btrfs-progs: common: introduce fmt_print_start_object David Sterba
  0 siblings, 2 replies; 6+ messages in thread
From: Sidong Yang @ 2020-11-11 16:39 UTC (permalink / raw)
  To: dsterba, linux-btrfs; +Cc: Sidong Yang

Introduce a new function that can be used when you need to print an json
object in command like "device stats".

Signed-off-by: Sidong Yang <realwakka@gmail.com>
---
 common/format-output.c | 20 ++++++++++++++++++++
 common/format-output.h |  3 +++
 2 files changed, 23 insertions(+)

diff --git a/common/format-output.c b/common/format-output.c
index 8df93ecb..f31e7259 100644
--- a/common/format-output.c
+++ b/common/format-output.c
@@ -213,6 +213,26 @@ void fmt_print_end_group(struct format_ctx *fctx, const char *name)
 	}
 }
 
+void fmt_print_start_object(struct format_ctx *fctx)
+{
+	if (bconf.output_format == CMD_FORMAT_JSON) {
+		fmt_separator(fctx);
+		fmt_inc_depth(fctx);
+		fctx->memb[fctx->depth] = 0;
+		putchar('{');
+	}
+}
+
+void fmt_print_end_object(struct format_ctx *fctx)
+{
+	if (bconf.output_format == CMD_FORMAT_JSON) {
+		fmt_dec_depth(fctx);
+		putchar('\n');
+		fmt_indent2(fctx->depth);
+		putchar('}');
+	}
+}
+
 /* Use rowspec to print according to currently set output format */
 void fmt_print(struct format_ctx *fctx, const char* key, ...)
 {
diff --git a/common/format-output.h b/common/format-output.h
index bcc2d74d..9d606482 100644
--- a/common/format-output.h
+++ b/common/format-output.h
@@ -87,4 +87,7 @@ void fmt_print_start_group(struct format_ctx *fctx, const char *name,
 		enum json_type jtype);
 void fmt_print_end_group(struct format_ctx *fctx, const char *name);
 
+void fmt_print_start_object(struct format_ctx *fctx);
+void fmt_print_end_object(struct format_ctx *fctx);
+
 #endif
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] btrfs-progs: device stats: add json output format
  2020-11-11 16:39 [PATCH v2 1/2] btrfs-progs: common: introduce fmt_print_start_object Sidong Yang
@ 2020-11-11 16:39 ` Sidong Yang
  2020-12-10 20:53   ` David Sterba
  2021-02-16 10:21   ` Filipe Manana
  2020-12-10 20:28 ` [PATCH v2 1/2] btrfs-progs: common: introduce fmt_print_start_object David Sterba
  1 sibling, 2 replies; 6+ messages in thread
From: Sidong Yang @ 2020-11-11 16:39 UTC (permalink / raw)
  To: dsterba, linux-btrfs; +Cc: 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
devid			1
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": [
    {
      "device": "/dev/vdb",
      "devid": "1",
      "write_io_errs": "0",
      "read_io_errs": "0",
      "flush_io_errs": "0",
      "corruption_errs": "0",
      "generation_errs": "0"
    }
  ],
}

Signed-off-by: Sidong Yang <realwakka@gmail.com>
---
v2:
 - use json array for print
---
 cmds/device.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/cmds/device.c b/cmds/device.c
index d72881f8..8b8fc85c 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,17 @@ 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 = "devid", .fmt = "%u", .out_text = "devid", .out_json = "devid" },
+	{ .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 +494,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 +543,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_ARRAY);
 	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];
@@ -548,6 +563,7 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
 			err |= 1;
 		} else {
 			char *canonical_path;
+			char devid_str[32];
 			int j;
 			static const struct {
 				const char name[32];
@@ -574,31 +590,36 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
 				snprintf(canonical_path, 32,
 					 "devid:%llu", args.devid);
 			}
+			snprintf(devid_str, 32, "%llu", args.devid);
+			fmt_print_start_object(&fctx);
+			fmt_print(&fctx, "device", canonical_path);
+			fmt_print(&fctx, "devid", di_args[i].devid);
 
 			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_object(&fctx);
 			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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] btrfs-progs: common: introduce fmt_print_start_object
  2020-11-11 16:39 [PATCH v2 1/2] btrfs-progs: common: introduce fmt_print_start_object Sidong Yang
  2020-11-11 16:39 ` [PATCH v2 2/2] btrfs-progs: device stats: add json output format Sidong Yang
@ 2020-12-10 20:28 ` David Sterba
  2020-12-11 16:11   ` Sidong Yang
  1 sibling, 1 reply; 6+ messages in thread
From: David Sterba @ 2020-12-10 20:28 UTC (permalink / raw)
  To: Sidong Yang; +Cc: dsterba, linux-btrfs

On Wed, Nov 11, 2020 at 04:39:08PM +0000, Sidong Yang wrote:
> Introduce a new function that can be used when you need to print an json
> object in command like "device stats".
> 
> Signed-off-by: Sidong Yang <realwakka@gmail.com>
> ---
>  common/format-output.c | 20 ++++++++++++++++++++
>  common/format-output.h |  3 +++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/common/format-output.c b/common/format-output.c
> index 8df93ecb..f31e7259 100644
> --- a/common/format-output.c
> +++ b/common/format-output.c
> @@ -213,6 +213,26 @@ void fmt_print_end_group(struct format_ctx *fctx, const char *name)
>  	}
>  }
>  
> +void fmt_print_start_object(struct format_ctx *fctx)
> +{
> +	if (bconf.output_format == CMD_FORMAT_JSON) {
> +		fmt_separator(fctx);
> +		fmt_inc_depth(fctx);
> +		fctx->memb[fctx->depth] = 0;
> +		putchar('{');
> +	}
> +}

This duplicates what fmt_print_start_group and fmt_print_end_group do,
so these should be extended to handle when 'name' is NULL and print only
the [ or {.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] btrfs-progs: device stats: add json output format
  2020-11-11 16:39 ` [PATCH v2 2/2] btrfs-progs: device stats: add json output format Sidong Yang
@ 2020-12-10 20:53   ` David Sterba
  2021-02-16 10:21   ` Filipe Manana
  1 sibling, 0 replies; 6+ messages in thread
From: David Sterba @ 2020-12-10 20:53 UTC (permalink / raw)
  To: Sidong Yang; +Cc: dsterba, linux-btrfs

On Wed, Nov 11, 2020 at 04:39:09PM +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
> devid			1
> 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": [
>     {
>       "device": "/dev/vdb",
>       "devid": "1",
>       "write_io_errs": "0",
>       "read_io_errs": "0",
>       "flush_io_errs": "0",
>       "corruption_errs": "0",
>       "generation_errs": "0"
>     }
>   ],
> }

This looks good, regarding the stats this should be sufficient. We could
add some more filesystem information, but as it would be probably
another hash key.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] btrfs-progs: common: introduce fmt_print_start_object
  2020-12-10 20:28 ` [PATCH v2 1/2] btrfs-progs: common: introduce fmt_print_start_object David Sterba
@ 2020-12-11 16:11   ` Sidong Yang
  0 siblings, 0 replies; 6+ messages in thread
From: Sidong Yang @ 2020-12-11 16:11 UTC (permalink / raw)
  To: dsterba, linux-btrfs

On Thu, Dec 10, 2020 at 09:28:47PM +0100, David Sterba wrote:
> On Wed, Nov 11, 2020 at 04:39:08PM +0000, Sidong Yang wrote:
> > Introduce a new function that can be used when you need to print an json
> > object in command like "device stats".
> > 
> > Signed-off-by: Sidong Yang <realwakka@gmail.com>
> > ---
> >  common/format-output.c | 20 ++++++++++++++++++++
> >  common/format-output.h |  3 +++
> >  2 files changed, 23 insertions(+)
> > 
> > diff --git a/common/format-output.c b/common/format-output.c
> > index 8df93ecb..f31e7259 100644
> > --- a/common/format-output.c
> > +++ b/common/format-output.c
> > @@ -213,6 +213,26 @@ void fmt_print_end_group(struct format_ctx *fctx, const char *name)
> >  	}
> >  }
> >  
> > +void fmt_print_start_object(struct format_ctx *fctx)
> > +{
> > +	if (bconf.output_format == CMD_FORMAT_JSON) {
> > +		fmt_separator(fctx);
> > +		fmt_inc_depth(fctx);
> > +		fctx->memb[fctx->depth] = 0;
> > +		putchar('{');
> > +	}
> > +}

Hi David. Thanks for review.
> 
> This duplicates what fmt_print_start_group and fmt_print_end_group do,
> so these should be extended to handle when 'name' is NULL and print only
> the [ or {.

I think it's good to extend fmt_print_start_group. I'll retry with it.

Thanks,
Sidong

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] btrfs-progs: device stats: add json output format
  2020-11-11 16:39 ` [PATCH v2 2/2] btrfs-progs: device stats: add json output format Sidong Yang
  2020-12-10 20:53   ` David Sterba
@ 2021-02-16 10:21   ` Filipe Manana
  1 sibling, 0 replies; 6+ messages in thread
From: Filipe Manana @ 2021-02-16 10:21 UTC (permalink / raw)
  To: Sidong Yang; +Cc: dsterba, linux-btrfs

On Wed, Nov 11, 2020 at 4:42 PM Sidong Yang <realwakka@gmail.com> 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
> devid                   1
> 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": [
>     {
>       "device": "/dev/vdb",
>       "devid": "1",
>       "write_io_errs": "0",
>       "read_io_errs": "0",
>       "flush_io_errs": "0",
>       "corruption_errs": "0",
>       "generation_errs": "0"
>     }
>   ],
> }
>
> Signed-off-by: Sidong Yang <realwakka@gmail.com>

Hi,

This breaks at least one test case from fstests:

$ ./check btrfs/006
FSTYP         -- btrfs
PLATFORM      -- Linux/x86_64 debian8 5.11.0-rc6-btrfs-next-80 #1 SMP
PREEMPT Wed Feb 3 11:28:05 WET 2021
MKFS_OPTIONS  -- /dev/sdc
MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1

btrfs/006 1s ... - output mismatch (see
/home/fdmanana/git/hub/xfstests/results//btrfs/006.out.bad)
    --- tests/btrfs/006.out 2020-06-10 19:29:03.810518987 +0100
    +++ /home/fdmanana/git/hub/xfstests/results//btrfs/006.out.bad
2021-02-16 10:18:53.967066620 +0000
    @@ -15,12 +15,14 @@

     == Sync filesystem
     == Show device stats by mountpoint
    + 1
     <NUMDEVS> [SCRATCH_DEV].corruption_errs <NUM>
     <NUMDEVS> [SCRATCH_DEV].flush_io_errs <NUM>
     <NUMDEVS> [SCRATCH_DEV].generation_errs <NUM>
    ...
    (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/btrfs/006.out
/home/fdmanana/git/hub/xfstests/results//btrfs/006.out.bad'  to see
the entire diff)
Ran: btrfs/006
Failures: btrfs/006
Failed 1 of 1 tests

That extra 1 is coming somewhere from this patch.


> ---
> v2:
>  - use json array for print
> ---
>  cmds/device.c | 33 +++++++++++++++++++++++++++------
>  1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/cmds/device.c b/cmds/device.c
> index d72881f8..8b8fc85c 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,17 @@ 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 = "devid", .fmt = "%u", .out_text = "devid", .out_json = "devid" },
> +       { .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 +494,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 +543,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_ARRAY);
>         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];
> @@ -548,6 +563,7 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
>                         err |= 1;
>                 } else {
>                         char *canonical_path;
> +                       char devid_str[32];
>                         int j;
>                         static const struct {
>                                 const char name[32];
> @@ -574,31 +590,36 @@ static int cmd_device_stats(const struct cmd_struct *cmd, int argc, char **argv)
>                                 snprintf(canonical_path, 32,
>                                          "devid:%llu", args.devid);
>                         }
> +                       snprintf(devid_str, 32, "%llu", args.devid);
> +                       fmt_print_start_object(&fctx);
> +                       fmt_print(&fctx, "device", canonical_path);
> +                       fmt_print(&fctx, "devid", di_args[i].devid);
>
>                         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_object(&fctx);
>                         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
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-02-16 10:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-11 16:39 [PATCH v2 1/2] btrfs-progs: common: introduce fmt_print_start_object Sidong Yang
2020-11-11 16:39 ` [PATCH v2 2/2] btrfs-progs: device stats: add json output format Sidong Yang
2020-12-10 20:53   ` David Sterba
2021-02-16 10:21   ` Filipe Manana
2020-12-10 20:28 ` [PATCH v2 1/2] btrfs-progs: common: introduce fmt_print_start_object David Sterba
2020-12-11 16:11   ` Sidong Yang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.