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

[Filipe Manana <fdmanana@gmail.com>]

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.”