[ndctl PATCH 1/2] ndctl: Add 'list' verbose options

[ndctl PATCH 1/2] ndctl: Add 'list' verbose options

[Keith Busch]

The informational and miscellaneous flag options are becoming more
numerous, and can be difficult to remember what can be listed. Rather
than add more flags to suppress and show some of the less pertinent
information, this patch adds a 'verbose' option that increases the detail
of information displayed.

The verbose option can be repeated multiple times to increase the
detail. There are currently three levels of verbose, and repeating more
than that has the same behavior as level three.

Signed-off-by: Keith Busch <keith.busch@intel.com>
---
 ndctl/list.c | 25 ++++++++++++++++++++++++-
 util/json.c  | 33 +++++++++++++++++++--------------
 util/json.h  |  1 +
 3 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/ndctl/list.c b/ndctl/list.c
index 6cf7c39..98e5b3d 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -36,6 +36,7 @@ static struct {
 	bool media_errors;
 	bool human;
 	bool firmware;
+	int verbose;
 } list;
 
 static unsigned long listopts_to_flags(void)
@@ -50,6 +51,8 @@ static unsigned long listopts_to_flags(void)
 		flags |= UTIL_JSON_DAX | UTIL_JSON_DAX_DEVS;
 	if (list.human)
 		flags |= UTIL_JSON_HUMAN;
+	if (list.verbose)
+		flags |= UTIL_JSON_VERBOSE;
 	return flags;
 }
 
@@ -112,7 +115,7 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 	numa = ndctl_region_get_numa_node(region);
 	if (numa >= 0) {
 		jobj = json_object_new_int(numa);
-		if (jobj)
+		if (jobj && flags & UTIL_JSON_VERBOSE)
 			json_object_object_add(jregion, "numa_node", jobj);
 	}
 
@@ -444,6 +447,8 @@ int cmd_list(int argc, const char **argv, void *ctx)
 				"include media errors"),
 		OPT_BOOLEAN('u', "human", &list.human,
 				"use human friendly number formats "),
+		OPT_INCR('v', "verbose", &list.verbose,
+				"increase output detail"),
 		OPT_END(),
 	};
 	const char * const u[] = {
@@ -468,6 +473,24 @@ int cmd_list(int argc, const char **argv, void *ctx)
 			param.mode = "dax";
 	}
 
+	switch (list.verbose) {
+	default:
+	case 3:
+		list.idle = true;
+		list.firmware = true;
+	case 2:
+		list.buses = true;
+		list.regions = true;
+	case 1:
+		list.dimms = true;
+		list.health = true;
+		list.media_errors = true;
+		list.namespaces = true;
+		list.dax = true;
+	case 0:
+		break;
+	}
+
 	if (num_list_flags() == 0)
 		list.namespaces = true;
 
diff --git a/util/json.c b/util/json.c
index 1772177..a9f028b 100644
--- a/util/json.c
+++ b/util/json.c
@@ -660,6 +660,21 @@ static struct json_object *util_raw_uuid(struct ndctl_namespace *ndns)
 	return json_object_new_string(buf);
 }
 
+static void util_raw_uuid_to_json(struct ndctl_namespace *ndns,
+				  unsigned long flags,
+				  struct json_object *jndns)
+{
+	struct json_object *jobj;
+
+	if (!(flags & UTIL_JSON_VERBOSE))
+		return;
+
+	jobj = util_raw_uuid(ndns);
+	if (!jobj)
+		return;
+	json_object_object_add(jndns, "raw_uuid", jobj);
+}
+
 struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 		unsigned long flags)
 {
@@ -732,11 +747,7 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 		if (!jobj)
 			goto err;
 		json_object_object_add(jndns, "uuid", jobj);
-
-		jobj = util_raw_uuid(ndns);
-		if (!jobj)
-			goto err;
-		json_object_object_add(jndns, "raw_uuid", jobj);
+		util_raw_uuid_to_json(ndns, flags, jndns);
 		bdev = ndctl_btt_get_block_device(btt);
 	} else if (pfn) {
 		ndctl_pfn_get_uuid(pfn, uuid);
@@ -745,10 +756,7 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 		if (!jobj)
 			goto err;
 		json_object_object_add(jndns, "uuid", jobj);
-		jobj = util_raw_uuid(ndns);
-		if (!jobj)
-			goto err;
-		json_object_object_add(jndns, "raw_uuid", jobj);
+		util_raw_uuid_to_json(ndns, flags, jndns);
 		bdev = ndctl_pfn_get_block_device(pfn);
 	} else if (dax) {
 		struct daxctl_region *dax_region;
@@ -760,10 +768,7 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 		if (!jobj)
 			goto err;
 		json_object_object_add(jndns, "uuid", jobj);
-		jobj = util_raw_uuid(ndns);
-		if (!jobj)
-			goto err;
-		json_object_object_add(jndns, "raw_uuid", jobj);
+		util_raw_uuid_to_json(ndns, flags, jndns);
 		if ((flags & UTIL_JSON_DAX) && dax_region) {
 			jobj = util_daxctl_region_to_json(dax_region, NULL,
 					flags);
@@ -810,7 +815,7 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
 	 * happens because they use pre-v1.2 labels or because they
 	 * don't have a label space (devtype=nd_namespace_io).
 	 */
-	if (sector_size < UINT_MAX) {
+	if (sector_size < UINT_MAX && flags & UTIL_JSON_VERBOSE) {
 		jobj = json_object_new_int(sector_size);
 		if (!jobj)
 			goto err;
diff --git a/util/json.h b/util/json.h
index c5d1603..17e9320 100644
--- a/util/json.h
+++ b/util/json.h
@@ -23,6 +23,7 @@ enum util_json_flags {
 	UTIL_JSON_DAX = (1 << 2),
 	UTIL_JSON_DAX_DEVS = (1 << 3),
 	UTIL_JSON_HUMAN = (1 << 4),
+	UTIL_JSON_VERBOSE = (1 << 5),
 };
 
 struct json_object;
-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[ndctl PATCH 2/2] ndctl: Add 'list' verbose documentation

[Keith Busch]

Signed-off-by: Keith Busch <keith.busch@intel.com>
---
 Documentation/ndctl/ndctl-list.txt | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/ndctl/ndctl-list.txt b/Documentation/ndctl/ndctl-list.txt
index 13ebdcd..791638c 100644
--- a/Documentation/ndctl/ndctl-list.txt
+++ b/Documentation/ndctl/ndctl-list.txt
@@ -200,6 +200,18 @@ include::xable-region-options.txt[]
   ]
 }
 
+-v::
+--verbose::
+	Increase verbosity of the output. This can be specified
+	multiple times to be even more verbose on the informational and
+	miscellaneous output, and can be used to override the flags for
+	showing specific information.
+
+[verse]
+ndctl list -v
+ndctl list -vv
+ndctl list -vvv
+
 include::human-option.txt[]
 
 ----
-- 
2.14.3

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 1/2] ndctl: Add 'list' verbose options

[Dan Williams]

On Tue, Jun 26, 2018 at 1:46 PM, Keith Busch <keith.busch@intel.com> wrote:
> The informational and miscellaneous flag options are becoming more
> numerous, and can be difficult to remember what can be listed. Rather
> than add more flags to suppress and show some of the less pertinent
> information, this patch adds a 'verbose' option that increases the detail
> of information displayed.
>
> The verbose option can be repeated multiple times to increase the
> detail. There are currently three levels of verbose, and repeating more
> than that has the same behavior as level three.
>
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
>  ndctl/list.c | 25 ++++++++++++++++++++++++-
>  util/json.c  | 33 +++++++++++++++++++--------------
>  util/json.h  |  1 +
>  3 files changed, 44 insertions(+), 15 deletions(-)
>
> diff --git a/ndctl/list.c b/ndctl/list.c
> index 6cf7c39..98e5b3d 100644
> --- a/ndctl/list.c
> +++ b/ndctl/list.c
> @@ -36,6 +36,7 @@ static struct {
>         bool media_errors;
>         bool human;
>         bool firmware;
> +       int verbose;
>  } list;
>
>  static unsigned long listopts_to_flags(void)
> @@ -50,6 +51,8 @@ static unsigned long listopts_to_flags(void)
>                 flags |= UTIL_JSON_DAX | UTIL_JSON_DAX_DEVS;
>         if (list.human)
>                 flags |= UTIL_JSON_HUMAN;
> +       if (list.verbose)
> +               flags |= UTIL_JSON_VERBOSE;
>         return flags;
>  }
>
> @@ -112,7 +115,7 @@ static struct json_object *region_to_json(struct ndctl_region *region,
>         numa = ndctl_region_get_numa_node(region);
>         if (numa >= 0) {
>                 jobj = json_object_new_int(numa);
> -               if (jobj)
> +               if (jobj && flags & UTIL_JSON_VERBOSE)
>                         json_object_object_add(jregion, "numa_node", jobj);
>         }
>
> @@ -444,6 +447,8 @@ int cmd_list(int argc, const char **argv, void *ctx)
>                                 "include media errors"),
>                 OPT_BOOLEAN('u', "human", &list.human,
>                                 "use human friendly number formats "),
> +               OPT_INCR('v', "verbose", &list.verbose,
> +                               "increase output detail"),
>                 OPT_END(),
>         };
>         const char * const u[] = {
> @@ -468,6 +473,24 @@ int cmd_list(int argc, const char **argv, void *ctx)
>                         param.mode = "dax";
>         }
>
> +       switch (list.verbose) {
> +       default:
> +       case 3:
> +               list.idle = true;
> +               list.firmware = true;
> +       case 2:
> +               list.buses = true;
> +               list.regions = true;
> +       case 1:
> +               list.dimms = true;
> +               list.health = true;

This turns on listing DIMMs even if --dimms was not specified on the
command line. I was thinking not until level 2 do we start turning on
more device objects beyond what is specified. For example "ndctl list
-v" is the same as "ndctl list" that lists namespaces by default.
However it goes further to turn on extra / namespace-relative data
like 'raw_uuid' and 'sector_size' but also data like "--media-errors"
and "--dax". Then it follows commands like "ndctl list -Dv" list the
DIMMs and turns on optional fields like 'handle' and 'phys_id' in
addition to "--health" information.

> +               list.media_errors = true;
> +               list.namespaces = true;
> +               list.dax = true;
> +       case 0:
> +               break;
> +       }
> +
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 1/2] ndctl: Add 'list' verbose options

[Keith Busch]

On Tue, Jun 26, 2018 at 02:11:30PM -0700, Dan Williams wrote:
> This turns on listing DIMMs even if --dimms was not specified on the
> command line. I was thinking not until level 2 do we start turning on
> more device objects beyond what is specified. For example "ndctl list
> -v" is the same as "ndctl list" that lists namespaces by default.
> However it goes further to turn on extra / namespace-relative data
> like 'raw_uuid' and 'sector_size' but also data like "--media-errors"
> and "--dax". Then it follows commands like "ndctl list -Dv" list the
> DIMMs and turns on optional fields like 'handle' and 'phys_id' in
> addition to "--health" information.

Aha, that makes more sense. Thanks for the info.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 2/2] ndctl: Add 'list' verbose documentation

[Verma, Vishal L]

On Tue, 2018-06-26 at 14:46 -0600, Keith Busch wrote:
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
>  Documentation/ndctl/ndctl-list.txt | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 

I think this can all just go in the first patch?

> diff --git a/Documentation/ndctl/ndctl-list.txt
> b/Documentation/ndctl/ndctl-list.txt
> index 13ebdcd..791638c 100644
> --- a/Documentation/ndctl/ndctl-list.txt
> +++ b/Documentation/ndctl/ndctl-list.txt
> @@ -200,6 +200,18 @@ include::xable-region-options.txt[]
>    ]
>  }
>  
> +-v::
> +--verbose::
> +	Increase verbosity of the output. This can be specified
> +	multiple times to be even more verbose on the informational and
> +	miscellaneous output, and can be used to override the flags for
> +	showing specific information.

Should we detail here what verbosity level enables which flags? Or is that
too much upkeep in case things change..

> +
> +[verse]
> +ndctl list -v
> +ndctl list -vv
> +ndctl list -vvv
> +
>  include::human-option.txt[]
>  
>  ----
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 2/2] ndctl: Add 'list' verbose documentation

[Dan Williams]

On Tue, Jun 26, 2018 at 3:59 PM, Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
> On Tue, 2018-06-26 at 14:46 -0600, Keith Busch wrote:
>> Signed-off-by: Keith Busch <keith.busch@intel.com>
>> ---
>>  Documentation/ndctl/ndctl-list.txt | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>
> I think this can all just go in the first patch?
>
>> diff --git a/Documentation/ndctl/ndctl-list.txt
>> b/Documentation/ndctl/ndctl-list.txt
>> index 13ebdcd..791638c 100644
>> --- a/Documentation/ndctl/ndctl-list.txt
>> +++ b/Documentation/ndctl/ndctl-list.txt
>> @@ -200,6 +200,18 @@ include::xable-region-options.txt[]
>>    ]
>>  }
>>
>> +-v::
>> +--verbose::
>> +     Increase verbosity of the output. This can be specified
>> +     multiple times to be even more verbose on the informational and
>> +     miscellaneous output, and can be used to override the flags for
>> +     showing specific information.
>
> Should we detail here what verbosity level enables which flags? Or is that
> too much upkeep in case things change..

Hopefully does not change too much. We've had some contention about
which fields are available in the listing, so it would be nice to give
a heads up when people say "hey! where did my numa node and
sector_size fields go?".
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm