Re: [PATCH 2/2] scrub status: add json output format

[David Sterba <dsterba@suse.cz>]

On Fri, Feb 07, 2025 at 04:33:02AM +0200, Racz Zoltan wrote:
> This patch adds support for json formatting of the "scrub status"
> command. Please not that in the info section the started-at key in
> 02:00:00 1970 because i bypassed the "no stats available" so I can make
> sure those stats are correctly formatted in the output as well. 
> 
> Example usage:
> 1. btrfs --format json scrub status /

Thanks. The status in json is useful and it found a few things than may
be missing in the json formatting. The most obvious one is that there's
too much duplication of the code in plain vs json output. The ideal
version is that there's only rowspec definition of all the keys and only
fmt_print for each one, it'll get formatted properly given the selected
output format.

But there are already exceptions in other code that prints both json and
plain text due to the requirements that can't be done with fmt_ but we
need to keep the visual output.


> +static const struct rowspec scrub_status_rowspec[] = {
> +	{ .key = "uuid", .fmt = "%s", .out_json = "uuid"},
> +	{ .key = "status", .fmt = "%s", .out_json = "status"},
> +	{ .key = "duration", .fmt = "%u:%s", .out_json = "duration"},

We'll need a new internal json type for dration, so the value is number
of seconds and formatted automatically. The .fmt can be any proper
printf formatter but it's left for flexibility until we find a reason to
make a separate type for that to avoid code repetition or differences
how the same type of information is formatted.

> +	{ .key = "started_at", .fmt = "%s", .out_json = "started-at"},

And another type for timestamp, input in seconds, formatted as some
standard human readable format that can be parsed back eventually.

> +	{ .key = "resumed_at", .fmt = "%s", .out_json = "resumed-at"},
> +	{ .key = "data_extents_scrubbed", .fmt = "%lld", .out_json = "data-extents-scrubbed"},

The keys are internal, I'd prefer to use "-" as separator.

> +	{ .key = "tree_extents_scrubbed", .fmt = "%lld", .out_json = "tree-extents-scrubbed"},
> +	{ .key = "data_bytes_scrubbed", .fmt = "%lld", .out_json = "data-bytes-scrubbed"},
> +	{ .key = "tree_bytes_scrubbed", .fmt = "%lld", .out_json = "tree-bytes-scrubbed"},
> +	{ .key = "read_errors", .fmt = "%lld", .out_json = "read-errors"},
> +	{ .key = "csum_errors", .fmt = "%lld", .out_json = "csum-errors"},
> +	{ .key = "verify_errors", .fmt = "%lld", .out_json = "verify-errors"},
> +	{ .key = "no_csum", .fmt = "%lld", .out_json = "no-csum"},
> +	{ .key = "csum_discards", .fmt = "%lld", .out_json = "csum-discards"},
> +	{ .key = "super_errors", .fmt = "%lld", .out_json = "super-errors"},
> +	{ .key = "malloc_errors", .fmt = "%lld", .out_json = "malloc-errors"},
> +	{ .key = "uncorrectable_errors", .fmt = "%lld", .out_json = "uncorrectable-errors"},
> +	{ .key = "unverified_errors", .fmt = "%lld", .out_json = "unverified-errors"},
> +	{ .key = "corrected_errors", .fmt = "%lld", .out_json = "corrected-errors"},
> +	{ .key = "last_physical", .fmt = "%lld", .out_json = "last-physical"},

All the numbers seem to be u64, so %llu format should be there but it's
wrong in current version already. This would be nice to fix first (in a
separate patch).

> +	{ .key = "time_left", .fmt = "%llu:%02llu:%02llu", .out_json = "time-left"},

Duration again, also it may need to be formatted with days taken into
account. With a filesystem it's not impossible.

> +	{ .key = "eta", .fmt = "%s", .out_json = "eta"},

Timestamp type.

> +	{ .key = "total_bytes_to_scrub", .fmt = "%lld", .out_json = "total-bytes-to-scrub"},
> +	{ .key = "bytes_scrubbed", .fmt = "%lld", .out_json = "bytes-scrubbed"},
> +	{ .key = "rate", .fmt = "%lld", .out_json = "rate"},
> +	{ .key = "limit", .fmt = "%lld", .out_json = "limit"},
> +
> +	ROWSPEC_END
> +};

So the plan for now is to first update the formatter and then use it for
scrub status in json. Let me know if you're up for it. Adding the types
should be easy, it's in fmt_print().