[PATCH 00/19] mdadm/tests: enhance/fix regression cases

[PATCH 00/19] mdadm/tests: enhance/fix regression cases

[Xiao Ni]

Hi all

This is the first set which has fixed and enhanced some cases. I'll
go on fixing/enhancing the following cases.

Xiao Ni (19):
  Change some error messages to info level
  mdadm: Start update_opt from 0
  Don't control reshape speed in daemon
  mdadm/tests: test enhance
  mdadm/tests: test don't fail when systemd reports error
  mdadm/tests: names_template enhance
  mdadm/tests: 03assem-incr enhance
  mdadm/tests 03r0assem enhance
  mdadm/tests: remove 03r5assem-failed
  mdadm/tests: 03r5assemV1
  mdadm/tests: remove 04r5swap.broken
  tests/04update-metadata skip linear
  mdadm/tests: 05r5-internalbitmap
  mdadm/tests: 05r6-bitmapfile
  mdadm/tests: 06name enhance
  mdadm/tests: 07autoassemble
  mdadm/tests: 07autodetect.broken can be removed
  mdadm/tests: 07changelevelintr
  mdadm/tests: disable selinux

 Assemble.c                     | 16 ++++-------
 Grow.c                         |  7 -----
 Manage.c                       |  2 +-
 mdadm.h                        |  4 +--
 test                           | 28 +++++++++++++++----
 tests/03assem-incr             | 20 ++++++++-----
 tests/03r0assem                | 10 -------
 tests/03r5assem-failed         | 12 --------
 tests/03r5assemV1              | 17 ------------
 tests/04r5swap.broken          |  7 -----
 tests/04update-metadata        | 35 +++++++++++++----------
 tests/05r5-internalbitmap      | 21 ++++++--------
 tests/05r6-bitmapfile          | 21 ++++++--------
 tests/06name                   | 10 +++++--
 tests/07autoassemble           | 37 ++++++++++++++++++++++--
 tests/07autoassemble.broken    |  8 ------
 tests/07autodetect.broken      |  5 ----
 tests/07changelevelintr        |  9 +++---
 tests/07changelevelintr.broken |  9 ------
 tests/func.sh                  | 51 ++++++++++++++++++++++++++++++++--
 tests/templates/names_template | 14 ++++++++--
 util.c                         |  4 +--
 22 files changed, 192 insertions(+), 155 deletions(-)
 delete mode 100644 tests/03r5assem-failed
 delete mode 100644 tests/04r5swap.broken
 delete mode 100644 tests/07autoassemble.broken
 delete mode 100644 tests/07autodetect.broken
 delete mode 100644 tests/07changelevelintr.broken

-- 
2.32.0 (Apple Git-132)

[PATCH 01/19] Change some error messages to info level

[Xiao Ni]

These logs are not error logs. Change them to info level.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 Assemble.c | 16 ++++++----------
 Manage.c   |  2 +-
 util.c     |  4 ++--
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/Assemble.c b/Assemble.c
index 83dced19ceba..65cdb737382a 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -1209,23 +1209,19 @@ static int start_array(int mdfd,
 		if (rv == 0) {
 			sysfs_rules_apply(mddev, content);
 			if (c->verbose >= 0) {
-				pr_err("%s has been started with %d drive%s",
+				pr_info("%s has been started with %d drive%s",
 				       mddev, okcnt, okcnt==1?"":"s");
 				if (okcnt < (unsigned)content->array.raid_disks)
-					fprintf(stderr, " (out of %d)",
-						content->array.raid_disks);
+					printf(" (out of %d)", content->array.raid_disks);
 				if (rebuilding_cnt)
-					fprintf(stderr, "%s %d rebuilding",
-						sparecnt?",":" and",
+					printf("%s %d rebuilding", sparecnt?",":" and",
 						rebuilding_cnt);
 				if (sparecnt)
-					fprintf(stderr, " and %d spare%s",
-						sparecnt,
+					printf(" and %d spare%s", sparecnt,
 						sparecnt == 1 ? "" : "s");
 				if (content->journal_clean)
-					fprintf(stderr, " and %d journal",
-						journalcnt);
-				fprintf(stderr, ".\n");
+					printf(" and %d journal", journalcnt);
+				printf(".\n");
 			}
 			if (content->reshape_active &&
 			    is_level456(content->array.level)) {
diff --git a/Manage.c b/Manage.c
index 96e5ee5427a2..5db72b778fbe 100644
--- a/Manage.c
+++ b/Manage.c
@@ -463,7 +463,7 @@ done:
 	}
 
 	if (verbose >= 0)
-		pr_err("stopped %s\n", devname);
+		pr_info("stopped %s\n", devname);
 	map_lock(&map);
 	map_remove(&map, devnm);
 	map_unlock(&map);
diff --git a/util.c b/util.c
index bf79742fe44e..48c97545a42a 100644
--- a/util.c
+++ b/util.c
@@ -633,9 +633,9 @@ int check_ext2(int fd, char *name)
 	bsize = sb[24]|(sb[25]|(sb[26]|sb[27]<<8)<<8)<<8;
 	size = sb[4]|(sb[5]|(sb[6]|sb[7]<<8)<<8)<<8;
 	size <<= bsize;
-	pr_err("%s appears to contain an ext2fs file system\n",
+	pr_info("%s appears to contain an ext2fs file system\n",
 		name);
-	cont_err("size=%lluK  mtime=%s", size, ctime(&mtime));
+	pr_info("size=%lluK  mtime=%s", size, ctime(&mtime));
 	return 1;
 }
 
-- 
2.32.0 (Apple Git-132)

[PATCH 02/19] mdadm: Start update_opt from 0

[Xiao Ni]

Before f2e8393bd722 ('Manage&Incremental: code refactor, string to enum'), it uses
NULL to represent it doesn't need to update. So init UOPT_UNDEFINED to 0. This
problem is found by test case 05r6tor0.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 mdadm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mdadm.h b/mdadm.h
index b71d7b32ee07..40818941bb16 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -535,7 +535,8 @@ enum special_options {
 };
 
 enum update_opt {
-	UOPT_NAME = 1,
+	UOPT_UNDEFINED = 0,
+	UOPT_NAME,
 	UOPT_PPL,
 	UOPT_NO_PPL,
 	UOPT_BITMAP,
@@ -575,7 +576,6 @@ enum update_opt {
 	UOPT_SPEC_FAILFAST,
 	UOPT_SPEC_NOFAILFAST,
 	UOPT_SPEC_REVERT_RESHAPE_NOBACKUP,
-	UOPT_UNDEFINED
 };
 extern void fprint_update_options(FILE *outf, enum update_opt update_mode);
 
-- 
2.32.0 (Apple Git-132)

[PATCH 03/19] Don't control reshape speed in daemon

[Xiao Ni]

It tries to set the max sync speed in reshape. This should be done by
administrators by control interfaces /proc/sys/dev/raid/speed_limit_max/min.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 Grow.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/Grow.c b/Grow.c
index 1923c27c4274..01bbb338cb1b 100644
--- a/Grow.c
+++ b/Grow.c
@@ -4484,7 +4484,6 @@ int child_monitor(int afd, struct mdinfo *sra, struct reshape *reshape,
 	 */
 	char *buf;
 	int degraded = -1;
-	unsigned long long speed;
 	unsigned long long suspend_point, array_size;
 	unsigned long long backup_point, wait_point;
 	unsigned long long reshape_completed;
@@ -4540,10 +4539,6 @@ int child_monitor(int afd, struct mdinfo *sra, struct reshape *reshape,
 	if (posix_memalign((void**)&buf, 4096, disks * chunk))
 		/* Don't start the 'reshape' */
 		return 0;
-	if (reshape->before.data_disks == reshape->after.data_disks) {
-		sysfs_get_ll(sra, NULL, "sync_speed_min", &speed);
-		sysfs_set_num(sra, NULL, "sync_speed_min", 200000);
-	}
 
 	if (increasing) {
 		array_size = sra->component_size * reshape->after.data_disks;
@@ -4676,8 +4671,6 @@ int child_monitor(int afd, struct mdinfo *sra, struct reshape *reshape,
 	sysfs_set_num(sra, NULL, "suspend_lo", 0);
 	sysfs_set_num(sra, NULL, "sync_min", 0);
 
-	if (reshape->before.data_disks == reshape->after.data_disks)
-		sysfs_set_num(sra, NULL, "sync_speed_min", speed);
 	free(buf);
 	return done;
 }
-- 
2.32.0 (Apple Git-132)

[PATCH 04/19] mdadm/tests: test enhance

[Xiao Ni]

There are two changes.
First, if md module is not loaded, it gives error when reading
speed_limit_max. So read the value after loading md module which
is done in do_setup

Second, sometimes the test reports error sync action doesn't
happen. But dmesg shows sync action is done. So limit the sync
speed before test. It doesn't affect the test run time. Because
check wait sets the max speed before waiting sync action. And
recording speed_limit_max/min in do_setup.

Fixes: 4c12714d1ca0 ('test: run tests on system level mdadm')
Signed-off-by: Xiao Ni <xni@redhat.com>
---
 test          | 10 +++++-----
 tests/func.sh | 26 +++++++++++++++++++++++---
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/test b/test
index 338c2db44fa7..ff403293d60b 100755
--- a/test
+++ b/test
@@ -6,7 +6,10 @@ targetdir="/var/tmp"
 logdir="$targetdir"
 config=/tmp/mdadm.conf
 testdir=$PWD/tests
-system_speed_limit=`cat /proc/sys/dev/raid/speed_limit_max`
+system_speed_limit_max=0
+system_speed_limit_min=0
+test_speed_limit_min=100
+test_speed_limit_max=500
 devlist=
 
 savelogs=0
@@ -39,10 +42,6 @@ ctrl_c() {
 	ctrl_c_error=1
 }
 
-restore_system_speed_limit() {
-	echo $system_speed_limit > /proc/sys/dev/raid/speed_limit_max
-}
-
 mdadm() {
 	rm -f $targetdir/stderr
 	case $* in
@@ -103,6 +102,7 @@ do_test() {
 		do_clean
 		# source script in a subshell, so it has access to our
 		# namespace, but cannot change it.
+		control_system_speed_limit
 		echo -ne "$_script... "
 		if ( set -ex ; . $_script ) &> $targetdir/log
 		then
diff --git a/tests/func.sh b/tests/func.sh
index b474442b6abe..221cff158f8c 100644
--- a/tests/func.sh
+++ b/tests/func.sh
@@ -136,6 +136,23 @@ check_env() {
 	fi
 }
 
+record_system_speed_limit() {
+	system_speed_limit_max=`cat /proc/sys/dev/raid/speed_limit_max`
+	system_speed_limit_min=`cat /proc/sys/dev/raid/speed_limit_min`
+}
+
+# To avoid sync action finishes before checking it, it needs to limit
+# the sync speed
+control_system_speed_limit() {
+	echo $test_speed_limit_min > /proc/sys/dev/raid/speed_limit_min
+	echo $test_speed_limit_max > /proc/sys/dev/raid/speed_limit_max
+}
+
+restore_system_speed_limit() {
+	echo $system_speed_limit_min > /proc/sys/dev/raid/speed_limit_max
+	echo $system_speed_limit_max > /proc/sys/dev/raid/speed_limit_max
+}
+
 do_setup() {
 	trap cleanup 0 1 3 15
 	trap ctrl_c 2
@@ -214,6 +231,7 @@ do_setup() {
 	ulimit -c unlimited
 	[ -f /proc/mdstat ] || modprobe md_mod
 	echo 0 > /sys/module/md_mod/parameters/start_ro
+	record_system_speed_limit
 }
 
 # check various things
@@ -265,15 +283,17 @@ check() {
 		fi
 	;;
 	wait )
-		p=`cat /proc/sys/dev/raid/speed_limit_max`
-		echo 2000000 > /proc/sys/dev/raid/speed_limit_max
+		min=`cat /proc/sys/dev/raid/speed_limit_min`
+		max=`cat /proc/sys/dev/raid/speed_limit_max`
+		echo 200000 > /proc/sys/dev/raid/speed_limit_max
 		sleep 0.1
 		while grep -Eq '(resync|recovery|reshape|check|repair) *=' /proc/mdstat ||
 			grep -v idle > /dev/null /sys/block/md*/md/sync_action
 		do
 			sleep 0.5
 		done
-		echo $p > /proc/sys/dev/raid/speed_limit_max
+		echo $min > /proc/sys/dev/raid/speed_limit_min
+		echo $max > /proc/sys/dev/raid/speed_limit_max
 	;;
 	state )
 		grep -sq "blocks.*\[$2\]\$" /proc/mdstat ||
-- 
2.32.0 (Apple Git-132)

[PATCH 05/19] mdadm/tests: test don't fail when systemd reports error

[Xiao Ni]

Sometimes systemd reports error in dmesg and test fails. Add
a condition to avoid this failure.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 test | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test b/test
index ff403293d60b..3da53f871122 100755
--- a/test
+++ b/test
@@ -109,7 +109,7 @@ do_test() {
 			if [ -f "${_script}.inject_error" ]; then
 				echo "dmesg checking is skipped because test inject error"
 			else
-				dmesg | grep -iq "error\|call trace\|segfault" &&
+				dmesg | grep -iq "error\|call trace\|segfault" | grep -v "systemd" &&
 					die "dmesg prints errors when testing $_basename!"
 			fi
 			echo "succeeded"
-- 
2.32.0 (Apple Git-132)

[PATCH 06/19] mdadm/tests: names_template enhance

[Xiao Ni]

For super1, if the length of hostname is >= 32, it doesn't add hostname
in metadata name. Fix this problem by checking the length of hostname.
Because other cases may use need to check this, so do the check in
do_setup.

And this patch adds a check if link /dev/md/name exists.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 test                           |  5 +++++
 tests/func.sh                  | 13 +++++++++++++
 tests/templates/names_template | 14 ++++++++++++--
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/test b/test
index 3da53f871122..814ce1992b0c 100755
--- a/test
+++ b/test
@@ -11,6 +11,11 @@ system_speed_limit_min=0
 test_speed_limit_min=100
 test_speed_limit_max=500
 devlist=
+# If super1 metadata name doesn't have the same hostname with machine,
+# it's treated as foreign.
+# For example, /dev/md0 is created, stops it, then assemble it, the
+# device node will be /dev/md127 (127 is choosed by mdadm autumatically)
+is_foreign="no"
 
 savelogs=0
 exitonerror=1
diff --git a/tests/func.sh b/tests/func.sh
index 221cff158f8c..cfe83e552a2a 100644
--- a/tests/func.sh
+++ b/tests/func.sh
@@ -153,6 +153,18 @@ restore_system_speed_limit() {
 	echo $system_speed_limit_max > /proc/sys/dev/raid/speed_limit_max
 }
 
+is_raid_foreign() {
+
+	# If the length of hostname is >= 32, super1 doesn't use
+	# hostname in metadata
+	hostname=$(hostname)
+	if [ `expr length $(hostname)` -lt 32 ]; then
+		is_foreign="no"
+	else
+		is_foreign="yes"
+	fi
+}
+
 do_setup() {
 	trap cleanup 0 1 3 15
 	trap ctrl_c 2
@@ -232,6 +244,7 @@ do_setup() {
 	[ -f /proc/mdstat ] || modprobe md_mod
 	echo 0 > /sys/module/md_mod/parameters/start_ro
 	record_system_speed_limit
+	is_raid_foreign
 }
 
 # check various things
diff --git a/tests/templates/names_template b/tests/templates/names_template
index 1b6cd14bf51d..88ad5b8c6b38 100644
--- a/tests/templates/names_template
+++ b/tests/templates/names_template
@@ -30,6 +30,7 @@ function names_verify() {
 	local DEVNODE_NAME="$1"
 	local WANTED_LINK="$2"
 	local WANTED_NAME="$3"
+	local EXPECTED=""
 
 	local RES="$(mdadm -D --export $DEVNODE_NAME | grep MD_DEVNAME)"
 	if [[ "$?" != "0" ]]; then
@@ -38,7 +39,12 @@ function names_verify() {
 	fi
 
 	if [[ "$WANTED_LINK" != "empty" ]]; then
-		local EXPECTED="MD_DEVNAME=$WANTED_LINK"
+		EXPECTED="MD_DEVNAME=$WANTED_LINK"
+
+		if [ ! -b /dev/md/$WANTED_LINK ]; then
+			echo "/dev/md/$WANTED_LINK doesn't exit"
+			exit 1
+		fi
 	fi
 
 	if [[ "$RES" != "$EXPECTED" ]]; then
@@ -52,7 +58,11 @@ function names_verify() {
 		exit 1
 	fi
 
-	local EXPECTED="MD_NAME=$(hostname):$WANTED_NAME"
+	if [ $is_foreign == "no" ]; then
+		EXPECTED="MD_NAME=$(hostname):$WANTED_NAME"
+	else
+		EXPECTED="MD_NAME=$WANTED_NAME"
+	fi
 	if [[ "$RES" != "$EXPECTED" ]]; then
 		echo "$RES doesn't match $EXPECTED."
 		exit 1
-- 
2.32.0 (Apple Git-132)

[PATCH 07/19] mdadm/tests: 03assem-incr enhance

[Xiao Ni]

It fails when hostname lenght > 32. Because the super1 metadata name
doesn't include hostname when hostname length > 32. Then mdadm thinks
the array is a foreign array if no device link is specified when
assembling the array. It chooses a minor number from 127.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 test               |  3 +++
 tests/03assem-incr | 20 +++++++++++++-------
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/test b/test
index 814ce1992b0c..1fce6be2c4a9 100755
--- a/test
+++ b/test
@@ -33,6 +33,9 @@ LVM_VOLGROUP=mdtest
 md0=/dev/md0
 md1=/dev/md1
 md2=/dev/md2
+# if user doesn't specify minor number, mdadm chooses minor number
+# automatically from 127.
+md127=/dev/md127
 mdp0=/dev/md_d0
 mdp1=/dev/md_d1
 
diff --git a/tests/03assem-incr b/tests/03assem-incr
index 38880a7fed10..21215a34f93b 100644
--- a/tests/03assem-incr
+++ b/tests/03assem-incr
@@ -9,15 +9,21 @@ set -x -e
 levels=(raid0 raid1 raid5)
 
 if [ "$LINEAR" == "yes" ]; then
-  levels+=( linear )
+	levels+=( linear )
 fi
 
 for l in ${levels[@]}
 do
-  mdadm -CR $md0 -l $l -n5 $dev0 $dev1 $dev2 $dev3 $dev4 --assume-clean
-  mdadm -S md0
-  mdadm -I $dev1
-  mdadm -I $dev3
-  mdadm -A /dev/md0  $dev0 $dev1 $dev2 $dev3 $dev4
-  mdadm -S /dev/md0
+	mdadm -CR $md0 -l $l -n5 $dev0 $dev1 $dev2 $dev3 $dev4 --assume-clean
+	mdadm -S $md0
+	mdadm -I $dev1
+	mdadm -I $dev3
+	mdadm -A $md0  $dev0 $dev1 $dev2 $dev3 $dev4
+	# If one array is foreign (metadata name doesn't have the machine's
+	# hostname), mdadm chooses a minor number automatically from 127
+	if [ $is_foreign == "no" ]; then
+		mdadm -S $md0
+	else
+		mdadm -S $md127
+	fi
 done
-- 
2.32.0 (Apple Git-132)

[PATCH 08/19] mdadm/tests 03r0assem enhance

[Xiao Ni]

dcc22ae74a864 ('super1: remove support for name= in config') already
removes name= support. So remove related test codes in 03r0assem.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 tests/03r0assem | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/tests/03r0assem b/tests/03r0assem
index f7c29e8c1ab6..4bf8b9e83b0f 100644
--- a/tests/03r0assem
+++ b/tests/03r0assem
@@ -33,16 +33,6 @@ mdadm -As -c $conf $md2
 $tst
 mdadm -S $md2
 
-{
-  echo DEVICE $devlist
-  echo array $md2 name=2
-} > $conf
-
-mdadm -As -c $conf $md2
-$tst
-mdadm -S $md2
-
-
 {
   echo DEVICE $devlist
   echo array $md2 devices=$dev0,$dev1,$dev2
-- 
2.32.0 (Apple Git-132)

[PATCH 09/19] mdadm/tests: remove 03r5assem-failed

[Xiao Ni]

03r5assem can run successfully with kernel 6.9.0-rc4

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 tests/03r5assem-failed | 12 ------------
 1 file changed, 12 deletions(-)
 delete mode 100644 tests/03r5assem-failed

diff --git a/tests/03r5assem-failed b/tests/03r5assem-failed
deleted file mode 100644
index d38241df0228..000000000000
--- a/tests/03r5assem-failed
+++ /dev/null
@@ -1,12 +0,0 @@
-
-# Create an array, fail one device while array is active, stop array,
-# then re-assemble listing the failed device first.
-
-mdadm -CR $md1 -l5 -n4 $dev0 $dev1 $dev2 $dev3
-check wait
-
-echo 2000 > /sys/block/md1/md/safe_mode_delay
-mkfs $md1
-mdadm $md1 -f $dev0
-mdadm -S $md1
-mdadm -A $md1 $dev0 $dev1 $dev2 $dev3 || exit 1
-- 
2.32.0 (Apple Git-132)

[PATCH 10/19] mdadm/tests: 03r5assemV1

[Xiao Ni]

dcc22ae74a864 ('super1: remove support for name= in config') already
removes name= support. So remove related test codes in 03r5assemV1.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 tests/03r5assemV1 | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/tests/03r5assemV1 b/tests/03r5assemV1
index bca0c583724b..6026011eda93 100644
--- a/tests/03r5assemV1
+++ b/tests/03r5assemV1
@@ -31,14 +31,6 @@ conf=$targetdir/mdadm.conf
 mdadm -As -c $conf $md1
 eval $tst
 
-{
-  echo DEVICE $devlist
-  echo array $md1 name=one
-} > $conf
-
-mdadm -As -c $conf
-eval $tst
-
 {
   echo DEVICE $devlist
   echo array $md1 devices=$dev0,$dev1,$dev2,$dev3,$dev4
@@ -88,15 +80,6 @@ mdadm -As -c $conf $md1
 check state U_U
 eval $tst
 
-{
-  echo DEVICE $devlist
-  echo array $md1 name=one
-} > $conf
-
-mdadm -As -c $conf
-check state U_U
-eval $tst
-
 {
   echo DEVICE $devlist
   echo array $md1 devices=$dev0,$dev1,$dev2
-- 
2.32.0 (Apple Git-132)

[PATCH 11/19] mdadm/tests: remove 04r5swap.broken

[Xiao Ni]

04r5swap can run successfully with kernel 6.9.0-rc4

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 tests/04r5swap.broken | 7 -------
 1 file changed, 7 deletions(-)
 delete mode 100644 tests/04r5swap.broken

diff --git a/tests/04r5swap.broken b/tests/04r5swap.broken
deleted file mode 100644
index e38987dbf01b..000000000000
--- a/tests/04r5swap.broken
+++ /dev/null
@@ -1,7 +0,0 @@
-always fails
-
-Fails with errors:
-
-  mdadm: /dev/loop0 has no superblock - assembly aborted
-
-   ERROR: no recovery happening
-- 
2.32.0 (Apple Git-132)

[PATCH 12/19] tests/04update-metadata skip linear

[Xiao Ni]

Add one check that if kernel doesn't support linear/multipath, it can
skip linear/multipath testing.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 test                    |  3 +++
 tests/04update-metadata | 35 ++++++++++++++++++++---------------
 tests/func.sh           |  2 ++
 3 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/test b/test
index 1fce6be2c4a9..f09994e78107 100755
--- a/test
+++ b/test
@@ -17,6 +17,9 @@ devlist=
 # device node will be /dev/md127 (127 is choosed by mdadm autumatically)
 is_foreign="no"
 
+skipping_linear="no"
+skipping_multipath="no"
+
 savelogs=0
 exitonerror=1
 ctrl_c_error=0
diff --git a/tests/04update-metadata b/tests/04update-metadata
index 2b72a303b6a0..c748770cfda7 100644
--- a/tests/04update-metadata
+++ b/tests/04update-metadata
@@ -8,24 +8,29 @@ set -xe
 
 dlist="$dev0 $dev1 $dev2 $dev3"
 
-for ls in linear/4 raid1/1 raid5/3 raid6/2
+if [ $skipping_linear == "yes" ]; then
+	level_list="raid1/1 raid5/3 raid6/2"
+else
+	level_list="linear/4 raid1/1 raid5/3 raid6/2"
+fi
+for ls in $level_list
 do
-  s=${ls#*/} l=${ls%/*}
-  if [[ $l == 'raid1' ]]; then
-	mdadm -CR --assume-clean -e 0.90 $md0 --level $l -n 4 $dlist
-  else
-	mdadm -CR --assume-clean -e 0.90 $md0 --level $l -n 4 -c 64 $dlist
-  fi
-  testdev $md0 $s 19904 64
-  mdadm -S $md0
-  mdadm -A $md0 --update=metadata $dlist
-  testdev $md0 $s 19904 64 check
-  mdadm -S $md0
+	s=${ls#*/} l=${ls%/*}
+	if [[ $l == 'raid1' ]]; then
+		mdadm -CR --assume-clean -e 0.90 $md0 --level $l -n 4 $dlist
+	else
+		mdadm -CR --assume-clean -e 0.90 $md0 --level $l -n 4 -c 64 $dlist
+	fi
+	testdev $md0 $s 19904 64
+	mdadm -S $md0
+	mdadm -A $md0 --update=metadata $dlist
+	testdev $md0 $s 19904 64 check
+	mdadm -S $md0
 done
 
 if mdadm -A $md0 --update=metadata $dlist
 then echo >&2 should fail with v1.0 metadata
-     exit 1
+	exit 1
 fi
 
 mdadm -CR -e 0.90 $md0 --level=6 -n4 -c32 $dlist
@@ -33,7 +38,7 @@ mdadm -S $md0
 
 if mdadm -A $md0 --update=metadata $dlist
 then echo >&2 should fail during resync
-     exit 1
+	exit 1
 fi
 mdadm -A $md0 $dlist
 mdadm --wait $md0 || true
@@ -48,5 +53,5 @@ mdadm -S $md0
 
 if mdadm -A $md0 --update=metadata $dlist
 then echo >&2 should fail when bitmap present
-     exit 1
+	exit 1
 fi
diff --git a/tests/func.sh b/tests/func.sh
index cfe83e552a2a..db55542d4011 100644
--- a/tests/func.sh
+++ b/tests/func.sh
@@ -125,6 +125,7 @@ check_env() {
 		MULTIPATH="yes"
 	if [ "$MULTIPATH" != "yes" ]; then
 		echo "test: skipping tests for multipath, which is removed in upstream 6.8+ kernels"
+		skipping_multipath="yes"
 	fi
 
 	# Check whether to run linear tests
@@ -133,6 +134,7 @@ check_env() {
 		LINEAR="yes"
 	if [ "$LINEAR" != "yes" ]; then
 		echo "test: skipping tests for linear, which is removed in upstream 6.8+ kernels"
+		skipping_linear="yes"
 	fi
 }
 
-- 
2.32.0 (Apple Git-132)

[PATCH 13/19] mdadm/tests: 05r5-internalbitmap

[Xiao Ni]

It's not right to compare bitmap bits with a number after io comes.
Because maybe those bits are already flused. Remove the related
tests.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 tests/05r5-internalbitmap | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/tests/05r5-internalbitmap b/tests/05r5-internalbitmap
index 13dc59215c1c..1a64482f11dd 100644
--- a/tests/05r5-internalbitmap
+++ b/tests/05r5-internalbitmap
@@ -9,21 +9,20 @@ mdadm -S $md0
 
 mdadm --assemble $md0 $dev1 $dev2 $dev3
 testdev $md0 2 $mdsize1 512
-dirty1=`mdadm -X $dev2 | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
 sleep 4
-dirty2=`mdadm -X $dev2 | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
+dirty1=`mdadm -X $dev2 | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
 
-if [ $dirty1 -lt 400 -o $dirty2 -ne 0 ]
-then  echo >&2 "ERROR bad 'dirty' counts: $dirty1 and $dirty2"
+if [ $dirty1 -ne 0 ]
+then  echo >&2 "ERROR bad 'dirty' counts: $dirty1"
   exit 1
 fi
 mdadm $md0 -f $dev1
 testdev $md0 2 $mdsize1 512
 sleep 4
-dirty3=`mdadm -X $dev2 | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
-if [ $dirty3 -lt 400 ]
+dirty2=`mdadm -X $dev2 | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
+if [ $dirty2 -lt 400 ]
 then
-   echo >&2 "ERROR dirty count $dirty3 is too small"
+   echo >&2 "ERROR dirty count $dirty2 is too small"
    exit 2
 fi
 
@@ -33,14 +32,12 @@ mdadm --assemble -R $md0  $dev2 $dev3
 mdadm --zero $dev1 # force --add, not --re-add
 mdadm $md0 --add $dev1
 check recovery
-
-dirty4=`mdadm -X $dev2 | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
 check wait
 sleep 4
-dirty5=`mdadm -X $dev2 | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
+dirty3=`mdadm -X $dev2 | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
 
-if [ $dirty4 -lt 400 -o $dirty5 -ne 0 ]
-then echo echo >&2 "ERROR bad 'dirty' counts at end: $dirty4 $dirty5"
+if [ $dirty3 -ne 0 ]
+then echo echo >&2 "ERROR bad 'dirty' counts at end: $dirty3"
   exit 1
 fi
 
-- 
2.32.0 (Apple Git-132)

[PATCH 14/19] mdadm/tests: 05r6-bitmapfile

[Xiao Ni]

It's not right to compare bitmap bits with a number after io comes.
Because maybe those bits are already flused. Remove the related
tests.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 tests/05r6-bitmapfile | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/tests/05r6-bitmapfile b/tests/05r6-bitmapfile
index d11896db221c..df66e59462cb 100644
--- a/tests/05r6-bitmapfile
+++ b/tests/05r6-bitmapfile
@@ -11,21 +11,20 @@ mdadm -S $md0
 
 mdadm --assemble $md0 --bitmap=$bmf $dev1 $dev2 $dev3 $dev4
 testdev $md0 2 $mdsize1 512
-dirty1=`mdadm -X $bmf | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
 sleep 4
-dirty2=`mdadm -X $bmf | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
+dirty1=`mdadm -X $bmf | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
 
-if [ $dirty1 -lt 400 -o $dirty2 -ne 0 ]
-then  echo >&2 "ERROR bad 'dirty' counts: $dirty1 and $dirty2"
+if [ $dirty1 -ne 0 ]
+then  echo >&2 "ERROR bad 'dirty' counts: $dirty1"
   exit 1
 fi
 mdadm $md0 -f $dev3
 testdev $md0 2 $mdsize1 512
 sleep 4
-dirty3=`mdadm -X $bmf | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
-if [ $dirty3 -lt 400 ]
+dirty2=`mdadm -X $bmf | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
+if [ $dirty2 -lt 400 ]
 then
-   echo >&2 "ERROR dirty count $dirty3 is too small"
+   echo >&2 "ERROR dirty count $dirty2 is too small"
    exit 2
 fi
 
@@ -35,14 +34,12 @@ mdadm --assemble -R $md0 --bitmap=$bmf $dev1 $dev2 $dev4
 mdadm --zero $dev3 # force --add, not --re-add
 mdadm $md0 --add $dev3
 check recovery
-
-dirty4=`mdadm -X $bmf | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
 check wait
 sleep 4
-dirty5=`mdadm -X $bmf | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
+dirty3=`mdadm -X $bmf | sed -n -e 's/.*Bitmap.* \([0-9]*\) dirty.*/\1/p'`
 
-if [ $dirty4 -lt 400 -o $dirty5 -ne 0 ]
-then echo echo >&2 "ERROR bad 'dirty' counts at end: $dirty4 $dirty5"
+if [ $dirty3 -ne 0 ]
+then echo echo >&2 "ERROR bad 'dirty' counts at end: $dirty3"
   exit 1
 fi
 
-- 
2.32.0 (Apple Git-132)

[PATCH 15/19] mdadm/tests: 06name enhance

[Xiao Ni]

It needs to check hostname in metadata name if one array is
local. Add the check in this case.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 tests/06name | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/06name b/tests/06name
index 86eaab69e3a1..c3213f6c9f7b 100644
--- a/tests/06name
+++ b/tests/06name
@@ -3,8 +3,14 @@ set -x
 # create an array with a name
 
 mdadm -CR $md0 -l0 -n2 --metadata=1 --name="Fred" $dev0 $dev1
-mdadm -E $dev0 | grep 'Name : Fred' > /dev/null || exit 1
-mdadm -D $md0 | grep 'Name : Fred' > /dev/null || exit 1
+
+if [ $is_foreign == "no" ]; then
+	mdadm -E $dev0 | grep "Name : $(hostname):Fred" > /dev/null || exit 1
+	mdadm -D $md0 | grep "Name : $(hostname):Fred" > /dev/null || exit 1
+else
+	mdadm -E $dev0 | grep "Name : Fred" > /dev/null || exit 1
+	mdadm -D $md0 | grep "Name : Fred" > /dev/null || exit 1
+fi
 mdadm -S $md0
 
 mdadm -A $md0 --name="Fred" $devlist
-- 
2.32.0 (Apple Git-132)

[PATCH 16/19] mdadm/tests: 07autoassemble

[Xiao Ni]

This test is used to test stacked array auto assemble.

There are two different cases depends on if array is foreign or not.
If the array is foreign, the stacked array (md0 is on md1 and md2)
can't be assembled with name md0. Because udev rule will run when md1
and md2 are assembled and mdadm -I doesn't specify homehost. So it
will treat stacked array (md0) as foreign array and choose md127 as
the device node name (/dev/md127)

Add the case that stacked array is local.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 test                        |  2 ++
 tests/07autoassemble        | 37 +++++++++++++++++++++++++++++++++++--
 tests/07autoassemble.broken |  8 --------
 3 files changed, 37 insertions(+), 10 deletions(-)
 delete mode 100644 tests/07autoassemble.broken

diff --git a/test b/test
index f09994e78107..4a88de58fdf5 100755
--- a/test
+++ b/test
@@ -39,6 +39,8 @@ md2=/dev/md2
 # if user doesn't specify minor number, mdadm chooses minor number
 # automatically from 127.
 md127=/dev/md127
+md126=/dev/md126
+md125=/dev/md125
 mdp0=/dev/md_d0
 mdp1=/dev/md_d1
 
diff --git a/tests/07autoassemble b/tests/07autoassemble
index e689be7c4a10..9dc781497070 100644
--- a/tests/07autoassemble
+++ b/tests/07autoassemble
@@ -10,7 +10,14 @@ mdadm -Ss
 mdadm -As -c /dev/null --homehost=testing -vvv
 testdev $md1 1 $mdsize1a 64
 testdev $md2 1 $mdsize1a 64
-testdev $md0 2 $mdsize11a 512
+# md1 and md2 will be incremental assemble by udev rule. And
+# the testing machines' hostname is not testing. The md0 will
+# be considered as a foreign array. It can use 0 as metadata
+# name. md127 will be used
+testdev $md127 2 $mdsize11a 512
+mdadm --stop $md127
+mdadm --zero-superblock $md1
+mdadm --zero-superblock $md2
 mdadm -Ss
 
 mdadm --zero-superblock $dev0 $dev1 $dev2 $dev3
@@ -20,5 +27,31 @@ mdadm -CR $md0 -l0 -n2 $md1 $dev2  --homehost=testing
 mdadm -Ss
 mdadm -As -c /dev/null --homehost=testing -vvv
 testdev $md1 1 $mdsize1a 64
-testdev $md0 1 $[mdsize1a+mdsize11a] 512
+testdev $md127 1 $[mdsize1a+mdsize11a] 512
+mdadm --stop $md127
+mdadm --zero-superblock $md1
+mdadm -Ss
+
+# Don't specify homehost when creating raid and use the test
+# machine's homehost. For super1.2, if homehost name's length
+# is > 32, it doesn't use homehost name in metadata name and
+# the array will be treated as foreign array
+mdadm --zero-superblock $dev0 $dev1 $dev2 $dev3
+mdadm -CR $md1 -l1 -n2 $dev0 $dev1
+mdadm -CR $md2 -l1 -n2 $dev2 $dev3
+mdadm -CR $md0 -l0 -n2 $md1 $md2
+mdadm -Ss
+mdadm -As -c /dev/null
+if [ $is_foreign == "yes" ]; then
+	# md127 is md1
+	testdev $md127 1 $mdsize1a 64
+	# md126 is md0, udev rule incremental assemble it
+	testdev $md126 2 $mdsize11a 512
+	# md125 is md2
+	testdev $md125 1 $mdsize1a 64
+else
+	testdev $md1 1 $mdsize1a 64
+	testdev $md2 1 $mdsize1a 64
+	testdev $md0 2 $mdsize11a 512
+fi
 mdadm -Ss
diff --git a/tests/07autoassemble.broken b/tests/07autoassemble.broken
deleted file mode 100644
index 8be09407f628..000000000000
--- a/tests/07autoassemble.broken
+++ /dev/null
@@ -1,8 +0,0 @@
-always fails
-
-Prints lots of messages, but the array doesn't assemble. Error
-possibly related to:
-
-  mdadm: /dev/md/1 is busy - skipping
-  mdadm: no recogniseable superblock on /dev/md/testing:0
-  mdadm: /dev/md/2 is busy - skipping
-- 
2.32.0 (Apple Git-132)

[PATCH 17/19] mdadm/tests: 07autodetect.broken can be removed

[Xiao Ni]

07autodetect can run successfully without error in kernel 6.9.0-rc5.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 tests/07autodetect.broken | 5 -----
 1 file changed, 5 deletions(-)
 delete mode 100644 tests/07autodetect.broken

diff --git a/tests/07autodetect.broken b/tests/07autodetect.broken
deleted file mode 100644
index 294954a1f50a..000000000000
--- a/tests/07autodetect.broken
+++ /dev/null
@@ -1,5 +0,0 @@
-always fails
-
-Fails with error:
-
-    ERROR: no resync happening
-- 
2.32.0 (Apple Git-132)

[PATCH 18/19] mdadm/tests: 07changelevelintr

[Xiao Ni]

It needs to specify a 2 powered array size when updating array size.
If not, it can't change chunksize.

And sometimes it reports error reshape doesn't happen. In fact the
reshape has finished. It doesn't need to wait before checking
reshape action. Because check function waits itself.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 tests/07changelevelintr        | 9 +++++----
 tests/07changelevelintr.broken | 9 ---------
 2 files changed, 5 insertions(+), 13 deletions(-)
 delete mode 100644 tests/07changelevelintr.broken

diff --git a/tests/07changelevelintr b/tests/07changelevelintr
index 18c63092071e..d921f2b284f9 100644
--- a/tests/07changelevelintr
+++ b/tests/07changelevelintr
@@ -27,11 +27,9 @@ checkgeo() {
 }
 
 restart() {
- sleep 0.5
  check reshape
  mdadm -S $md0
  mdadm -A $md0 $devs --backup-file=$bu
- sleep 0.5
  check reshape
 }
 
@@ -49,13 +47,16 @@ mdadm -G $md0 --layout rs --backup-file=$bu
 restart
 checkgeo md0 raid5 5 $[128*1024] 3
 
-mdadm -G $md0 --array-size 58368
+# It needs to shrink array size first. Choose a value that
+# is power of 2 for array size. If not, it can't change
+# chunk size.
+mdadm -G $md0 --array-size 51200
 mdadm -G $md0 --raid-disks 4 -c 64 --backup-file=$bu
 restart
 checkgeo md0 raid5 4 $[64*1024] 3
 
 devs="$dev0 $dev1 $dev2 $dev3"
-mdadm -G $md0 --array-size 19456
+mdadm -G $md0 --array-size 18432
 mdadm -G $md0 -n 2 -c 256 --backup-file=$bu
 restart
 checkgeo md0 raid5 2 $[256*1024] 3
diff --git a/tests/07changelevelintr.broken b/tests/07changelevelintr.broken
deleted file mode 100644
index 284b49068295..000000000000
--- a/tests/07changelevelintr.broken
+++ /dev/null
@@ -1,9 +0,0 @@
-always fails
-
-Fails with errors:
-
-  mdadm: this change will reduce the size of the array.
-         use --grow --array-size first to truncate array.
-         e.g. mdadm --grow /dev/md0 --array-size 56832
-
-  ERROR: no reshape happening
-- 
2.32.0 (Apple Git-132)

[PATCH 19/19] mdadm/tests: disable selinux

[Xiao Ni]

Sometimes systemd service fails because selinux. Disable selinux
during testing now. We can enable it in future when having a better
method.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 test          |  3 +++
 tests/func.sh | 10 ++++++++++
 2 files changed, 13 insertions(+)

diff --git a/test b/test
index 4a88de58fdf5..47f53ad78b75 100755
--- a/test
+++ b/test
@@ -16,6 +16,8 @@ devlist=
 # For example, /dev/md0 is created, stops it, then assemble it, the
 # device node will be /dev/md127 (127 is choosed by mdadm autumatically)
 is_foreign="no"
+#disable selinux
+sys_selinux="Permissive"
 
 skipping_linear="no"
 skipping_multipath="no"
@@ -351,6 +353,7 @@ main() {
 		fi
 	done
 
+	restore_selinux
 	exit 0
 }
 
diff --git a/tests/func.sh b/tests/func.sh
index db55542d4011..b2e4d122aa7f 100644
--- a/tests/func.sh
+++ b/tests/func.sh
@@ -167,6 +167,15 @@ is_raid_foreign() {
 	fi
 }
 
+record_selinux() {
+	sys_selinux=`getenforce`
+	setenforce Permissive
+}
+
+restore_selinux() {
+	setenforce $sys_selinux
+}
+
 do_setup() {
 	trap cleanup 0 1 3 15
 	trap ctrl_c 2
@@ -247,6 +256,7 @@ do_setup() {
 	echo 0 > /sys/module/md_mod/parameters/start_ro
 	record_system_speed_limit
 	is_raid_foreign
+	record_selinux
 }
 
 # check various things
-- 
2.32.0 (Apple Git-132)

Re: [PATCH 01/19] Change some error messages to info level

[Mariusz Tkaczyk]

On Wed, 22 May 2024 16:50:38 +0800
Xiao Ni <xni@redhat.com> wrote:

> These logs are not error logs. Change them to info level.
> 
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
Applied!

Thanks,
Mariusz

Re: [PATCH 02/19] mdadm: Start update_opt from 0

[Mariusz Tkaczyk]

On Wed, 22 May 2024 16:50:39 +0800
Xiao Ni <xni@redhat.com> wrote:

> Before f2e8393bd722 ('Manage&Incremental: code refactor, string to enum'), it
> uses NULL to represent it doesn't need to update. So init UOPT_UNDEFINED to
> 0. This problem is found by test case 05r6tor0.
> 
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
Applied! 

Thanks,
Mariusz

Re: [PATCH 03/19] Don't control reshape speed in daemon

[Mariusz Tkaczyk]

On Wed, 22 May 2024 16:50:40 +0800
Xiao Ni <xni@redhat.com> wrote:

> It tries to set the max sync speed in reshape. This should be done by
> administrators by control interfaces /proc/sys/dev/raid/speed_limit_max/min.
> 
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
I totally agree.

Applied! 

Thanks,
Mariusz

Re: [PATCH 04/19] mdadm/tests: test enhance

[Mariusz Tkaczyk]

On Wed, 22 May 2024 16:50:41 +0800
Xiao Ni <xni@redhat.com> wrote:

> There are two changes.
> First, if md module is not loaded, it gives error when reading
> speed_limit_max. So read the value after loading md module which
> is done in do_setup
> 
> Second, sometimes the test reports error sync action doesn't
> happen. But dmesg shows sync action is done. So limit the sync
> speed before test. It doesn't affect the test run time. Because
> check wait sets the max speed before waiting sync action. And
> recording speed_limit_max/min in do_setup.
> 
> Fixes: 4c12714d1ca0 ('test: run tests on system level mdadm')
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
>  test          | 10 +++++-----
>  tests/func.sh | 26 +++++++++++++++++++++++---
>  2 files changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/test b/test
> index 338c2db44fa7..ff403293d60b 100755
> --- a/test
> +++ b/test
> @@ -6,7 +6,10 @@ targetdir="/var/tmp"
>  logdir="$targetdir"
>  config=/tmp/mdadm.conf
>  testdir=$PWD/tests
> -system_speed_limit=`cat /proc/sys/dev/raid/speed_limit_max`
> +system_speed_limit_max=0
> +system_speed_limit_min=0
> +test_speed_limit_min=100
> +test_speed_limit_max=500
>  devlist=
>  
>  savelogs=0
> @@ -39,10 +42,6 @@ ctrl_c() {
>  	ctrl_c_error=1
>  }
>  
> -restore_system_speed_limit() {
> -	echo $system_speed_limit > /proc/sys/dev/raid/speed_limit_max
> -}
> -
>  mdadm() {
>  	rm -f $targetdir/stderr
>  	case $* in
> @@ -103,6 +102,7 @@ do_test() {
>  		do_clean
>  		# source script in a subshell, so it has access to our
>  		# namespace, but cannot change it.
> +		control_system_speed_limit
>  		echo -ne "$_script... "
>  		if ( set -ex ; . $_script ) &> $targetdir/log
>  		then
> diff --git a/tests/func.sh b/tests/func.sh
> index b474442b6abe..221cff158f8c 100644
> --- a/tests/func.sh
> +++ b/tests/func.sh
> @@ -136,6 +136,23 @@ check_env() {
>  	fi
>  }
>  
> +record_system_speed_limit() {
> +	system_speed_limit_max=`cat /proc/sys/dev/raid/speed_limit_max`
> +	system_speed_limit_min=`cat /proc/sys/dev/raid/speed_limit_min`
> +}
> +
> +# To avoid sync action finishes before checking it, it needs to limit
> +# the sync speed
> +control_system_speed_limit() {
> +	echo $test_speed_limit_min > /proc/sys/dev/raid/speed_limit_min
> +	echo $test_speed_limit_max > /proc/sys/dev/raid/speed_limit_max
> +}
> +
> +restore_system_speed_limit() {
> +	echo $system_speed_limit_min > /proc/sys/dev/raid/speed_limit_max
> +	echo $system_speed_limit_max > /proc/sys/dev/raid/speed_limit_max
> +}
> +

Hi Xiao,
control and restore functions are not used or in this patch. if you want to use
them in next patches please highlight it in description.
Beside that LGTM.

I will loop over the patches and take as much I can. Looks like there are no
conflicts.

Thanks,
Mariusz

Re: [PATCH 05/19] mdadm/tests: test don't fail when systemd reports error

[Mariusz Tkaczyk]

On Wed, 22 May 2024 16:50:42 +0800
Xiao Ni <xni@redhat.com> wrote:

> Sometimes systemd reports error in dmesg and test fails. Add
> a condition to avoid this failure.
> 
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
Applied! 

Thanks,
Mariusz

Re: [PATCH 06/19] mdadm/tests: names_template enhance

[Mariusz Tkaczyk]

On Wed, 22 May 2024 16:50:43 +0800
Xiao Ni <xni@redhat.com> wrote:

> For super1, if the length of hostname is >= 32, it doesn't add hostname
> in metadata name. Fix this problem by checking the length of hostname.
> Because other cases may use need to check this, so do the check in
> do_setup.
> 
> And this patch adds a check if link /dev/md/name exists.
> 
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
LGTM but I cannot apply this due to conflict after omitting 04.

Thanks,
Mariusz

Re: [PATCH 07/19] mdadm/tests: 03assem-incr enhance

[Mariusz Tkaczyk]

On Wed, 22 May 2024 16:50:44 +0800
Xiao Ni <xni@redhat.com> wrote:

> It fails when hostname lenght > 32. Because the super1 metadata name
> doesn't include hostname when hostname length > 32. Then mdadm thinks
> the array is a foreign array if no device link is specified when
> assembling the array. It chooses a minor number from 127.
> 
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---

LGTM.
Skipping for now, obvious conflict.

Thanks,
Mariusz

Re: [PATCH 04/19] mdadm/tests: test enhance

[Xiao Ni]

On Thu, May 23, 2024 at 10:26 PM Mariusz Tkaczyk
<mariusz.tkaczyk@linux.intel.com> wrote:
>
> On Wed, 22 May 2024 16:50:41 +0800
> Xiao Ni <xni@redhat.com> wrote:
>
> > There are two changes.
> > First, if md module is not loaded, it gives error when reading
> > speed_limit_max. So read the value after loading md module which
> > is done in do_setup
> >
> > Second, sometimes the test reports error sync action doesn't
> > happen. But dmesg shows sync action is done. So limit the sync
> > speed before test. It doesn't affect the test run time. Because
> > check wait sets the max speed before waiting sync action. And
> > recording speed_limit_max/min in do_setup.
> >
> > Fixes: 4c12714d1ca0 ('test: run tests on system level mdadm')
> > Signed-off-by: Xiao Ni <xni@redhat.com>
> > ---
> >  test          | 10 +++++-----
> >  tests/func.sh | 26 +++++++++++++++++++++++---
> >  2 files changed, 28 insertions(+), 8 deletions(-)
> >
> > diff --git a/test b/test
> > index 338c2db44fa7..ff403293d60b 100755
> > --- a/test
> > +++ b/test
> > @@ -6,7 +6,10 @@ targetdir="/var/tmp"
> >  logdir="$targetdir"
> >  config=/tmp/mdadm.conf
> >  testdir=$PWD/tests
> > -system_speed_limit=`cat /proc/sys/dev/raid/speed_limit_max`
> > +system_speed_limit_max=0
> > +system_speed_limit_min=0
> > +test_speed_limit_min=100
> > +test_speed_limit_max=500
> >  devlist=
> >
> >  savelogs=0
> > @@ -39,10 +42,6 @@ ctrl_c() {
> >       ctrl_c_error=1
> >  }
> >
> > -restore_system_speed_limit() {
> > -     echo $system_speed_limit > /proc/sys/dev/raid/speed_limit_max
> > -}
> > -
> >  mdadm() {
> >       rm -f $targetdir/stderr
> >       case $* in
> > @@ -103,6 +102,7 @@ do_test() {
> >               do_clean
> >               # source script in a subshell, so it has access to our
> >               # namespace, but cannot change it.
> > +             control_system_speed_limit
> >               echo -ne "$_script... "
> >               if ( set -ex ; . $_script ) &> $targetdir/log
> >               then
> > diff --git a/tests/func.sh b/tests/func.sh
> > index b474442b6abe..221cff158f8c 100644
> > --- a/tests/func.sh
> > +++ b/tests/func.sh
> > @@ -136,6 +136,23 @@ check_env() {
> >       fi
> >  }
> >
> > +record_system_speed_limit() {
> > +     system_speed_limit_max=`cat /proc/sys/dev/raid/speed_limit_max`
> > +     system_speed_limit_min=`cat /proc/sys/dev/raid/speed_limit_min`
> > +}
> > +
> > +# To avoid sync action finishes before checking it, it needs to limit
> > +# the sync speed
> > +control_system_speed_limit() {
> > +     echo $test_speed_limit_min > /proc/sys/dev/raid/speed_limit_min
> > +     echo $test_speed_limit_max > /proc/sys/dev/raid/speed_limit_max
> > +}
> > +
> > +restore_system_speed_limit() {
> > +     echo $system_speed_limit_min > /proc/sys/dev/raid/speed_limit_max
> > +     echo $system_speed_limit_max > /proc/sys/dev/raid/speed_limit_max
> > +}
> > +
>
> Hi Xiao,
> control and restore functions are not used or in this patch. if you want to use
> them in next patches please highlight it in description.
> Beside that LGTM.

Hi Mariusz

The record_system_speed_limit and control_system_speed_limit are used
in this patch. restore_system_speed_limit has been used before my
patch set already. So I don't need to change it.

Regards
Xiao
>
> I will loop over the patches and take as much I can. Looks like there are no
> conflicts.
>
> Thanks,
> Mariusz
>

Re: [PATCH 08/19] mdadm/tests 03r0assem enhance

[Mariusz Tkaczyk]

On Wed, 22 May 2024 16:50:45 +0800
Xiao Ni <xni@redhat.com> wrote:

> dcc22ae74a864 ('super1: remove support for name= in config') already
> removes name= support. So remove related test codes in 03r0assem.
> 
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
Applied! 

Thanks,
Mariusz

Re: [PATCH 04/19] mdadm/tests: test enhance

[Mariusz Tkaczyk]

On Thu, 23 May 2024 22:39:16 +0800
Xiao Ni <xni@redhat.com> wrote:

> > Hi Xiao,
> > control and restore functions are not used or in this patch. if you want to
> > use them in next patches please highlight it in description.
> > Beside that LGTM.  
> 
> Hi Mariusz
> 
> The record_system_speed_limit and control_system_speed_limit are used
> in this patch. restore_system_speed_limit has been used before my
> patch set already. So I don't need to change it.
> 

Thanks for fast response. The patches are not pushed yet, I will wait. I looped
the code and don't see usages. You just declared them,where they are used?

Thanks,
Mariusz

Re: [PATCH 04/19] mdadm/tests: test enhance

[Xiao Ni]

On Wed, May 22, 2024 at 4:51 PM Xiao Ni <xni@redhat.com> wrote:
>
> There are two changes.
> First, if md module is not loaded, it gives error when reading
> speed_limit_max. So read the value after loading md module which
> is done in do_setup
>
> Second, sometimes the test reports error sync action doesn't
> happen. But dmesg shows sync action is done. So limit the sync
> speed before test. It doesn't affect the test run time. Because
> check wait sets the max speed before waiting sync action. And
> recording speed_limit_max/min in do_setup.
>
> Fixes: 4c12714d1ca0 ('test: run tests on system level mdadm')
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
>  test          | 10 +++++-----
>  tests/func.sh | 26 +++++++++++++++++++++++---
>  2 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/test b/test
> index 338c2db44fa7..ff403293d60b 100755
> --- a/test
> +++ b/test
> @@ -6,7 +6,10 @@ targetdir="/var/tmp"
>  logdir="$targetdir"
>  config=/tmp/mdadm.conf
>  testdir=$PWD/tests
> -system_speed_limit=`cat /proc/sys/dev/raid/speed_limit_max`
> +system_speed_limit_max=0
> +system_speed_limit_min=0
> +test_speed_limit_min=100
> +test_speed_limit_max=500
>  devlist=
>
>  savelogs=0
> @@ -39,10 +42,6 @@ ctrl_c() {
>         ctrl_c_error=1
>  }
>
> -restore_system_speed_limit() {
> -       echo $system_speed_limit > /proc/sys/dev/raid/speed_limit_max
> -}
> -
>  mdadm() {
>         rm -f $targetdir/stderr
>         case $* in
> @@ -103,6 +102,7 @@ do_test() {
>                 do_clean
>                 # source script in a subshell, so it has access to our
>                 # namespace, but cannot change it.
> +               control_system_speed_limit

It controls the system speed here. You can see
restore_system_speed_limit in the source code. It was added in
4c12714d1ca06533fe7a887966df2558fd2f96b2


>                 echo -ne "$_script... "
>                 if ( set -ex ; . $_script ) &> $targetdir/log
>                 then
> diff --git a/tests/func.sh b/tests/func.sh
> index b474442b6abe..221cff158f8c 100644
> --- a/tests/func.sh
> +++ b/tests/func.sh
> @@ -136,6 +136,23 @@ check_env() {
>         fi
>  }
>
> +record_system_speed_limit() {
> +       system_speed_limit_max=`cat /proc/sys/dev/raid/speed_limit_max`
> +       system_speed_limit_min=`cat /proc/sys/dev/raid/speed_limit_min`
> +}
> +
> +# To avoid sync action finishes before checking it, it needs to limit
> +# the sync speed
> +control_system_speed_limit() {
> +       echo $test_speed_limit_min > /proc/sys/dev/raid/speed_limit_min
> +       echo $test_speed_limit_max > /proc/sys/dev/raid/speed_limit_max
> +}
> +
> +restore_system_speed_limit() {
> +       echo $system_speed_limit_min > /proc/sys/dev/raid/speed_limit_max
> +       echo $system_speed_limit_max > /proc/sys/dev/raid/speed_limit_max
> +}
> +
>  do_setup() {
>         trap cleanup 0 1 3 15
>         trap ctrl_c 2
> @@ -214,6 +231,7 @@ do_setup() {
>         ulimit -c unlimited
>         [ -f /proc/mdstat ] || modprobe md_mod
>         echo 0 > /sys/module/md_mod/parameters/start_ro
> +       record_system_speed_limit

And it records the system speed here.

Best Regards
Xiao

>  }
>
>  # check various things
> @@ -265,15 +283,17 @@ check() {
>                 fi
>         ;;
>         wait )
> -               p=`cat /proc/sys/dev/raid/speed_limit_max`
> -               echo 2000000 > /proc/sys/dev/raid/speed_limit_max
> +               min=`cat /proc/sys/dev/raid/speed_limit_min`
> +               max=`cat /proc/sys/dev/raid/speed_limit_max`
> +               echo 200000 > /proc/sys/dev/raid/speed_limit_max
>                 sleep 0.1
>                 while grep -Eq '(resync|recovery|reshape|check|repair) *=' /proc/mdstat ||
>                         grep -v idle > /dev/null /sys/block/md*/md/sync_action
>                 do
>                         sleep 0.5
>                 done
> -               echo $p > /proc/sys/dev/raid/speed_limit_max
> +               echo $min > /proc/sys/dev/raid/speed_limit_min
> +               echo $max > /proc/sys/dev/raid/speed_limit_max
>         ;;
>         state )
>                 grep -sq "blocks.*\[$2\]\$" /proc/mdstat ||
> --
> 2.32.0 (Apple Git-132)
>
>

Re: [PATCH 04/19] mdadm/tests: test enhance

[Mariusz Tkaczyk]

On Wed, 22 May 2024 16:50:41 +0800
Xiao Ni <xni@redhat.com> wrote:

> There are two changes.
> First, if md module is not loaded, it gives error when reading
> speed_limit_max. So read the value after loading md module which
> is done in do_setup
> 
> Second, sometimes the test reports error sync action doesn't
> happen. But dmesg shows sync action is done. So limit the sync
> speed before test. It doesn't affect the test run time. Because
> check wait sets the max speed before waiting sync action. And
> recording speed_limit_max/min in do_setup.
> 
> Fixes: 4c12714d1ca0 ('test: run tests on system level mdadm')
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
Applied! 

Thanks,
Mariusz

Re: [PATCH 00/19] mdadm/tests: enhance/fix regression cases

[Mariusz Tkaczyk]

On Wed, 22 May 2024 16:50:37 +0800
Xiao Ni <xni@redhat.com> wrote:

> Hi all
> 
> This is the first set which has fixed and enhanced some cases. I'll
> go on fixing/enhancing the following cases.
> 

Hello,
I applied them, expect 14 because this test has been recently removed by me:
https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=50b100768a115526f5029113af957658ef76b383

Thank you, great job!
Mariusz