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

[Donet Tom <donettom@linux.ibm.com>]

On 2/19/25 22:40, Alison Schofield wrote:
> 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
Sure.
>
>
>> 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.
Sure. I will add.
>> 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;
Yes . This is the correct fix. I will add it and send V2.
>>   	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
>>