diff mbox series

[blktests,1/2] zbd/rc: Support dm-crypt

Message ID 20210713101239.269789-2-shinichiro.kawasaki@wdc.com (mailing list archive)
State New, archived
Headers show
Series zbd: Support dm-crypt | expand

Commit Message

Shinichiro Kawasaki July 13, 2021, 10:12 a.m. UTC
Linux kernel 5.9 added zoned block device support to dm-crypt. To test
dm-crypt devices, modify the function _get_dev_container_and_sector().
To handle device-mapper table format difference between dm-crypt and
dm-linear/flakey, add dev_idx and off_idx local variables.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 tests/zbd/rc | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

Comments

Bart Van Assche July 13, 2021, 1:59 p.m. UTC | #1
On 7/13/21 3:12 AM, Shin'ichiro Kawasaki wrote:
> -	# Parse dm table lines for dm-linear or dm-flakey target
> +	if _test_dev_has_dm_map crypt; then
> +		dev_idx=6
> +		off_idx=7
> +	fi
> +
> +	# Parse dm table lines for dm-linear, dm-flakey or dm-crypt target
>  	while read -r -a tbl_line; do
>  		local -i map_start=${tbl_line[0]}
>  		local -i map_end=$((tbl_line[0] + tbl_line[1]))
> @@ -355,10 +362,11 @@ _get_dev_container_and_sector() {
>  			continue
>  		fi
>  
> -		offset=${tbl_line[4]}
> -		if ! cont_dev=$(_get_dev_path_by_id "${tbl_line[3]}"); then
> +		offset=${tbl_line[off_idx]}
> +		if ! cont_dev=$(_get_dev_path_by_id \
> +					"${tbl_line[dev_idx]}"); then
>  			echo -n "Cannot access to container device: "
> -			echo "${tbl_line[3]}"
> +			echo "${tbl_line[dev_idx]}"
>  			return 1
>  		fi

To me the above code looks like code that is hard to maintain. Can the
above parser be replaced by reading /sys/block/*/slaves? An example from
my workstation for dm-crypt:

$ ls /sys/block/dm-0/slaves
nvme0n1p2

Thanks,

Bart.
Shinichiro Kawasaki July 14, 2021, 11:13 a.m. UTC | #2
On Jul 13, 2021 / 06:59, Bart Van Assche wrote:
> On 7/13/21 3:12 AM, Shin'ichiro Kawasaki wrote:
> > -	# Parse dm table lines for dm-linear or dm-flakey target
> > +	if _test_dev_has_dm_map crypt; then
> > +		dev_idx=6
> > +		off_idx=7
> > +	fi
> > +
> > +	# Parse dm table lines for dm-linear, dm-flakey or dm-crypt target
> >  	while read -r -a tbl_line; do
> >  		local -i map_start=${tbl_line[0]}
> >  		local -i map_end=$((tbl_line[0] + tbl_line[1]))
> > @@ -355,10 +362,11 @@ _get_dev_container_and_sector() {
> >  			continue
> >  		fi
> >  
> > -		offset=${tbl_line[4]}
> > -		if ! cont_dev=$(_get_dev_path_by_id "${tbl_line[3]}"); then
> > +		offset=${tbl_line[off_idx]}
> > +		if ! cont_dev=$(_get_dev_path_by_id \
> > +					"${tbl_line[dev_idx]}"); then
> >  			echo -n "Cannot access to container device: "
> > -			echo "${tbl_line[3]}"
> > +			echo "${tbl_line[dev_idx]}"
> >  			return 1
> >  		fi
> 
> To me the above code looks like code that is hard to maintain. Can the
> above parser be replaced by reading /sys/block/*/slaves? An example from
> my workstation for dm-crypt:
> 
> $ ls /sys/block/dm-0/slaves
> nvme0n1p2

Hi, Bart. Thanks for the suggestion, but I don't think sysfs slaves attribute
will simplify the code. The helper function _get_dev_container_and_sector()
requires pairs of 'map starting sector offset' and 'map target device'. Dm
table provides both of them, but the sysfs slaves attribute provides only the
map target device. As far as I understand, the helper function must parse the
dm table anyway to get the map starting sector offsets, and paired devices.
diff mbox series

Patch

diff --git a/tests/zbd/rc b/tests/zbd/rc
index 1237363..9deadc1 100644
--- a/tests/zbd/rc
+++ b/tests/zbd/rc
@@ -327,6 +327,7 @@  _get_dev_container_and_sector() {
 	local cont_dev
 	local -i offset
 	local -a tbl_line
+	local -i dev_idx=3 off_idx=4
 
 	if _test_dev_is_partition; then
 		offset=$(<"${TEST_DEV_PART_SYSFS}/start")
@@ -340,13 +341,19 @@  _get_dev_container_and_sector() {
 		return 1
 	fi
 	if ! _test_dev_has_dm_map linear &&
-			! _test_dev_has_dm_map flakey; then
-		echo -n "dm mapping test other than linear/flakey is"
+			! _test_dev_has_dm_map flakey &&
+			! _test_dev_has_dm_map crypt; then
+		echo -n "dm mapping test other than linear/flakey/crypt is"
 		echo "not implemented"
 		return 1
 	fi
 
-	# Parse dm table lines for dm-linear or dm-flakey target
+	if _test_dev_has_dm_map crypt; then
+		dev_idx=6
+		off_idx=7
+	fi
+
+	# Parse dm table lines for dm-linear, dm-flakey or dm-crypt target
 	while read -r -a tbl_line; do
 		local -i map_start=${tbl_line[0]}
 		local -i map_end=$((tbl_line[0] + tbl_line[1]))
@@ -355,10 +362,11 @@  _get_dev_container_and_sector() {
 			continue
 		fi
 
-		offset=${tbl_line[4]}
-		if ! cont_dev=$(_get_dev_path_by_id "${tbl_line[3]}"); then
+		offset=${tbl_line[off_idx]}
+		if ! cont_dev=$(_get_dev_path_by_id \
+					"${tbl_line[dev_idx]}"); then
 			echo -n "Cannot access to container device: "
-			echo "${tbl_line[3]}"
+			echo "${tbl_line[dev_idx]}"
 			return 1
 		fi