[PATCH RFC 0/5] mdadm tests fix and enhance

[PATCH RFC 0/5] mdadm tests fix and enhance

[Xiao Ni]

Hi all

This patch set tries to fix and enhance mdadm tests. This is just the
beginning of this job. I want to send it out to get some opinions to
avoid some faults and better methods.

Xiao Ni (5):
  tests/test enhance
  tests/00createnames enhance
  tests/01r5fail enhance
  tests/01r5integ.broken
  tests/01raid6integ.broken can be removed

 test                           | 18 ++++++++++++++++--
 tests/00createnames            |  9 +++++++++
 tests/01r5fail                 |  6 +-----
 tests/01r5integ.broken         |  7 -------
 tests/01raid6integ.broken      |  7 -------
 tests/templates/names_template | 15 ++++++++++++++-
 6 files changed, 40 insertions(+), 22 deletions(-)
 delete mode 100644 tests/01r5integ.broken
 delete mode 100644 tests/01raid6integ.broken

-- 
2.32.0 (Apple Git-132)

[PATCH 1/5] 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.

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

diff --git a/test b/test
index 338c2db44fa7..2aa3bd75d503 100755
--- a/test
+++ b/test
@@ -6,7 +6,8 @@ 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
 devlist=
 
 savelogs=0
@@ -39,8 +40,19 @@ ctrl_c() {
 	ctrl_c_error=1
 }
 
+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 $system_speed_limit_min > /proc/sys/dev/raid/speed_limit_max
+}
+
 restore_system_speed_limit() {
-	echo $system_speed_limit > /proc/sys/dev/raid/speed_limit_max
+	echo $system_speed_limit_max > /proc/sys/dev/raid/speed_limit_max
 }
 
 mdadm() {
@@ -103,6 +115,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
@@ -309,6 +322,7 @@ print_warning() {
 main() {
 	print_warning
 	do_setup
+	record_system_speed_limit
 
 	echo "Testing on linux-$(uname -r) kernel"
 	[ "$savelogs" == "1" ] &&
-- 
2.32.0 (Apple Git-132)

Re: [PATCH 1/5] tests/test enhance

[Mateusz Kusiak]

Hi Xiao,
one small note from me.

On 18.04.2024 12:23, Xiao Ni wrote:
> @@ -309,6 +322,7 @@ print_warning() {
>   main() {
>   	print_warning
>   	do_setup
> +	record_system_speed_limit
>   
>   	echo "Testing on linux-$(uname -r) kernel"
>   	[ "$savelogs" == "1" ] &&


I feel like record_system_speed_limit() should be called in do_setup() rather than main().
Saving current system settings is job of setup.

Thanks,
Mateusz

Re: [PATCH 1/5] tests/test enhance

[Mateusz Kusiak]

On 19.04.2024 09:16, Mateusz Kusiak wrote:
> Hi Xiao,
> one small note from me.
> 
> On 18.04.2024 12:23, Xiao Ni wrote:
>> @@ -309,6 +322,7 @@ print_warning() {
>>   main() {
>>       print_warning
>>       do_setup
>> +    record_system_speed_limit
>>       echo "Testing on linux-$(uname -r) kernel"
>>       [ "$savelogs" == "1" ] &&
> 
> 
> I feel like record_system_speed_limit() should be called in do_setup() rather than main().
> Saving current system settings is job of setup.
> 
> Thanks,
> Mateusz
> 

One more thing. Feel free to add tag "fixes".
I broke this behavior (lowering sync speed) in 4c12714d1ca0 ("test: run tests on system level mdadm").

Thanks,
Mateusz

Re: [PATCH 1/5] tests/test enhance

[Xiao Ni]

On Fri, Apr 19, 2024 at 3:30 PM Mateusz Kusiak
<mateusz.kusiak@linux.intel.com> wrote:
>
> On 19.04.2024 09:16, Mateusz Kusiak wrote:
> > Hi Xiao,
> > one small note from me.
> >
> > On 18.04.2024 12:23, Xiao Ni wrote:
> >> @@ -309,6 +322,7 @@ print_warning() {
> >>   main() {
> >>       print_warning
> >>       do_setup
> >> +    record_system_speed_limit
> >>       echo "Testing on linux-$(uname -r) kernel"
> >>       [ "$savelogs" == "1" ] &&
> >
> >
> > I feel like record_system_speed_limit() should be called in do_setup() rather than main().
> > Saving current system settings is job of setup.

Thanks for the suggestion.

> >
> > Thanks,
> > Mateusz
> >
>
> One more thing. Feel free to add tag "fixes".
> I broke this behavior (lowering sync speed) in 4c12714d1ca0 ("test: run tests on system level mdadm").

Ok, no problem.

Regards
Xiao
>
> Thanks,
> Mateusz
>

[PATCH 2/5] tests/00createnames enhance

[Xiao Ni]

Now 00createnames doesn't check Create names=yes config. Without this
config, mdadm creates /dev/md127 device node when mdadm --create
/dev/md/test. With this config, it creates /dev/md_test. This patch
only adds the check. If it has this config, it returns directly
without error.

And this case has an error. 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.

And this patch adds a check if device node exists.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 tests/00createnames            |  9 +++++++++
 tests/templates/names_template | 15 ++++++++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/tests/00createnames b/tests/00createnames
index a95e7d2bb085..2eb3d9331dda 100644
--- a/tests/00createnames
+++ b/tests/00createnames
@@ -3,6 +3,15 @@ set -x -e
 
 # Test how <devname> and --name= are handled for create mode.
 
+create_with_name=`cat /etc/mdadm.conf | grep "^Create*.names=yes"`
+
+if [ -n "$create_with_name" ]; then
+	exit 0
+fi
+
+#The following cases don't consider the names=yes setting in mdadm.conf
+#It needs to add the respecting cases which consider names=yes config
+
 # The most trivial case.
 names_create "/dev/md/name"
 names_verify "/dev/md127" "name" "name"
diff --git a/tests/templates/names_template b/tests/templates/names_template
index 1b6cd14bf51d..4c7ff8c27f73 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
@@ -46,13 +47,25 @@ function names_verify() {
 		exit 1
 	fi
 
+	if [ -b /dev/md/$WANTED_LINK ]; then
+		echo "/dev/md/$WANTED_LINK doesn't exit"
+	fi
+
 	local RES="$(mdadm -D --export $DEVNODE_NAME | grep MD_NAME)"
 	if [[ "$?" != "0" ]]; then
 		echo "Cannot get metadata from $dev0."
 		exit 1
 	fi
 
-	local EXPECTED="MD_NAME=$(hostname):$WANTED_NAME"
+	#If the lenght of hostname is >= 32, super1 doesn't use hostname
+	#in metadata
+	local is_super1="$(mdadm -D --export $DEVNODE_NAME | grep MD_METADATA=1.2)"
+	hostname=$(hostname)
+	if [ -n "$is_super1" -a `expr length $(hostname)` -lt 32 ]; 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)

Re: [PATCH 2/5] tests/00createnames enhance

[Mariusz Tkaczyk]

On Thu, 18 Apr 2024 18:23:18 +0800
Xiao Ni <xni@redhat.com> wrote:

> Now 00createnames doesn't check Create names=yes config. Without this
> config, mdadm creates /dev/md127 device node when mdadm --create
> /dev/md/test. With this config, it creates /dev/md_test. This patch
> only adds the check. If it has this config, it returns directly
> without error.

Hi Xiao,
Thanks for patches I will review them all later (probably next week).

About this one:

The proposed change is not complete as config may be read from both
/etc/mdadm.conf and /etc/mdadm/mdadm.conf. Ideally, you should check them
both in the approach you proposed.

There is also possible to have /etc/mdadm.d/ directory - it is always checked
and read if exists and it cannot be disabled. See load_conffile() and
CONFFILEFLAGS in makefile for details.

Test relies on the global configuration and user may forgot that it is set.
That will give us positive test result because test was not run due to
configuration issue. This is risky, I would prefer fail to indicate
that something is wrong. User can skip this test.

What about adding empty mdadm config to the command `-c ./mdadm_empty.conf`? I
see it as the best option for now. That save use from checking 2 config
locations and any user defined behaviors. Do you see any disadvantages?

As config directory is not configurable we have to accept the risk that
something could be there.
Ideally, you can propose patch with confdir customization to apply same
solution as for conffile (just set it to empty directory) but as it is probably
rarely used we can accept risk here for now (unless somebody reported). I give
it up to you as it not having confdir customization is more like new
feature.

Another possible solution would be to learn mdadm print it's configuration and
print it before running test and fail if not compatible setting detected.

I did not realize that it would be a problem, that for catching!

Thanks,
Mariusz

Re: [PATCH 2/5] tests/00createnames enhance

[Xiao Ni]

On Fri, Apr 19, 2024 at 3:20 PM Mariusz Tkaczyk
<mariusz.tkaczyk@linux.intel.com> wrote:
>
> On Thu, 18 Apr 2024 18:23:18 +0800
> Xiao Ni <xni@redhat.com> wrote:
>
> > Now 00createnames doesn't check Create names=yes config. Without this
> > config, mdadm creates /dev/md127 device node when mdadm --create
> > /dev/md/test. With this config, it creates /dev/md_test. This patch
> > only adds the check. If it has this config, it returns directly
> > without error.
>
> Hi Xiao,
> Thanks for patches I will review them all later (probably next week).
>
> About this one:
>
> The proposed change is not complete as config may be read from both
> /etc/mdadm.conf and /etc/mdadm/mdadm.conf. Ideally, you should check them
> both in the approach you proposed.
>
> There is also possible to have /etc/mdadm.d/ directory - it is always checked
> and read if exists and it cannot be disabled. See load_conffile() and
> CONFFILEFLAGS in makefile for details.

Hi Mariusz

Yes, you're right. Thanks for this.

>
> Test relies on the global configuration and user may forgot that it is set.
> That will give us positive test result because test was not run due to
> configuration issue. This is risky, I would prefer fail to indicate
> that something is wrong. User can skip this test.

Ok, we can keep it the way it is.

>
> What about adding empty mdadm config to the command `-c ./mdadm_empty.conf`? I
> see it as the best option for now. That save use from checking 2 config
> locations and any user defined behaviors. Do you see any disadvantages?

You mean specifying config file in test case when creating raid?

>
> As config directory is not configurable we have to accept the risk that
> something could be there.
> Ideally, you can propose patch with confdir customization to apply same
> solution as for conffile (just set it to empty directory) but as it is probably
> rarely used we can accept risk here for now (unless somebody reported). I give
> it up to you as it not having confdir customization is more like new
> feature.

I'll remove the adding check in the case. I think few people use the config too.


>
> Another possible solution would be to learn mdadm print it's configuration and
> print it before running test and fail if not compatible setting detected.
>
> I did not realize that it would be a problem, that for catching!

Regards
Xiao
>
> Thanks,
> Mariusz
>

Re: [PATCH 2/5] tests/00createnames enhance

[Mariusz Tkaczyk]

On Mon, 22 Apr 2024 14:56:17 +0800
Xiao Ni <xni@redhat.com> wrote:

> >
> > What about adding empty mdadm config to the command `-c
> > ./mdadm_empty.conf`? I see it as the best option for now. That save use
> > from checking 2 config locations and any user defined behaviors. Do you see
> > any disadvantages?  
> 
> You mean specifying config file in test case when creating raid?

Yes, with that default config won't be read.

Mariusz

Re: [PATCH 2/5] tests/00createnames enhance

[Mariusz Tkaczyk]

On Thu, 18 Apr 2024 18:23:18 +0800
Xiao Ni <xni@redhat.com> wrote:

> +	local is_super1="$(mdadm -D --export $DEVNODE_NAME | grep
> MD_METADATA=1.2)"

You can also limit this test to super-1.2 it may depend on config, so we can
specify metadata directly in create command (if it is not specified).

Mariusz

Re: [PATCH 2/5] tests/00createnames enhance

[Xiao Ni]

On Fri, Apr 19, 2024 at 5:47 PM Mariusz Tkaczyk
<mariusz.tkaczyk@linux.intel.com> wrote:
>
> On Thu, 18 Apr 2024 18:23:18 +0800
> Xiao Ni <xni@redhat.com> wrote:
>
> > +     local is_super1="$(mdadm -D --export $DEVNODE_NAME | grep
> > MD_METADATA=1.2)"
>
> You can also limit this test to super-1.2 it may depend on config, so we can
> specify metadata directly in create command (if it is not specified).

Could you explain more? I don't catch you here.

Regards
Xiao
>
> Mariusz
>

Re: [PATCH 2/5] tests/00createnames enhance

[Mariusz Tkaczyk]

On Mon, 22 Apr 2024 14:57:16 +0800
Xiao Ni <xni@redhat.com> wrote:

> On Fri, Apr 19, 2024 at 5:47 PM Mariusz Tkaczyk
> <mariusz.tkaczyk@linux.intel.com> wrote:
> >
> > On Thu, 18 Apr 2024 18:23:18 +0800
> > Xiao Ni <xni@redhat.com> wrote:
> >  
> > > +     local is_super1="$(mdadm -D --export $DEVNODE_NAME | grep
> > > MD_METADATA=1.2)"  
> >
> > You can also limit this test to super-1.2 it may depend on config, so we can
> > specify metadata directly in create command (if it is not specified).  
> 
> Could you explain more? I don't catch you here.

Default metadata could be customized. At first glance, I thought
that you added this because we metadata is not specified so I checked:
	if [[ -z "$NAME" ]]; then
		mdadm -CR "$DEVNAME" -l0 -n 1 $dev0 --force
	else
		mdadm -CR "$DEVNAME" --name="$NAME" --metadata=1.2 -l0 -n 1
	$dev0 --force
        fi


It seems that I forgot to add metadata specifier in first create command. If
you have different metadata than 1.2 you can add --metadata=1.2 and then:

> > > +     local is_super1="$(mdadm -D --export $DEVNODE_NAME | grep
> > > MD_METADATA=1.2)" 

won't be needed. Do I miss something?

Thanks,
Mariusz

Re: [PATCH 2/5] tests/00createnames enhance

[Xiao Ni]

On Mon, Apr 22, 2024 at 6:15 PM Mariusz Tkaczyk
<mariusz.tkaczyk@linux.intel.com> wrote:
>
> On Mon, 22 Apr 2024 14:57:16 +0800
> Xiao Ni <xni@redhat.com> wrote:
>
> > On Fri, Apr 19, 2024 at 5:47 PM Mariusz Tkaczyk
> > <mariusz.tkaczyk@linux.intel.com> wrote:
> > >
> > > On Thu, 18 Apr 2024 18:23:18 +0800
> > > Xiao Ni <xni@redhat.com> wrote:
> > >
> > > > +     local is_super1="$(mdadm -D --export $DEVNODE_NAME | grep
> > > > MD_METADATA=1.2)"
> > >
> > > You can also limit this test to super-1.2 it may depend on config, so we can
> > > specify metadata directly in create command (if it is not specified).
> >
> > Could you explain more? I don't catch you here.
>
> Default metadata could be customized. At first glance, I thought
> that you added this because we metadata is not specified so I checked:
>         if [[ -z "$NAME" ]]; then
>                 mdadm -CR "$DEVNAME" -l0 -n 1 $dev0 --force
>         else
>                 mdadm -CR "$DEVNAME" --name="$NAME" --metadata=1.2 -l0 -n 1
>         $dev0 --force
>         fi
>
>
> It seems that I forgot to add metadata specifier in first create command. If
> you have different metadata than 1.2 you can add --metadata=1.2 and then:
>
> > > > +     local is_super1="$(mdadm -D --export $DEVNODE_NAME | grep
> > > > MD_METADATA=1.2)"
>
> won't be needed. Do I miss something?

Ah, yes, you are right. It doesn't need to check metadata.  I thought
there were other metadata possibilities. Thanks for pointing this
problem out :)

Best Regards
Xiao

>
> Thanks,
> Mariusz
>

[PATCH 3/5] tests/01r5fail enhance

[Xiao Ni]

After removing dev0, the recovery starts because it already has a spare
disk. It's good to check recovery. But it's not right to check recovery
after adding dev3. Because the recovery may finish. It depends on the
recovery performance of the testing machine. If the recovery finishes,
it will fail. But dev3 is only added as a spare disk, we can't expect
there is a recovery happens.

So remove the codes about adding dev3.

Signed-off-by: Xiao Ni <xni@redhat.com>
---
 tests/01r5fail | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tests/01r5fail b/tests/01r5fail
index 873dba585e58..c210d6e747f2 100644
--- a/tests/01r5fail
+++ b/tests/01r5fail
@@ -17,11 +17,7 @@ check wait
 mdadm $md0 --fail $dev0
 mdadm $md0 --remove $dev3 $dev0
 check recovery
-check state _UUU
-
-mdadm $md0 -a $dev3
-check recovery
 check wait
 check state UUUU
 
-mdadm -S $md0
\ No newline at end of file
+mdadm -S $md0
-- 
2.32.0 (Apple Git-132)

Re: [PATCH 3/5] tests/01r5fail enhance

[Mariusz Tkaczyk]

On Thu, 18 Apr 2024 18:23:19 +0800
Xiao Ni <xni@redhat.com> wrote:

> After removing dev0, the recovery starts because it already has a spare
> disk. It's good to check recovery. But it's not right to check recovery
> after adding dev3. Because the recovery may finish. It depends on the
> recovery performance of the testing machine. If the recovery finishes,
> it will fail. But dev3 is only added as a spare disk, we can't expect
> there is a recovery happens.
> 
> So remove the codes about adding dev3.
> 
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---

LGTM.

Mariusz

Re: [PATCH 3/5] tests/01r5fail enhance

[Mariusz Tkaczyk]

On Thu, 18 Apr 2024 18:23:19 +0800
Xiao Ni <xni@redhat.com> wrote:

> After removing dev0, the recovery starts because it already has a spare
> disk. It's good to check recovery. But it's not right to check recovery
> after adding dev3. Because the recovery may finish. It depends on the
> recovery performance of the testing machine. If the recovery finishes,
> it will fail. But dev3 is only added as a spare disk, we can't expect
> there is a recovery happens.
> 
> So remove the codes about adding dev3.
> 
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---


Applying this one!

Thanks,
Mariusz

[PATCH 4/5] tests/01r5integ.broken

[Xiao Ni]

01r5integ can be run successfully 152 times without error with
kernel 6.9.0-rc4 and mdadm - v4.3-51-g52bead95. So remove this
one broken case.

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

diff --git a/tests/01r5integ.broken b/tests/01r5integ.broken
deleted file mode 100644
index 207376372243..000000000000
--- a/tests/01r5integ.broken
+++ /dev/null
@@ -1,7 +0,0 @@
-fails rarely
-
-Fails about 1 in every 30 runs with a sha mismatch error:
-
-    c49ab26e1b01def7874af9b8a6d6d0c29fdfafe6 /dev/md0 does not match
-    15dc2f73262f811ada53c65e505ceec9cf025cb9 /dev/md0 with /dev/loop3
-    missing
-- 
2.32.0 (Apple Git-132)

Re: [PATCH 4/5] tests/01r5integ.broken

[Mariusz Tkaczyk]

On Thu, 18 Apr 2024 18:23:20 +0800
Xiao Ni <xni@redhat.com> wrote:

> 01r5integ can be run successfully 152 times without error with
> kernel 6.9.0-rc4 and mdadm - v4.3-51-g52bead95. So remove this
> one broken case.
> 
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---

Applying this one!

Thanks,
Mariusz

[PATCH 5/5] tests/01raid6integ.broken can be removed

[Xiao Ni]

01raid6integ can be run successfully with kernel 6.9.0-rc3.
So remove 01raid6integ.broken.

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

diff --git a/tests/01raid6integ.broken b/tests/01raid6integ.broken
deleted file mode 100644
index 1df735f08c8c..000000000000
--- a/tests/01raid6integ.broken
+++ /dev/null
@@ -1,7 +0,0 @@
-fails infrequently
-
-Fails about 1 in 5 with a sha mismatch:
-
-    8286c2bc045ae2cfe9f8b7ae3a898fa25db6926f /dev/md0 does not match
-    a083a0738b58caab37fd568b91b177035ded37df /dev/md0 with /dev/loop2 and
-    /dev/loop3 missing
-- 
2.32.0 (Apple Git-132)

Re: [PATCH 5/5] tests/01raid6integ.broken can be removed

[Mariusz Tkaczyk]

On Thu, 18 Apr 2024 18:23:21 +0800
Xiao Ni <xni@redhat.com> wrote:

> 01raid6integ can be run successfully with kernel 6.9.0-rc3.
> So remove 01raid6integ.broken.
> 
> Signed-off-by: Xiao Ni <xni@redhat.com>
> ---
I don't follow the *.broken file concept. We could also describe that in comment
in the test, so LGTM for the changes.

If you want to, you can remove all *.broken files and add some comments
in test instead. If we have some tests failing marked as broken long time ago,
you can either remove those scenarios as we are obiously not interested in
fixing those scenarios.

Mariusz

Re: [PATCH 5/5] tests/01raid6integ.broken can be removed

[Song Liu]

On Fri, Apr 19, 2024 at 3:06 AM Mariusz Tkaczyk
<mariusz.tkaczyk@linux.intel.com> wrote:
>
> On Thu, 18 Apr 2024 18:23:21 +0800
> Xiao Ni <xni@redhat.com> wrote:
>
> > 01raid6integ can be run successfully with kernel 6.9.0-rc3.
> > So remove 01raid6integ.broken.
> >
> > Signed-off-by: Xiao Ni <xni@redhat.com>
> > ---
> I don't follow the *.broken file concept. We could also describe that in comment
> in the test, so LGTM for the changes.
>
> If you want to, you can remove all *.broken files and add some comments
> in test instead. If we have some tests failing marked as broken long time ago,
> you can either remove those scenarios as we are obiously not interested in
> fixing those scenarios.

test script has options to skip broken tests (--skip-broken,
--skip-always-broken).
If we remove all the .broken files, we need to update the script to handle
comments in the test file.

Thanks,
Song

Re: [PATCH 5/5] tests/01raid6integ.broken can be removed

[Xiao Ni]

On Fri, Apr 19, 2024 at 11:58 PM Song Liu <song@kernel.org> wrote:
>
> On Fri, Apr 19, 2024 at 3:06 AM Mariusz Tkaczyk
> <mariusz.tkaczyk@linux.intel.com> wrote:
> >
> > On Thu, 18 Apr 2024 18:23:21 +0800
> > Xiao Ni <xni@redhat.com> wrote:
> >
> > > 01raid6integ can be run successfully with kernel 6.9.0-rc3.
> > > So remove 01raid6integ.broken.
> > >
> > > Signed-off-by: Xiao Ni <xni@redhat.com>
> > > ---
> > I don't follow the *.broken file concept. We could also describe that in comment
> > in the test, so LGTM for the changes.
> >
> > If you want to, you can remove all *.broken files and add some comments
> > in test instead. If we have some tests failing marked as broken long time ago,
> > you can either remove those scenarios as we are obiously not interested in
> > fixing those scenarios.
>
> test script has options to skip broken tests (--skip-broken,
> --skip-always-broken).
> If we remove all the .broken files, we need to update the script to handle
> comments in the test file.
>
> Thanks,
> Song
>

Hi all

I'll try to fix all the broken test cases this time and remove the *.broken

Thanks
Xiao

Re: [PATCH 5/5] tests/01raid6integ.broken can be removed

[Mariusz Tkaczyk]

On Mon, 22 Apr 2024 21:54:14 +0800
Xiao Ni <xni@redhat.com> wrote:

> On Fri, Apr 19, 2024 at 11:58 PM Song Liu <song@kernel.org> wrote:
> >
> > On Fri, Apr 19, 2024 at 3:06 AM Mariusz Tkaczyk
> > <mariusz.tkaczyk@linux.intel.com> wrote:  
> > >
> > > On Thu, 18 Apr 2024 18:23:21 +0800
> > > Xiao Ni <xni@redhat.com> wrote:
> > >  
> > > > 01raid6integ can be run successfully with kernel 6.9.0-rc3.
> > > > So remove 01raid6integ.broken.
> > > >
> > > > Signed-off-by: Xiao Ni <xni@redhat.com>
> > > > ---  
> > > I don't follow the *.broken file concept. We could also describe that in
> > > comment in the test, so LGTM for the changes.
> > >
> > > If you want to, you can remove all *.broken files and add some comments
> > > in test instead. If we have some tests failing marked as broken long time
> > > ago, you can either remove those scenarios as we are obiously not
> > > interested in fixing those scenarios.  
> >
> > test script has options to skip broken tests (--skip-broken,
> > --skip-always-broken).
> > If we remove all the .broken files, we need to update the script to handle
> > comments in the test file.
> >
> > Thanks,
> > Song
> >  
> 
> Hi all
> 
> I'll try to fix all the broken test cases this time and remove the *.broken
> 
> Thanks
> Xiao
> 
> 

Anyway, this one removes one .broken file so applying!

Thanks,
Mariusz