* [PATCH v3 1/2] btrfs-progs: common: extend fmt_print_start_group handles unnamed group @ 2020-12-11 16:48 Sidong Yang 2020-12-11 16:48 ` [PATCH v3 2/2] btrfs-progs: device stats: add json output format Sidong Yang 2020-12-11 17:31 ` [PATCH v3 1/2] btrfs-progs: common: extend fmt_print_start_group handles unnamed group David Sterba 0 siblings, 2 replies; 13+ messages in thread From: Sidong Yang @ 2020-12-11 16:48 UTC (permalink / raw) To: linux-btrfs, dsterba; +Cc: Sidong Yang This patch extends fmt_print_start_group() that it can handle when name argument is NULL. It is useful for printing unnamed array or map. Signed-off-by: Sidong Yang <realwakka@gmail.com> --- v3: - extend fmt_print_start_group rather than writing new function --- common/format-output.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/common/format-output.c b/common/format-output.c index 8df93ecb..2f74595c 100644 --- a/common/format-output.c +++ b/common/format-output.c @@ -181,17 +181,23 @@ void fmt_end_value(struct format_ctx *fctx, const struct rowspec *row) void fmt_print_start_group(struct format_ctx *fctx, const char *name, enum json_type jtype) { + char bracket; if (bconf.output_format == CMD_FORMAT_JSON) { fmt_separator(fctx); fmt_inc_depth(fctx); fctx->jtype[fctx->depth] = jtype; fctx->memb[fctx->depth] = 0; if (jtype == JSON_TYPE_MAP) - printf("\"%s\": {", name); + bracket = '{'; else if (jtype == JSON_TYPE_ARRAY) - printf("\"%s\": [", name); + bracket = '['; else fmt_error(fctx); + + if (name) + printf("\"%s\": %c", name, bracket); + else + putchar(bracket); } } -- 2.25.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v3 2/2] btrfs-progs: device stats: add json output format 2020-12-11 16:48 [PATCH v3 1/2] btrfs-progs: common: extend fmt_print_start_group handles unnamed group Sidong Yang @ 2020-12-11 16:48 ` Sidong Yang 2020-12-11 17:30 ` David Sterba 2020-12-11 17:31 ` [PATCH v3 1/2] btrfs-progs: common: extend fmt_print_start_group handles unnamed group David Sterba 1 sibling, 1 reply; 13+ messages in thread From: Sidong Yang @ 2020-12-11 16:48 UTC (permalink / raw) To: linux-btrfs, dsterba; +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> --- v3: - use fmt_print_start_group with NULL name --- cmds/device.c | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/cmds/device.c b/cmds/device.c index d72881f8..204e834b 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_group(&fctx, NULL, JSON_TYPE_MAP); + 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_group(&fctx, NULL); 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] 13+ messages in thread
* Re: [PATCH v3 2/2] btrfs-progs: device stats: add json output format 2020-12-11 16:48 ` [PATCH v3 2/2] btrfs-progs: device stats: add json output format Sidong Yang @ 2020-12-11 17:30 ` David Sterba 2020-12-11 17:46 ` David Sterba 0 siblings, 1 reply; 13+ messages in thread From: David Sterba @ 2020-12-11 17:30 UTC (permalink / raw) To: Sidong Yang; +Cc: linux-btrfs, dsterba On Fri, Dec 11, 2020 at 04:48:12PM +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. I did not realize that before, but we can't change the output like that. That would break fstests. > 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 But at least it's just one more switch that keeps the printf and json format inside the loop, the rest can stay. > 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" > } > ], ^ I've verified that the comma is really there, it's not a valid json so there's a bug in the formatter. To verify that the output is valid you can use eg. 'jq', simply pipe the output of the commadn there. $ ./btrfs --format json dev stats /mnt | jq parse error: Expected another key-value pair at line 16, column 1 > } > > Signed-off-by: Sidong Yang <realwakka@gmail.com> > --- > v3: > - use fmt_print_start_group with NULL name > --- > cmds/device.c | 33 +++++++++++++++++++++++++++------ > 1 file changed, 27 insertions(+), 6 deletions(-) > > diff --git a/cmds/device.c b/cmds/device.c > index d72881f8..204e834b 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", The usage help text should also advertise the formats, so this needs the helpinfo stubs: --- a/cmds/device.c +++ b/cmds/device.c @@ -527,6 +527,8 @@ static const char * const cmd_device_stats_usage[] = { "", "-c|--check return non-zero if any stat counter is not zero", "-z|--reset show current stats and reset values to zero", + HELPINFO_INSERT_GLOBALS, + HELPINFO_INSERT_FORMAT, NULL }; --- > @@ -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_group(&fctx, NULL, JSON_TYPE_MAP); > + 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]); So the switch goes here > if ((check == 1) > && (args.values[dev_stats[j].num] > 0)) > err |= 64; > } > - > + fmt_print_end_group(&fctx, NULL); > 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 [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] btrfs-progs: device stats: add json output format 2020-12-11 17:30 ` David Sterba @ 2020-12-11 17:46 ` David Sterba 2020-12-11 18:09 ` Sidong Yang 2020-12-16 6:30 ` Su Yue 0 siblings, 2 replies; 13+ messages in thread From: David Sterba @ 2020-12-11 17:46 UTC (permalink / raw) To: dsterba, Sidong Yang, linux-btrfs On Fri, Dec 11, 2020 at 06:30:25PM +0100, David Sterba wrote: > On Fri, Dec 11, 2020 at 04:48:12PM +0000, Sidong Yang wrote: > > 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" > > } > > ], > ^ > > I've verified that the comma is really there, it's not a valid json so > there's a bug in the formatter. To verify that the output is valid you > can use eg. 'jq', simply pipe the output of the commadn there. > > $ ./btrfs --format json dev stats /mnt | jq > parse error: Expected another key-value pair at line 16, column 1 I've pushed the updated plain text formatting to devel, so the only remaining bug is the above extra comma. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] btrfs-progs: device stats: add json output format 2020-12-11 17:46 ` David Sterba @ 2020-12-11 18:09 ` Sidong Yang 2020-12-16 17:23 ` David Sterba 2020-12-16 6:30 ` Su Yue 1 sibling, 1 reply; 13+ messages in thread From: Sidong Yang @ 2020-12-11 18:09 UTC (permalink / raw) To: dsterba, linux-btrfs On Fri, Dec 11, 2020 at 06:46:29PM +0100, David Sterba wrote: > On Fri, Dec 11, 2020 at 06:30:25PM +0100, David Sterba wrote: > > On Fri, Dec 11, 2020 at 04:48:12PM +0000, Sidong Yang wrote: > > > 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" > > > } > > > ], > > ^ > > > > I've verified that the comma is really there, it's not a valid json so > > there's a bug in the formatter. To verify that the output is valid you > > can use eg. 'jq', simply pipe the output of the commadn there. > > > > $ ./btrfs --format json dev stats /mnt | jq > > parse error: Expected another key-value pair at line 16, column 1 > > I've pushed the updated plain text formatting to devel, so the only > remaining bug is the above extra comma. I've tested devel branch. but there is no extra comma and I also tested with jq you said. jq results no error. I think that It's just mistype in message. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] btrfs-progs: device stats: add json output format 2020-12-11 18:09 ` Sidong Yang @ 2020-12-16 17:23 ` David Sterba 2020-12-16 17:41 ` David Sterba 0 siblings, 1 reply; 13+ messages in thread From: David Sterba @ 2020-12-16 17:23 UTC (permalink / raw) To: Sidong Yang; +Cc: dsterba, linux-btrfs On Fri, Dec 11, 2020 at 06:09:11PM +0000, Sidong Yang wrote: > On Fri, Dec 11, 2020 at 06:46:29PM +0100, David Sterba wrote: > > On Fri, Dec 11, 2020 at 06:30:25PM +0100, David Sterba wrote: > > > On Fri, Dec 11, 2020 at 04:48:12PM +0000, Sidong Yang wrote: > > > > 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" > > > > } > > > > ], > > > ^ > > > > > > I've verified that the comma is really there, it's not a valid json so > > > there's a bug in the formatter. To verify that the output is valid you > > > can use eg. 'jq', simply pipe the output of the commadn there. > > > > > > $ ./btrfs --format json dev stats /mnt | jq > > > parse error: Expected another key-value pair at line 16, column 1 > > > > I've pushed the updated plain text formatting to devel, so the only > > remaining bug is the above extra comma. > > I've tested devel branch. but there is no extra comma and I also tested > with jq you said. jq results no error. I think that It's just mistype in > message. That's strange because the formatter is buggy, eg. the simplest fmt_start()/fmt_end() leads to $ ./json-formatter-test { "__header": { "version": "1" }, } --- Which is of course invalid and jq confirms that. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] btrfs-progs: device stats: add json output format 2020-12-16 17:23 ` David Sterba @ 2020-12-16 17:41 ` David Sterba 0 siblings, 0 replies; 13+ messages in thread From: David Sterba @ 2020-12-16 17:41 UTC (permalink / raw) To: dsterba, Sidong Yang, linux-btrfs On Wed, Dec 16, 2020 at 06:23:50PM +0100, David Sterba wrote: > On Fri, Dec 11, 2020 at 06:09:11PM +0000, Sidong Yang wrote: > > On Fri, Dec 11, 2020 at 06:46:29PM +0100, David Sterba wrote: > > > On Fri, Dec 11, 2020 at 06:30:25PM +0100, David Sterba wrote: > > > > On Fri, Dec 11, 2020 at 04:48:12PM +0000, Sidong Yang wrote: > > > > > 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" > > > > > } > > > > > ], > > > > ^ > > > > > > > > I've verified that the comma is really there, it's not a valid json so > > > > there's a bug in the formatter. To verify that the output is valid you > > > > can use eg. 'jq', simply pipe the output of the commadn there. > > > > > > > > $ ./btrfs --format json dev stats /mnt | jq > > > > parse error: Expected another key-value pair at line 16, column 1 > > > > > > I've pushed the updated plain text formatting to devel, so the only > > > remaining bug is the above extra comma. > > > > I've tested devel branch. but there is no extra comma and I also tested > > with jq you said. jq results no error. I think that It's just mistype in > > message. > > That's strange because the formatter is buggy, eg. the simplest > fmt_start()/fmt_end() leads to > > $ ./json-formatter-test > { > "__header": { > "version": "1" > }, > } > --- > > Which is of course invalid and jq confirms that. The reason was a garbage value on the depth 0 of the stack that got interpreted as "there were values printed, insert the comma". That also explains why it worked for you. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] btrfs-progs: device stats: add json output format 2020-12-11 17:46 ` David Sterba 2020-12-11 18:09 ` Sidong Yang @ 2020-12-16 6:30 ` Su Yue 2020-12-16 10:52 ` Sidong Yang 2020-12-16 17:18 ` David Sterba 1 sibling, 2 replies; 13+ messages in thread From: Su Yue @ 2020-12-16 6:30 UTC (permalink / raw) To: dsterba, Sidong Yang, linux-btrfs On Sat, Dec 12, 2020 at 3:04 AM David Sterba <dsterba@suse.cz> wrote: > > On Fri, Dec 11, 2020 at 06:30:25PM +0100, David Sterba wrote: > > On Fri, Dec 11, 2020 at 04:48:12PM +0000, Sidong Yang wrote: > > > 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" > > > } > > > ], > > ^ > > > > I've verified that the comma is really there, it's not a valid json so > > there's a bug in the formatter. To verify that the output is valid you > > can use eg. 'jq', simply pipe the output of the commadn there. > > > > $ ./btrfs --format json dev stats /mnt | jq > > parse error: Expected another key-value pair at line 16, column 1 > > I've pushed the updated plain text formatting to devel, so the only > remaining bug is the above extra comma. Another format bug(one extra newline): ======================================== ➜ btrfs-progs git:(314d96c8) btrfs device stats /mnt [/dev/mapper/test-1].write_io_errs 0 [/dev/mapper/test-1].read_io_errs 0 [/dev/mapper/test-1].flush_io_errs 0 [/dev/mapper/test-1].corruption_errs 0 [/dev/mapper/test-1].generation_errs 0 ➜ btrfs-progs git:(314d96c8) ======================================== The new line is printed by the change: '+ fmt_end(&fctx);' and fstests/btrfs/006 fails: ================================================================================ btrfs/006 1s ... - output mismatch (see /root/xfstests-dev/results//btrfs/006.out.bad) --- tests/btrfs/006.out 2020-12-16 03:40:19.632039261 +0000 +++ /root/xfstests-dev/results//btrfs/006.out.bad 2020-12-16 06:25:56.424424113 +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 /root/xfstests-dev/tests/btrfs/006.out /root/xfstests-dev/results//btrfs/006.out.bad' to see the entire diff) Ran: btrfs/006 Failures: btrfs/006 Failed 1 of 1 tests ================================================================================ The new line made filter produce the '+1'. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] btrfs-progs: device stats: add json output format 2020-12-16 6:30 ` Su Yue @ 2020-12-16 10:52 ` Sidong Yang 2020-12-16 12:52 ` Su Yue 2020-12-16 17:18 ` David Sterba 1 sibling, 1 reply; 13+ messages in thread From: Sidong Yang @ 2020-12-16 10:52 UTC (permalink / raw) To: Su Yue; +Cc: dsterba, linux-btrfs On Wed, Dec 16, 2020 at 02:30:04PM +0800, Su Yue wrote: > On Sat, Dec 12, 2020 at 3:04 AM David Sterba <dsterba@suse.cz> wrote: > > > > On Fri, Dec 11, 2020 at 06:30:25PM +0100, David Sterba wrote: > > > On Fri, Dec 11, 2020 at 04:48:12PM +0000, Sidong Yang wrote: > > > > 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" > > > > } > > > > ], > > > ^ > > > > > > I've verified that the comma is really there, it's not a valid json so > > > there's a bug in the formatter. To verify that the output is valid you > > > can use eg. 'jq', simply pipe the output of the commadn there. > > > > > > $ ./btrfs --format json dev stats /mnt | jq > > > parse error: Expected another key-value pair at line 16, column 1 > > > > I've pushed the updated plain text formatting to devel, so the only > > remaining bug is the above extra comma. > > Another format bug(one extra newline): > ======================================== > ➜ btrfs-progs git:(314d96c8) btrfs device stats /mnt > [/dev/mapper/test-1].write_io_errs 0 > [/dev/mapper/test-1].read_io_errs 0 > [/dev/mapper/test-1].flush_io_errs 0 > [/dev/mapper/test-1].corruption_errs 0 > [/dev/mapper/test-1].generation_errs 0 > > ➜ btrfs-progs git:(314d96c8) > ======================================== > The new line is printed by the change: > '+ fmt_end(&fctx);' > > and fstests/btrfs/006 fails: > ================================================================================ > btrfs/006 1s ... - output mismatch (see > /root/xfstests-dev/results//btrfs/006.out.bad) > --- tests/btrfs/006.out 2020-12-16 03:40:19.632039261 +0000 > +++ /root/xfstests-dev/results//btrfs/006.out.bad 2020-12-16 > 06:25:56.424424113 +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 /root/xfstests-dev/tests/btrfs/006.out > /root/xfstests-dev/results//btrfs/006.out.bad' to see the entire > diff) > Ran: btrfs/006 > Failures: btrfs/006 > Failed 1 of 1 tests > ================================================================================ > > The new line made filter produce the '+1'. Thanks for testing this patch. I checked the fmt_end() and there is an additional newline. I think that fmt_end() should be used for formatting. so it seems that the only way to fix this problem is to remove the code that inserts a newline in fmt_end(). I searched the code that use the function and there is no code that used this function but this patch. Do you have any ideas? Thanks, Sidong ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] btrfs-progs: device stats: add json output format 2020-12-16 10:52 ` Sidong Yang @ 2020-12-16 12:52 ` Su Yue 2020-12-16 17:21 ` David Sterba 0 siblings, 1 reply; 13+ messages in thread From: Su Yue @ 2020-12-16 12:52 UTC (permalink / raw) To: Sidong Yang; +Cc: dsterba, linux-btrfs On Wed, Dec 16, 2020 at 6:52 PM Sidong Yang <realwakka@gmail.com> wrote: > > On Wed, Dec 16, 2020 at 02:30:04PM +0800, Su Yue wrote: > > On Sat, Dec 12, 2020 at 3:04 AM David Sterba <dsterba@suse.cz> wrote: > > > > > > On Fri, Dec 11, 2020 at 06:30:25PM +0100, David Sterba wrote: > > > > On Fri, Dec 11, 2020 at 04:48:12PM +0000, Sidong Yang wrote: > > > > > 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" > > > > > } > > > > > ], > > > > ^ > > > > > > > > I've verified that the comma is really there, it's not a valid json so > > > > there's a bug in the formatter. To verify that the output is valid you > > > > can use eg. 'jq', simply pipe the output of the commadn there. > > > > > > > > $ ./btrfs --format json dev stats /mnt | jq > > > > parse error: Expected another key-value pair at line 16, column 1 > > > > > > I've pushed the updated plain text formatting to devel, so the only > > > remaining bug is the above extra comma. > > > > Another format bug(one extra newline): > > ======================================== > > ➜ btrfs-progs git:(314d96c8) btrfs device stats /mnt > > [/dev/mapper/test-1].write_io_errs 0 > > [/dev/mapper/test-1].read_io_errs 0 > > [/dev/mapper/test-1].flush_io_errs 0 > > [/dev/mapper/test-1].corruption_errs 0 > > [/dev/mapper/test-1].generation_errs 0 > > > > ➜ btrfs-progs git:(314d96c8) > > ======================================== > > The new line is printed by the change: > > '+ fmt_end(&fctx);' > > > > and fstests/btrfs/006 fails: > > ================================================================================ > > btrfs/006 1s ... - output mismatch (see > > /root/xfstests-dev/results//btrfs/006.out.bad) > > --- tests/btrfs/006.out 2020-12-16 03:40:19.632039261 +0000 > > +++ /root/xfstests-dev/results//btrfs/006.out.bad 2020-12-16 > > 06:25:56.424424113 +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 /root/xfstests-dev/tests/btrfs/006.out > > /root/xfstests-dev/results//btrfs/006.out.bad' to see the entire > > diff) > > Ran: btrfs/006 > > Failures: btrfs/006 > > Failed 1 of 1 tests > > ================================================================================ > > > > The new line made filter produce the '+1'. > > Thanks for testing this patch. > I checked the fmt_end() and there is an additional newline. > I think that fmt_end() should be used for formatting. so it seems that Yes, it's for the purpose of formatting. > the only way to fix this problem is to remove the code that inserts a > newline in fmt_end(). I searched the code that use the function and > there is no code that used this function but this patch. Do you have any > ideas? > I'm OK about removing the "putchar('\n');". It's just a tiny format issue so no bother to do extra works. David? > Thanks, > Sidong ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] btrfs-progs: device stats: add json output format 2020-12-16 12:52 ` Su Yue @ 2020-12-16 17:21 ` David Sterba 0 siblings, 0 replies; 13+ messages in thread From: David Sterba @ 2020-12-16 17:21 UTC (permalink / raw) To: Su Yue; +Cc: Sidong Yang, dsterba, linux-btrfs On Wed, Dec 16, 2020 at 08:52:08PM +0800, Su Yue wrote: > > > The new line made filter produce the '+1'. > > > > Thanks for testing this patch. > > I checked the fmt_end() and there is an additional newline. > > I think that fmt_end() should be used for formatting. so it seems that > > Yes, it's for the purpose of formatting. > > > the only way to fix this problem is to remove the code that inserts a > > newline in fmt_end(). I searched the code that use the function and > > there is no code that used this function but this patch. Do you have any > > ideas? > > > I'm OK about removing the "putchar('\n');". It's just a tiny format issue so no > bother to do extra works. The way the json formatting works the newline must be printed from there. The problematic requirement is delayed insertion of the "," when there are more objects on the same level. But we don't know that until the next object is processed. Mixing the textual and json output for the stats makes it a bit more complicated as it prints the newline for each row unconditionally, breaking the assumption of the formatter. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/2] btrfs-progs: device stats: add json output format 2020-12-16 6:30 ` Su Yue 2020-12-16 10:52 ` Sidong Yang @ 2020-12-16 17:18 ` David Sterba 1 sibling, 0 replies; 13+ messages in thread From: David Sterba @ 2020-12-16 17:18 UTC (permalink / raw) To: Su Yue; +Cc: dsterba, Sidong Yang, linux-btrfs On Wed, Dec 16, 2020 at 02:30:04PM +0800, Su Yue wrote: > On Sat, Dec 12, 2020 at 3:04 AM David Sterba <dsterba@suse.cz> wrote: > > > > On Fri, Dec 11, 2020 at 06:30:25PM +0100, David Sterba wrote: > > > On Fri, Dec 11, 2020 at 04:48:12PM +0000, Sidong Yang wrote: > > > > 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" > > > > } > > > > ], > > > ^ > > > > > > I've verified that the comma is really there, it's not a valid json so > > > there's a bug in the formatter. To verify that the output is valid you > > > can use eg. 'jq', simply pipe the output of the commadn there. > > > > > > $ ./btrfs --format json dev stats /mnt | jq > > > parse error: Expected another key-value pair at line 16, column 1 > > > > I've pushed the updated plain text formatting to devel, so the only > > remaining bug is the above extra comma. > > Another format bug(one extra newline): > ======================================== > ➜ btrfs-progs git:(314d96c8) btrfs device stats /mnt > [/dev/mapper/test-1].write_io_errs 0 > [/dev/mapper/test-1].read_io_errs 0 > [/dev/mapper/test-1].flush_io_errs 0 > [/dev/mapper/test-1].corruption_errs 0 > [/dev/mapper/test-1].generation_errs 0 > > ➜ btrfs-progs git:(314d96c8) > ======================================== > The new line is printed by the change: > '+ fmt_end(&fctx);' > > and fstests/btrfs/006 fails: > ================================================================================ > btrfs/006 1s ... - output mismatch (see > /root/xfstests-dev/results//btrfs/006.out.bad) > --- tests/btrfs/006.out 2020-12-16 03:40:19.632039261 +0000 > +++ /root/xfstests-dev/results//btrfs/006.out.bad 2020-12-16 > 06:25:56.424424113 +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 /root/xfstests-dev/tests/btrfs/006.out > /root/xfstests-dev/results//btrfs/006.out.bad' to see the entire > diff) > Ran: btrfs/006 > Failures: btrfs/006 > Failed 1 of 1 tests > ================================================================================ > > The new line made filter produce the '+1'. I noticed the newline but did not think it would break fstests, so it needs to be removed. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/2] btrfs-progs: common: extend fmt_print_start_group handles unnamed group 2020-12-11 16:48 [PATCH v3 1/2] btrfs-progs: common: extend fmt_print_start_group handles unnamed group Sidong Yang 2020-12-11 16:48 ` [PATCH v3 2/2] btrfs-progs: device stats: add json output format Sidong Yang @ 2020-12-11 17:31 ` David Sterba 1 sibling, 0 replies; 13+ messages in thread From: David Sterba @ 2020-12-11 17:31 UTC (permalink / raw) To: Sidong Yang; +Cc: linux-btrfs, dsterba On Fri, Dec 11, 2020 at 04:48:11PM +0000, Sidong Yang wrote: > This patch extends fmt_print_start_group() that it can handle when name > argument is NULL. It is useful for printing unnamed array or map. > > Signed-off-by: Sidong Yang <realwakka@gmail.com> > --- > v3: > - extend fmt_print_start_group rather than writing new function > --- > common/format-output.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/common/format-output.c b/common/format-output.c > index 8df93ecb..2f74595c 100644 > --- a/common/format-output.c > +++ b/common/format-output.c > @@ -181,17 +181,23 @@ void fmt_end_value(struct format_ctx *fctx, const struct rowspec *row) > void fmt_print_start_group(struct format_ctx *fctx, const char *name, > enum json_type jtype) > { > + char bracket; > if (bconf.output_format == CMD_FORMAT_JSON) { > fmt_separator(fctx); > fmt_inc_depth(fctx); > fctx->jtype[fctx->depth] = jtype; > fctx->memb[fctx->depth] = 0; This can be simplified a bit, the name can be conditionally printed here > if (jtype == JSON_TYPE_MAP) > - printf("\"%s\": {", name); > + bracket = '{'; and this just does the right putchar(). With this change now added to devel, thanks. > else if (jtype == JSON_TYPE_ARRAY) > - printf("\"%s\": [", name); > + bracket = '['; > else > fmt_error(fctx); > + > + if (name) > + printf("\"%s\": %c", name, bracket); > + else > + putchar(bracket); ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-12-16 17:43 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-12-11 16:48 [PATCH v3 1/2] btrfs-progs: common: extend fmt_print_start_group handles unnamed group Sidong Yang 2020-12-11 16:48 ` [PATCH v3 2/2] btrfs-progs: device stats: add json output format Sidong Yang 2020-12-11 17:30 ` David Sterba 2020-12-11 17:46 ` David Sterba 2020-12-11 18:09 ` Sidong Yang 2020-12-16 17:23 ` David Sterba 2020-12-16 17:41 ` David Sterba 2020-12-16 6:30 ` Su Yue 2020-12-16 10:52 ` Sidong Yang 2020-12-16 12:52 ` Su Yue 2020-12-16 17:21 ` David Sterba 2020-12-16 17:18 ` David Sterba 2020-12-11 17:31 ` [PATCH v3 1/2] btrfs-progs: common: extend fmt_print_start_group handles unnamed group David Sterba
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.