[PATCH 0/5] tiny fixes for xfstests

[PATCH 0/5] tiny fixes for xfstests

[Hou Tao]

Hou Tao (5):
  check: use _try_scratch_mount instead of _scratch_mount to mount
    SCRATCH_DEV
  generic/019: check scratch device is a block device or not
  generic/131: wait until the server is ready
  generic/466: explicitly request $SCRATCH_DEV to be a block device
  fsx: check ENOSYS in test_copy_range()

 check             |  2 +-
 ltp/fsx.c         |  2 +-
 tests/generic/019 |  1 +
 tests/generic/131 | 19 +++++++++++++------
 tests/generic/466 |  1 +
 5 files changed, 17 insertions(+), 8 deletions(-)

-- 
2.16.2.dirty

[PATCH 1/5] check: use _try_scratch_mount instead of _scratch_mount to mount SCRATCH_DEV

[Hou Tao]

Else there won't be any error messages when mounting SCRATCH_DEV failed,
because _scratch_mount exits early by invoking _fail.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 check | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/check b/check
index e85f40ac..e4d76737 100755
--- a/check
+++ b/check
@@ -607,7 +607,7 @@ for section in $HOST_OPTIONS_SECTIONS; do
 
 	  # call the overridden mount - make sure the FS mounts with
 	  # the same options that we'll mount with later.
-	  if ! _scratch_mount >$tmp.err 2>&1
+	  if ! _try_scratch_mount >$tmp.err 2>&1
 	  then
 	      echo "our local mount routine ..."
 	      cat $tmp.err
-- 
2.16.2.dirty

[PATCH 2/5] generic/019: check scratch device is a block device or not

[Hou Tao]

To ensure "blockdev --getsz $SCRATCH_DEV" will succeed.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 tests/generic/019 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/generic/019 b/tests/generic/019
index 32db36fc..8e1c9331 100755
--- a/tests/generic/019
+++ b/tests/generic/019
@@ -22,6 +22,7 @@ status=1	# failure is the default!
 _supported_fs generic
 _supported_os Linux
 _require_scratch
+_require_block_device $SCRATCH_DEV
 _require_fail_make_request
 
 SYSFS_BDEV=`_sysfs_dev $SCRATCH_DEV`
-- 
2.16.2.dirty

[PATCH 3/5] generic/131: wait until the server is ready

[Hou Tao]

When running xfstests under KVM VM and the load of host is high,
only delaying 1s and checking the readiness of server are not
enough, and the test case will fail early.

Fix it by repeatedly checking the readiness signal until it's found,
or the server exits.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 tests/generic/131 | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/tests/generic/131 b/tests/generic/131
index d7c146ae..1af86e77 100755
--- a/tests/generic/131
+++ b/tests/generic/131
@@ -37,13 +37,20 @@ TESTFILE=$TEST_DIR/lock_file
 src/locktest $TESTFILE 2>&1 > $TEST_DIR/server.out &
 locktest_pid1=$!
 
-sleep 1
+while true; do
+	sleep 1
 
-PORT=$(cat $TEST_DIR/server.out | grep "^server port: " | awk '{print $3}')
-if [ -z $PORT ]; then
-	echo "Could not get server port"
-	exit 1
-fi
+	PORT=$(cat $TEST_DIR/server.out | grep "^server port: " | awk '{print $3}')
+	if [ -n "$PORT" ]; then
+		break
+	fi
+
+	# check the existence of server process
+	if ! kill -s 0 $locktest_pid1 >/dev/null 2>&1; then
+		echo "Could not get server port"
+		exit 1
+	fi
+done
 
 # Start the client
 src/locktest -p $PORT -h localhost $TESTFILE 2>&1 > $TEST_DIR/client.out
-- 
2.16.2.dirty

Re: [PATCH 3/5] generic/131: wait until the server is ready

[Eryu Guan]

On Sat, Dec 29, 2018 at 10:34:50AM +0800, Hou Tao wrote:
> When running xfstests under KVM VM and the load of host is high,
> only delaying 1s and checking the readiness of server are not
> enough, and the test case will fail early.
> 
> Fix it by repeatedly checking the readiness signal until it's found,
> or the server exits.
> 
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
>  tests/generic/131 | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/generic/131 b/tests/generic/131
> index d7c146ae..1af86e77 100755
> --- a/tests/generic/131
> +++ b/tests/generic/131
> @@ -37,13 +37,20 @@ TESTFILE=$TEST_DIR/lock_file
>  src/locktest $TESTFILE 2>&1 > $TEST_DIR/server.out &
>  locktest_pid1=$!
>  
> -sleep 1
> +while true; do
> +	sleep 1

Looks like we'll loop forever if the locktest process is there but fails
to print out the server port due to random reasons.

I think it's better to retry N times and give up if the server is still
not ready.

Thanks,
Eryu

>  
> -PORT=$(cat $TEST_DIR/server.out | grep "^server port: " | awk '{print $3}')
> -if [ -z $PORT ]; then
> -	echo "Could not get server port"
> -	exit 1
> -fi
> +	PORT=$(cat $TEST_DIR/server.out | grep "^server port: " | awk '{print $3}')
> +	if [ -n "$PORT" ]; then
> +		break
> +	fi
> +
> +	# check the existence of server process
> +	if ! kill -s 0 $locktest_pid1 >/dev/null 2>&1; then
> +		echo "Could not get server port"
> +		exit 1
> +	fi
> +done
>  
>  # Start the client
>  src/locktest -p $PORT -h localhost $TESTFILE 2>&1 > $TEST_DIR/client.out
> -- 
> 2.16.2.dirty
> 

Re: [PATCH 3/5] generic/131: wait until the server is ready

[Hou Tao]

Hi,

On 2018/12/29 16:14, Eryu Guan wrote:
> On Sat, Dec 29, 2018 at 10:34:50AM +0800, Hou Tao wrote:
>> When running xfstests under KVM VM and the load of host is high,
>> only delaying 1s and checking the readiness of server are not
>> enough, and the test case will fail early.
>>
>> Fix it by repeatedly checking the readiness signal until it's found,
>> or the server exits.
>>
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>> ---
>>  tests/generic/131 | 19 +++++++++++++------
>>  1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/generic/131 b/tests/generic/131
>> index d7c146ae..1af86e77 100755
>> --- a/tests/generic/131
>> +++ b/tests/generic/131
>> @@ -37,13 +37,20 @@ TESTFILE=$TEST_DIR/lock_file
>>  src/locktest $TESTFILE 2>&1 > $TEST_DIR/server.out &
>>  locktest_pid1=$!
>>  
>> -sleep 1
>> +while true; do
>> +	sleep 1
> 
> Looks like we'll loop forever if the locktest process is there but fails
> to print out the server port due to random reasons.
> 
> I think it's better to retry N times and give up if the server is still
> not ready.
> 
Thanks for your suggestion, will send v2 soon.

> Thanks,
> Eryu
> 
>>  
>> -PORT=$(cat $TEST_DIR/server.out | grep "^server port: " | awk '{print $3}')
>> -if [ -z $PORT ]; then
>> -	echo "Could not get server port"
>> -	exit 1
>> -fi
>> +	PORT=$(cat $TEST_DIR/server.out | grep "^server port: " | awk '{print $3}')
>> +	if [ -n "$PORT" ]; then
>> +		break
>> +	fi
>> +
>> +	# check the existence of server process
>> +	if ! kill -s 0 $locktest_pid1 >/dev/null 2>&1; then
>> +		echo "Could not get server port"
>> +		exit 1
>> +	fi
>> +done
>>  
>>  # Start the client
>>  src/locktest -p $PORT -h localhost $TESTFILE 2>&1 > $TEST_DIR/client.out
>> -- 
>> 2.16.2.dirty
>>
> 
> 

[PATCH 4/5] generic/466: explicitly request $SCRATCH_DEV to be a block device

[Hou Tao]

so "blockdev --getsize64 $SCRATCH_DEV" will succeed.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 tests/generic/466 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/generic/466 b/tests/generic/466
index bc577df7..8168b5be 100755
--- a/tests/generic/466
+++ b/tests/generic/466
@@ -29,6 +29,7 @@ _cleanup()
 _supported_os Linux
 _supported_fs generic
 _require_scratch_nocheck
+_require_block_device $SCRATCH_DEV
 
 rm -f $seqres.full
 
-- 
2.16.2.dirty

[PATCH 5/5] fsx: check ENOSYS in test_copy_range()

[Hou Tao]

In configure script, we only check whether or not the build
of copy-range test program is successful, but that doesn't
mean the kernel has implemented the syscall, so checking
for this case.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 ltp/fsx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ltp/fsx.c b/ltp/fsx.c
index 316f08eb..b84180f7 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -1583,7 +1583,7 @@ test_copy_range(void)
 	loff_t o1 = 0, o2 = 0;
 
 	if (syscall(__NR_copy_file_range, fd, &o1, fd, &o2, 0, 0) == -1 &&
-	    (errno == EOPNOTSUPP || errno == ENOTTY)) {
+	    (errno == ENOSYS || errno == EOPNOTSUPP || errno == ENOTTY)) {
 		if (!quiet)
 			fprintf(stderr,
 				"main: filesystem does not support "
-- 
2.16.2.dirty

Re: [PATCH 5/5] fsx: check ENOSYS in test_copy_range()

[Eryu Guan]

On Sat, Dec 29, 2018 at 10:34:52AM +0800, Hou Tao wrote:
> In configure script, we only check whether or not the build
> of copy-range test program is successful, but that doesn't
> mean the kernel has implemented the syscall, so checking
> for this case.
> 
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
>  ltp/fsx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index 316f08eb..b84180f7 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -1583,7 +1583,7 @@ test_copy_range(void)
>  	loff_t o1 = 0, o2 = 0;
>  
>  	if (syscall(__NR_copy_file_range, fd, &o1, fd, &o2, 0, 0) == -1 &&
> -	    (errno == EOPNOTSUPP || errno == ENOTTY)) {
> +	    (errno == ENOSYS || errno == EOPNOTSUPP || errno == ENOTTY)) {

Should we also check for ENOSYS in other operations like
{clone,dedupe}_range and fallocate?

Other fixes look good to me, thanks a lot for the fixes!

Thanks,
Eryu

>  		if (!quiet)
>  			fprintf(stderr,
>  				"main: filesystem does not support "
> -- 
> 2.16.2.dirty
> 

Re: [PATCH 5/5] fsx: check ENOSYS in test_copy_range()

[Darrick J. Wong]

On Sat, Dec 29, 2018 at 04:16:19PM +0800, Eryu Guan wrote:
> On Sat, Dec 29, 2018 at 10:34:52AM +0800, Hou Tao wrote:
> > In configure script, we only check whether or not the build
> > of copy-range test program is successful, but that doesn't
> > mean the kernel has implemented the syscall, so checking
> > for this case.
> > 
> > Signed-off-by: Hou Tao <houtao1@huawei.com>
> > ---
> >  ltp/fsx.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/ltp/fsx.c b/ltp/fsx.c
> > index 316f08eb..b84180f7 100644
> > --- a/ltp/fsx.c
> > +++ b/ltp/fsx.c
> > @@ -1583,7 +1583,7 @@ test_copy_range(void)
> >  	loff_t o1 = 0, o2 = 0;
> >  
> >  	if (syscall(__NR_copy_file_range, fd, &o1, fd, &o2, 0, 0) == -1 &&
> > -	    (errno == EOPNOTSUPP || errno == ENOTTY)) {
> > +	    (errno == ENOSYS || errno == EOPNOTSUPP || errno == ENOTTY)) {
> 
> Should we also check for ENOSYS in other operations like
> {clone,dedupe}_range

Unimplemented ioctls usually return EOPNOTSUPP or ENOTTY, not ENOSYS, so
I don't think that's necessary.... unless there are kernels that don't
implement the ioctl syscall?

In any case, the manpage for clone and dedupe don't say anything about
returning ENOSYS.

> and fallocate?

Yes.

--D

> Other fixes look good to me, thanks a lot for the fixes!
> 
> Thanks,
> Eryu
> 
> >  		if (!quiet)
> >  			fprintf(stderr,
> >  				"main: filesystem does not support "
> > -- 
> > 2.16.2.dirty
> > 

Re: [PATCH 5/5] fsx: check ENOSYS in test_copy_range()

[Hou Tao]

Hi,

On 2018/12/30 2:56, Darrick J. Wong wrote:
> On Sat, Dec 29, 2018 at 04:16:19PM +0800, Eryu Guan wrote:
>> On Sat, Dec 29, 2018 at 10:34:52AM +0800, Hou Tao wrote:
>>> In configure script, we only check whether or not the build
>>> of copy-range test program is successful, but that doesn't
>>> mean the kernel has implemented the syscall, so checking
>>> for this case.
>>>
>>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>>> ---
>>>  ltp/fsx.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/ltp/fsx.c b/ltp/fsx.c
>>> index 316f08eb..b84180f7 100644
>>> --- a/ltp/fsx.c
>>> +++ b/ltp/fsx.c
>>> @@ -1583,7 +1583,7 @@ test_copy_range(void)
>>>  	loff_t o1 = 0, o2 = 0;
>>>  
>>>  	if (syscall(__NR_copy_file_range, fd, &o1, fd, &o2, 0, 0) == -1 &&
>>> -	    (errno == EOPNOTSUPP || errno == ENOTTY)) {
>>> +	    (errno == ENOSYS || errno == EOPNOTSUPP || errno == ENOTTY)) {
>>
>> Should we also check for ENOSYS in other operations like
>> {clone,dedupe}_range
> 
> Unimplemented ioctls usually return EOPNOTSUPP or ENOTTY, not ENOSYS, so
> I don't think that's necessary.... unless there are kernels that don't
> implement the ioctl syscall?
> 
> In any case, the manpage for clone and dedupe don't say anything about
> returning ENOSYS.
> 
>> and fallocate?
> > Yes.
I will send a v2 to also check test_fallocate(), and thanks for your review.

> 
> --D
> 
>> Other fixes look good to me, thanks a lot for the fixes!
>>
>> Thanks,
>> Eryu
>>
>>>  		if (!quiet)
>>>  			fprintf(stderr,
>>>  				"main: filesystem does not support "
>>> -- 
>>> 2.16.2.dirty
>>>
> 
> 

Re: [PATCH 5/5] fsx: check ENOSYS in test_copy_range()

[Darrick J. Wong]

On Sat, Dec 29, 2018 at 10:34:52AM +0800, Hou Tao wrote:
> In configure script, we only check whether or not the build
> of copy-range test program is successful, but that doesn't
> mean the kernel has implemented the syscall, so checking
> for this case.
> 
> Signed-off-by: Hou Tao <houtao1@huawei.com>

Err, oops.  Thanks for catching that!

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  ltp/fsx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index 316f08eb..b84180f7 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -1583,7 +1583,7 @@ test_copy_range(void)
>  	loff_t o1 = 0, o2 = 0;
>  
>  	if (syscall(__NR_copy_file_range, fd, &o1, fd, &o2, 0, 0) == -1 &&
> -	    (errno == EOPNOTSUPP || errno == ENOTTY)) {
> +	    (errno == ENOSYS || errno == EOPNOTSUPP || errno == ENOTTY)) {
>  		if (!quiet)
>  			fprintf(stderr,
>  				"main: filesystem does not support "
> -- 
> 2.16.2.dirty
>