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

[Sidong Yang <realwakka@gmail.com>]

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