RE: [PATCH ndctl] test/daxctl-create.sh: remove region and dax device assumptions

From: Dan Williams <dan.j.williams@intel.com>
To: Vishal Verma <vishal.l.verma@intel.com>,
	<linux-cxl@vger.kernel.org>, <nvdimm@lists.linux.dev>
Cc: Dan Williams <dan.j.williams@intel.com>,
	Joao Martins <joao.m.martins@oracle.com>,
	Vishal Verma <vishal.l.verma@intel.com>
Subject: RE: [PATCH ndctl] test/daxctl-create.sh: remove region and dax device assumptions
Date: Thu, 11 Jan 2024 11:23:41 -0800	[thread overview]
Message-ID: <65a0403d151f9_5cee2944d@dwillia2-xfh.jf.intel.com.notmuch> (raw)
In-Reply-To: <20240110-vv-daxctl-create-v1-1-01f5d58afcd8@intel.com>

Vishal Verma wrote:
> The daxctl-create.sh test had some hard-coded assumptions about what dax
> device it expects to find, and what region number it will be under. This
> usually worked when the unit test environment only had efi_fake_mem
> devices as the sources of hmem memory. With CXL however, the region
> numbering namespace is shared with CXL regions, often pushing the
> efi_fake_mem region to something other than 'region0'.
> 
> Remove any region and device number assumptions from this test so it
> works regardless of how regions get enumerated.
> 
> Cc: Joao Martins <joao.m.martins@oracle.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> ---
>  test/daxctl-create.sh | 62 +++++++++++++++++++++++++++++----------------------
>  1 file changed, 35 insertions(+), 27 deletions(-)
> 
> diff --git a/test/daxctl-create.sh b/test/daxctl-create.sh
> index d319a39..a5df6f2 100755
> --- a/test/daxctl-create.sh
> +++ b/test/daxctl-create.sh
> @@ -29,14 +29,20 @@ find_testdev()
>  	fi
>  
>  	# find a victim region provided by dax_hmem
> -	testpath=$("$DAXCTL" list -r 0 | jq -er '.[0].path | .//""')
> +	region_json="$("$DAXCTL" list -R)"
> +	testpath=$(jq -er '.[0].path | .//""' <<< "$region_json")

This fixes the case where the first dax-region may not be region-id 0,
but would it also fail if the first region is not the "hmem" region?

I wonder if the above and the next line can be fixed with a jq select()
like?

    select(.path | contains("hmem"))

>  	if [[ ! "$testpath" == *"hmem"* ]]; then
>  		printf "Unable to find a victim region\n"
>  		exit "$rc"
>  	fi
> +	region_id=$(jq -er '.[0].id | .//""' <<< "$region_json")
> +	if [[ ! "$region_id" ]]; then
> +		printf "Unable to determine victim region id\n"
> +		exit "$rc"
> +	fi
>  
>  	# find a victim device
> -	testdev=$("$DAXCTL" list -D -r 0 | jq -er '.[0].chardev | .//""')
> +	testdev=$("$DAXCTL" list -D -r "$region_id" | jq -er '.[0].chardev | .//""')
>  	if [[ ! $testdev  ]]; then
>  		printf "Unable to find a victim device\n"
>  		exit "$rc"
> @@ -56,9 +62,10 @@ setup_dev()
>  		exit "$rc"
>  	fi
>  
> +	"$DAXCTL" reconfigure-device -m devdax -f "$testdev"
>  	"$DAXCTL" disable-device "$testdev"
>  	"$DAXCTL" reconfigure-device -s 0 "$testdev"
> -	available=$("$DAXCTL" list -r 0 | jq -er '.[0].available_size | .//""')
> +	available=$("$DAXCTL" list -r "$region_id" | jq -er '.[0].available_size | .//""')

These and the rest of the changes look good.

[..]