[PATCH 0/5] mdadm/test: Update for mdadm test part

[PATCH 0/5] mdadm/test: Update for mdadm test part

[Zhilong Liu]

Hi, Jes;
  Here update trivial fixes for test part, add new test case
for readonly/readwrite feature and get rid of unused files.

Thanks,
Zhilong

Zhilong Liu (5):
  mdadm/bitmap: examine-bitmap failed when bitmap is external mode
  mdadm/test: use the first element of array as parsing condition
  mdadm/test: get rid of the tests/testdev
  mdadm/test: get rid of tests/check
  mdadm/test: add new testcase for testing readonly/readwrite

 bitmap.c         | 13 ++++++++-----
 test             |  2 +-
 tests/00readonly | 22 ++++++++++++++++++++++
 tests/check      | 35 -----------------------------------
 tests/testdev    | 13 -------------
 5 files changed, 31 insertions(+), 54 deletions(-)
 create mode 100644 tests/00readonly
 delete mode 100644 tests/check
 delete mode 100644 tests/testdev

-- 
2.6.6

[PATCH 1/5] mdadm/bitmap: examine-bitmap failed when bitmap is external mode

[Zhilong Liu]

--examine-bitmap: the bitmap_file_open() shouldn't omit the
regular file descriptor when the bitmap is external mode.
Such as: ./mdadm -X /mnt/3

This commit is partial revert of commit 0a6bff09d416
(mdadm/util: unify fstat checking blkdev into function)

Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
 bitmap.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/bitmap.c b/bitmap.c
index 3653660..de5f9db 100644
--- a/bitmap.c
+++ b/bitmap.c
@@ -183,6 +183,7 @@ static int
 bitmap_file_open(char *filename, struct supertype **stp, int node_num)
 {
 	int fd;
+	struct stat stb;
 	struct supertype *st = *stp;
 
 	fd = open(filename, O_RDONLY|O_DIRECT);
@@ -192,7 +193,13 @@ bitmap_file_open(char *filename, struct supertype **stp, int node_num)
 		return -1;
 	}
 
-	if (fstat_is_blkdev(fd, filename, NULL)) {
+	if (fstat(fd, &stb) < 0) {
+		pr_err("fstat failed for %s: %s\n",
+			filename, strerror(errno));
+		close(fd);
+		return -1;
+	}
+	if ((stb.st_mode & S_IFMT) == S_IFBLK) {
 		/* block device, so we are probably after an internal bitmap */
 		if (!st)
 			st = guess_super(fd);
@@ -211,11 +218,7 @@ bitmap_file_open(char *filename, struct supertype **stp, int node_num)
 				fd = -1;
 			}
 		}
-
 		*stp = st;
-	} else {
-		close(fd);
-		return -1;
 	}
 
 	return fd;
-- 
2.6.6

[PATCH 2/5] mdadm/test: use the first element of array as parsing condition

[Zhilong Liu]

it would complain "too many arguments" when array[] has
few members, it's proper to check whether or not array[0]
is null.

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

diff --git a/test b/test
index 87e2df2..25268a0 100755
--- a/test
+++ b/test
@@ -75,7 +75,7 @@ save_log() {
 		echo
 	elif [ "$DEVTYPE" == 'loop' -o "$DEVTYPE" == 'disk' ]
 	then
-		if [ ! -z ${array[@]} -a ${#array[@]} -ge 1 ]
+		if [ ! -z "$array" -a ${#array[@]} -ge 1 ]
 		then
 			md_disks=($($mdadm -D -Y ${array[@]} | grep "/dev/" | cut -d'=' -f2))
 			cat /proc/mdstat | grep -q "bitmap"
-- 
2.6.6

[PATCH 3/5] mdadm/test: get rid of the tests/testdev

[Zhilong Liu]

It's time to get rid of the tests/testdev due
to it has covered by testdev() in new 'test'.

Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
 tests/testdev | 13 -------------
 1 file changed, 13 deletions(-)
 delete mode 100644 tests/testdev

diff --git a/tests/testdev b/tests/testdev
deleted file mode 100644
index 8b6e6f0..0000000
--- a/tests/testdev
+++ /dev/null
@@ -1,13 +0,0 @@
-dev=$1
-cnt=$2
-size=$3
-chunk=$4
-mkfs -j $dev > /dev/null 2>&1 && fsck -fn $dev >&2
-dsize=$[size/chunk]
-dsize=$[dsize*chunk]
-rasize=$[dsize*2*cnt]
-if [ $rasize -ne `/sbin/blockdev --getsize $dev` ]
-then
-  echo "ERROR: size is wrong for $dev: $cnt * $size (chunk=$chunk) = $rasize, not `/sbin/blockdev --getsize $dev`"
-  exit 1;
-fi
-- 
2.6.6

[PATCH 4/5] mdadm/test: get rid of tests/check

[Zhilong Liu]

The tests/check can be removed due to check()
in new 'test' has covered various checking.

Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
 tests/check | 35 -----------------------------------
 1 file changed, 35 deletions(-)
 delete mode 100644 tests/check

diff --git a/tests/check b/tests/check
deleted file mode 100644
index f4ed6d5..0000000
--- a/tests/check
+++ /dev/null
@@ -1,35 +0,0 @@
-
-case $1 in
- raid* | linear )
-   grep -s "active $1 " /proc/mdstat > /dev/null || {
-		echo >&2 "ERROR active $1 not found" ; cat /proc/mdstat ; exit 1;}
-  ;;
- resync | recovery )
-   sleep 0.1
-   grep -s $1 /proc/mdstat > /dev/null || {
-		echo >&2 ERROR no $1 happening; cat /proc/mdstat; exit 1; }
-  ;;
-
-  nosync )
-    sleep 0.5
-    grep -s 're[synccovery]* =' > /dev/null /proc/mdstat && {
-		echo >&2 "ERROR resync or recovery is happening!"; cat /proc/mdstat ; exit 1; }
-  ;;
-
- wait )
-   sleep 0.1
-   while grep 're[synccovery]* =' > /dev/null /proc/mdstat
-   do sleep 2;
-   done
-   ;;
-
- state )
-    grep -s "blocks.*\[$2\]\$" /proc/mdstat > /dev/null || {
-		echo >&2 "ERROR state $2 not found!"; cat /proc/mdstat ; exit 1; }
-    sleep 0.5
-   ;;
-
- * ) echo >&2 ERROR unknown check $1 ; exit 1;
-esac
-
-exit 0
-- 
2.6.6

[PATCH 5/5] mdadm/test: add new testcase for testing readonly/readwrite

[Zhilong Liu]

This is a test case for testing --readonly and
--readwrite feature, it covers common metadata
versions and raid levels.

Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
 tests/00readonly | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 tests/00readonly

diff --git a/tests/00readonly b/tests/00readonly
new file mode 100644
index 0000000..28b0fa1
--- /dev/null
+++ b/tests/00readonly
@@ -0,0 +1,22 @@
+#!/bin/bash
+
+for metadata in 0.9 1.0 1.1 1.2
+do
+	for level in linear raid0 raid1 raid4 raid5 raid6 raid10
+	do
+		mdadm -CR $md0 -l $level -n 4 --metadata=$metadata \
+			$dev1 $dev2 $dev3 $dev4 --assume-clean
+		check nosync
+		check $level
+		mdadm -ro $md0
+		check readonly
+		state=$(cat /sys/block/md0/md/array_state)
+		[ "$state" == "readonly" ] ||
+			die "array_state should be 'readonly', but is $state"
+		mdadm -w $md0
+		check $level
+		mdadm -S $md0
+	done
+done
+
+exit 0
-- 
2.6.6

Re: [PATCH 1/5] mdadm/bitmap: examine-bitmap failed when bitmap is external mode

[Jes Sorensen]

On 08/28/2017 05:24 AM, Zhilong Liu wrote:
> --examine-bitmap: the bitmap_file_open() shouldn't omit the
> regular file descriptor when the bitmap is external mode.
> Such as: ./mdadm -X /mnt/3
> 
> This commit is partial revert of commit 0a6bff09d416
> (mdadm/util: unify fstat checking blkdev into function)
> 
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
> ---
>   bitmap.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)

I am not in love with this approach, but I guess we have to go with it.

I have applied the patch with one minor change, the pr_err() portion 
should respect that code is 80 characters across.

Cheers,
Jes

> diff --git a/bitmap.c b/bitmap.c
> index 3653660..de5f9db 100644
> --- a/bitmap.c
> +++ b/bitmap.c
> @@ -183,6 +183,7 @@ static int
>   bitmap_file_open(char *filename, struct supertype **stp, int node_num)
>   {
>   	int fd;
> +	struct stat stb;
>   	struct supertype *st = *stp;
>   
>   	fd = open(filename, O_RDONLY|O_DIRECT);
> @@ -192,7 +193,13 @@ bitmap_file_open(char *filename, struct supertype **stp, int node_num)
>   		return -1;
>   	}
>   
> -	if (fstat_is_blkdev(fd, filename, NULL)) {
> +	if (fstat(fd, &stb) < 0) {
> +		pr_err("fstat failed for %s: %s\n",
> +			filename, strerror(errno));
> +		close(fd);
> +		return -1;
> +	}
> +	if ((stb.st_mode & S_IFMT) == S_IFBLK) {
>   		/* block device, so we are probably after an internal bitmap */
>   		if (!st)
>   			st = guess_super(fd);
> @@ -211,11 +218,7 @@ bitmap_file_open(char *filename, struct supertype **stp, int node_num)
>   				fd = -1;
>   			}
>   		}
> -
>   		*stp = st;
> -	} else {
> -		close(fd);
> -		return -1;
>   	}
>   
>   	return fd;
> 

Re: [PATCH 2/5] mdadm/test: use the first element of array as parsing condition

[Jes Sorensen]

On 08/28/2017 05:24 AM, Zhilong Liu wrote:
> it would complain "too many arguments" when array[] has
> few members, it's proper to check whether or not array[0]
> is null.
> 
> Reported-by: Xiao Ni <xni@redhat.com>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
> ---
>   test | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Applied!

Thanks,
Jes

Re: [PATCH 3/5] mdadm/test: get rid of the tests/testdev

[Jes Sorensen]

On 08/28/2017 05:24 AM, Zhilong Liu wrote:
> It's time to get rid of the tests/testdev due
> to it has covered by testdev() in new 'test'.
> 
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
> ---
>   tests/testdev | 13 -------------
>   1 file changed, 13 deletions(-)
>   delete mode 100644 tests/testdev

Applied

Thanks,
Jes

Re: [PATCH 4/5] mdadm/test: get rid of tests/check

[Jes Sorensen]

On 08/28/2017 05:24 AM, Zhilong Liu wrote:
> The tests/check can be removed due to check()
> in new 'test' has covered various checking.
> 
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
> ---
>   tests/check | 35 -----------------------------------
>   1 file changed, 35 deletions(-)
>   delete mode 100644 tests/check

Applied!

Thanks,
Jes

Re: [PATCH 5/5] mdadm/test: add new testcase for testing readonly/readwrite

[Jes Sorensen]

On 08/28/2017 05:24 AM, Zhilong Liu wrote:
> This is a test case for testing --readonly and
> --readwrite feature, it covers common metadata
> versions and raid levels.
> 
> Signed-off-by: Zhilong Liu <zlliu@suse.com>

Applied,

Thanks,
Jes