Re: [PATCH ndctl] cxl: Add list all option to the cxl command

["Verma, Vishal L" <vishal.l.verma@intel.com>]

On Thu, 2022-07-14 at 11:27 -0700, sunfishho12@gmail.com wrote:
> From: Matthew Ho <sunfishho12@gmail.com>
> 
> This adds a new subcommand cxl list --all, which is equivalent to cxl
> list -MBPEDTHIiu. This addition makes it easier to list all the CXL
> devices at once, as one does not need to append a subcommand for each
> device. --all is also easier to remember than -MBPEDTHIiu. When
> region support is added, this will be updated to include it.
> 
> Reviewed-by: Adam Manzanares <a.manzanares@samsung.com>
> Signed-off-by: Matthew Ho <sunfishho12@gmail.com>
> ---
>  cxl/filter.h |  1 +
>  cxl/list.c   | 15 +++++++++++++++
>  2 files changed, 16 insertions(+)

Hi Matthew, thanks for sending this - I agree it gets tedious trying to
list multiple things.

However, I think we should do this similar to ndctl-list's --verbose
options, where adding the number of 'v's adds in more and more detail.

For example:

cxl list (default) : Regions and memdevs (this isn't there today, but
in a pending series I'm about to send out).

cxl list -v: -RMBDPT

cxl list -vv: -RMBDPTi

cxl list -vvv: -RMBDPTiHI

-u/--human can be excluded from the verbosity levels as that can be
passed in if needed, and there may be use cases where it isn't desired
(in scripts).

Thoughts on this - do you want to take a shot at implementing it this
way?

> 
> diff --git a/cxl/filter.h b/cxl/filter.h
> index 697b7779c08e..7009c00a1c23 100644
> --- a/cxl/filter.h
> +++ b/cxl/filter.h
> @@ -13,6 +13,7 @@ struct cxl_filter_params {
>         const char *port_filter;
>         const char *endpoint_filter;
>         const char *decoder_filter;
> +       bool all;
>         bool single;
>         bool endpoints;
>         bool decoders;
> diff --git a/cxl/list.c b/cxl/list.c
> index 1e9d441190a0..738282e95434 100644
> --- a/cxl/list.c
> +++ b/cxl/list.c
> @@ -50,6 +50,8 @@ static const struct option options[] = {
>                     "include memory device health information "),
>         OPT_BOOLEAN('I', "partition", &param.partition,
>                     "include memory device partition information "),
> +       OPT_BOOLEAN(0, "all", &param.all,
> +                   "include info on all components of the CXL
> hierarchy"),
>  #ifdef ENABLE_DEBUG
>         OPT_BOOLEAN(0, "debug", &debug, "debug list walk"),
>  #endif
> @@ -82,6 +84,19 @@ int cmd_list(int argc, const char **argv, struct
> cxl_ctx *ctx)
>                 usage_with_options(u, options);
>         }
> 
> +       if (param.all){
> +               param.memdevs = true;
> +               param.buses = true;
> +               param.ports = true;
> +               param.endpoints = true;
> +               param.decoders = true;
> +               param.partition = true;
> +               param.health = true;
> +               param.targets = true;
> +               param.human = true;
> +               param.idle = true;
> +       }
> +
>         if (num_list_flags() == 0) {
>                 if (param.memdev_filter || param.serial_filter)
>                         param.memdevs = true;
> --
> 2.34.1
>