[PATCH 0/3] generic/551: improve error reporting and handling

[PATCH 0/3] generic/551: improve error reporting and handling

[Naohiro Aota]

When generic/551 failed, it is difficult to investigate the root cause
because (1) it continues working after an error, corrupting the disk state,
(2) there is no detail info printed, and (3) aio-dio-write-verify failed to
check the event errors.

So, this series addresses these issues.

Naohiro Aota (3):
  aio-dio-write-verify: check for the IO errors
  aio-dio-write-verify: print more info on the error case
  generic/551: bail out test if aio-dio-write-verify failed

 src/aio-dio-regress/aio-dio-write-verify.c | 17 ++++++++++++++---
 tests/generic/551                          |  6 ++++--
 2 files changed, 18 insertions(+), 5 deletions(-)

-- 
2.41.0

[PATCH 1/3] aio-dio-write-verify: check for the IO errors

[Naohiro Aota]

The async write IOs can return some errors, which may lead to a short read or
corruption in io_verify() stage. Catch an error early to identify the root
cause easily.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 src/aio-dio-regress/aio-dio-write-verify.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/aio-dio-regress/aio-dio-write-verify.c b/src/aio-dio-regress/aio-dio-write-verify.c
index 302b8fe4a096..90e41b391289 100644
--- a/src/aio-dio-regress/aio-dio-write-verify.c
+++ b/src/aio-dio-regress/aio-dio-write-verify.c
@@ -238,6 +238,16 @@ static int io_write(int fd, int num_events)
 		return 1;
 	}
 
+	for (i = 0; i < num_events; i++) {
+		int err = (int)evs[i].res;
+
+		if (err < 0) {
+			fprintf(stderr, "error %s with event %d\n",
+				strerror(err), i);
+			return 1;
+		}
+	}
+
 	/* Try to destroy at here, not necessary, so don't check result */
 	io_destroy(ctx);
 
-- 
2.41.0

Re: [PATCH 1/3] aio-dio-write-verify: check for the IO errors

[Zorro Lang]

On Tue, Aug 22, 2023 at 04:28:50PM +0900, Naohiro Aota wrote:
> The async write IOs can return some errors, which may lead to a short read or
> corruption in io_verify() stage. Catch an error early to identify the root
> cause easily.
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---

Looks good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  src/aio-dio-regress/aio-dio-write-verify.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/src/aio-dio-regress/aio-dio-write-verify.c b/src/aio-dio-regress/aio-dio-write-verify.c
> index 302b8fe4a096..90e41b391289 100644
> --- a/src/aio-dio-regress/aio-dio-write-verify.c
> +++ b/src/aio-dio-regress/aio-dio-write-verify.c
> @@ -238,6 +238,16 @@ static int io_write(int fd, int num_events)
>  		return 1;
>  	}
>  
> +	for (i = 0; i < num_events; i++) {
> +		int err = (int)evs[i].res;
> +
> +		if (err < 0) {
> +			fprintf(stderr, "error %s with event %d\n",
> +				strerror(err), i);
> +			return 1;
> +		}
> +	}
> +
>  	/* Try to destroy at here, not necessary, so don't check result */
>  	io_destroy(ctx);
>  
> -- 
> 2.41.0
> 

[PATCH 2/3] aio-dio-write-verify: print more info on the error case

[Naohiro Aota]

When short read or corruption happened, it is difficult to locate which IO
event failed. Print the address to make it identifiable.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 src/aio-dio-regress/aio-dio-write-verify.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/aio-dio-regress/aio-dio-write-verify.c b/src/aio-dio-regress/aio-dio-write-verify.c
index 90e41b391289..dabbfacde3f8 100644
--- a/src/aio-dio-regress/aio-dio-write-verify.c
+++ b/src/aio-dio-regress/aio-dio-write-verify.c
@@ -267,13 +267,14 @@ static int io_verify(int fd)
 			perror("pread");
 			return 1;
 		} else if (sret != p->param->buf_size) {
-			fprintf(stderr, "short read %zd was less than %zu\n",
-			        sret, p->param->buf_size);
+			fprintf(stderr, "short read %zd was less than %zu at %zu\n",
+			        sret, p->param->buf_size, p->param->offset);
 			return 1;
 		}
 		if (memcmp(p->param->buf,
 		           p->param->cmp_buf, p->param->buf_size)) {
-			printf("Find corruption\n");
+			printf("Find corruption at %zu length %zu\n", p->param->offset,
+			       p->param->buf_size);
 			dump_buffer(p->param->buf, p->param->offset,
 			            p->param->buf_size);
 			corrupted++;
-- 
2.41.0

Re: [PATCH 2/3] aio-dio-write-verify: print more info on the error case

[Zorro Lang]

On Tue, Aug 22, 2023 at 04:28:51PM +0900, Naohiro Aota wrote:
> When short read or corruption happened, it is difficult to locate which IO
> event failed. Print the address to make it identifiable.
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  src/aio-dio-regress/aio-dio-write-verify.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/src/aio-dio-regress/aio-dio-write-verify.c b/src/aio-dio-regress/aio-dio-write-verify.c
> index 90e41b391289..dabbfacde3f8 100644
> --- a/src/aio-dio-regress/aio-dio-write-verify.c
> +++ b/src/aio-dio-regress/aio-dio-write-verify.c
> @@ -267,13 +267,14 @@ static int io_verify(int fd)
>  			perror("pread");
>  			return 1;
>  		} else if (sret != p->param->buf_size) {
> -			fprintf(stderr, "short read %zd was less than %zu\n",
> -			        sret, p->param->buf_size);
> +			fprintf(stderr, "short read %zd was less than %zu at %zu\n",
> +			        sret, p->param->buf_size, p->param->offset);
>  			return 1;
>  		}
>  		if (memcmp(p->param->buf,
>  		           p->param->cmp_buf, p->param->buf_size)) {
> -			printf("Find corruption\n");
> +			printf("Find corruption at %zu length %zu\n", p->param->offset,
> +			       p->param->buf_size);
>  			dump_buffer(p->param->buf, p->param->offset,
>  			            p->param->buf_size);
>  			corrupted++;
> -- 
> 2.41.0
> 

[PATCH 3/3] generic/551: bail out test if aio-dio-write-verify failed

[Naohiro Aota]

When the AIO program failed, it is better to bail out the test to keep the
failed state intact.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 tests/generic/551 | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/generic/551 b/tests/generic/551
index 2689270aa3dd..58c7f9a516c7 100755
--- a/tests/generic/551
+++ b/tests/generic/551
@@ -56,16 +56,18 @@ do_test()
 	truncsize=$(((RANDOM * diosize + RANDOM % diosize) % max_io_size_b))
 
 	$AIO_TEST -t $truncsize $oper_list $localfile
-	if [ $? -ne 0 ];then
+	ret=$?
+	if [ $ret -ne 0 ];then
 		echo "$AIO_TEST -t $truncsize $oper_list $localfile"
 		echo "==========^^ Fail ^^=========="
 	fi
+	return $ret
 }
 
 testimes=$((LOAD_FACTOR * 100))
 while [ $testimes -gt 0 ]; do
 	echo > $localfile
-	do_test
+	do_test || break
 	((testimes--))
 done
 
-- 
2.41.0

Re: [PATCH 3/3] generic/551: bail out test if aio-dio-write-verify failed

[Zorro Lang]

On Tue, Aug 22, 2023 at 04:28:52PM +0900, Naohiro Aota wrote:
> When the AIO program failed, it is better to bail out the test to keep the
> failed state intact.
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/generic/551 | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/generic/551 b/tests/generic/551
> index 2689270aa3dd..58c7f9a516c7 100755
> --- a/tests/generic/551
> +++ b/tests/generic/551
> @@ -56,16 +56,18 @@ do_test()
>  	truncsize=$(((RANDOM * diosize + RANDOM % diosize) % max_io_size_b))
>  
>  	$AIO_TEST -t $truncsize $oper_list $localfile
> -	if [ $? -ne 0 ];then
> +	ret=$?
> +	if [ $ret -ne 0 ];then
>  		echo "$AIO_TEST -t $truncsize $oper_list $localfile"
>  		echo "==========^^ Fail ^^=========="
>  	fi
> +	return $ret
>  }
>  
>  testimes=$((LOAD_FACTOR * 100))
>  while [ $testimes -gt 0 ]; do
>  	echo > $localfile
> -	do_test
> +	do_test || break
>  	((testimes--))
>  done
>  
> -- 
> 2.41.0
>