On Tue, 2019-02-12 at 13:29 -0800, Dan Williams wrote:
> Namespaces may contain an info-block that correlates with the possible
> modes of a namespace: 'sector', 'fsdax', or 'devdax'. Provide a command
> that can temporarily convert the namespace into 'raw' mode to read the
> info-block.
> 
> Also, similar to 'read-labels' provide a facility to decode the
> info-block into json.

Nice, I like this!

> 
> Signed-off-by: Dan Williams <[email protected]>
> ---
>  ndctl/action.h    |    1 
>  ndctl/builtin.h   |    1 
>  ndctl/check.c     |   20 ---
>  ndctl/namespace.c |  401 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  ndctl/namespace.h |   51 +++++++
>  ndctl/ndctl.c     |    1 
>  util/fletcher.h   |    1 
>  util/size.h       |    1 
>  8 files changed, 456 insertions(+), 21 deletions(-)

[..]

>  
> +#define READ_INFOBLOCK_OPTIONS() \
> +OPT_FILENAME('o', "output", &param.outfile, "output-file", \
> +     "filename to write label area contents"), \

copy/paste staleness, should be "write infoblock contents"

> +OPT_FILENAME('f', "file", &param.infile, "input-file", \
> +     "filename to readinfo block instead of a namespace"), \

Perhaps make this consistent with the label commands, and call the file
option -i/--input. This also frees up -f for --force, which might be
nice to have since the namespaces are expected to be disabled (That's
only a minor convenience thing though - I'm equally OK leaving out  the
--force for now)

> +OPT_BOOLEAN('n', "no-verify", &param.no_verify, \
> +     "skip parent uuid, and checksum validation"), \

Since the option handling framework automatically provides a no-* option
for boolean options, should we call this --verify, default to on, and
then document the availability of the automatic --no-verify option?

Currently, this ends up in an automatic --no-no-verify option :)

> +OPT_BOOLEAN('j', "json", &param.json, "parse label data into json"), \
> +OPT_BOOLEAN('u', "human", &param.human, "use human friendly number formats ")
> +

[..]

> +     if (mode == ACTION_READ_INFOBLOCK && param.infile && argc) {
> +             error("specify a namespace, or --file, not both\n");
> +             rc = -EINVAL;
> +     }
> +

Similar comment as read-labels about auto enabling --json if --human
supplied standalone.



_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Reply via email to