Re: [PATCH] ndctl: json: Region capabilities are not displayed if any of the BTT, PFN, or DAX are not present

[Alison Schofield <alison.schofield@intel.com>]

On Wed, Feb 19, 2025 at 03:40:49AM -0600, Donet Tom wrote:


Thanks Tom!

Please v2 with patch prefix and commit msg of:
[ndctl PATCH v2] ndctl/list: display region caps for any of BTT, PFN, DAX


> If any one of BTT, PFN, or DAX is not present, but the other two
> are, then the region capabilities are not displayed in the
> ndctl list -R -C command.
> 
> This is because util_region_capabilities_to_json() returns NULL
> if any one of BTT, PFN, or DAX is not present.
> 
> In this patch, we have changed the logic to display all the region
> capabilities that are present.
> 
> Test Results with CONFIG_BTT disabled
> =====================================
> Without this patch
> ------------------
>  # ./build/ndctl/ndctl  list -R -C
>  [
>   {
>     "dev":"region1",
>     "size":549755813888,
>     "align":16777216,
>     "available_size":549755813888,
>     "max_available_extent":549755813888,
>     "type":"pmem",
>     "iset_id":11510624209454722969,
>     "persistence_domain":"memory_controller"
>   },
> 
> With this patch
> ---------------
>  # ./build/ndctl/ndctl  list -R -C
>  [
>   {
>     "dev":"region1",
>     "size":549755813888,
>     "align":16777216,
>     "available_size":549755813888,
>     "max_available_extent":549755813888,
>     "type":"pmem",
>     "iset_id":11510624209454722969,
>     "capabilities":[
>       {
>         "mode":"fsdax",
>         "alignments":[
>           65536,
>           16777216
>         ]
>       },
>       {
>         "mode":"devdax",
>         "alignments":[
>           65536,
>           16777216
>         ]
>       }
>     ],
>     "persistence_domain":"memory_controller"
>   },
> 

Please add a formatted fixes tag.
Double check, but I believe this was introduced with commit 965fa02e372f,
util: Distribute 'filter' and 'json' helpers to per-tool objects
It seems we broke it in ndctl release v73.


> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
> ---
>  ndctl/json.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/ndctl/json.c b/ndctl/json.c
> index 23bad7f..3df3bc4 100644
> --- a/ndctl/json.c
> +++ b/ndctl/json.c
> @@ -381,9 +381,6 @@ struct json_object *util_region_capabilities_to_json(struct ndctl_region *region
>  	struct ndctl_pfn *pfn = ndctl_region_get_pfn_seed(region);
>  	struct ndctl_dax *dax = ndctl_region_get_dax_seed(region);
>  
> -	if (!btt || !pfn || !dax)
> -		return NULL;
> -

How about a one line change that avoids getting the jcaps array
needlessly:

	if (!btt && !pfn && !dax)
		return NULL;

>  	jcaps = json_object_new_array();
>  	if (!jcaps)
>  		return NULL;
> @@ -436,7 +433,8 @@ struct json_object *util_region_capabilities_to_json(struct ndctl_region *region
>  		json_object_object_add(jcap, "alignments", jobj);
>  	}
>  
> -	return jcaps;
> +	if (btt || pfn || dax)
> +		return jcaps;
>  err:
>  	json_object_put(jcaps);
>  	return NULL;
> -- 
> 2.43.5
>