[PATCH blktests v2 0/2] fix nbd/002

[PATCH blktests v2 0/2] fix nbd/002

[Shin'ichiro Kawasaki]

Recently, CKI project found blktests nbd/002 failure. The test case sets up the
connection to the nbd device, then checks partition existence in the device to
confirm that the kernel reads the partition table in the device. Usually, this
partition read by kernel is completed before the test script checks the
partition existence. However, the partition read often completes after the
partition existence check, then the test case fails. I think the test script
checks the partition existence too early, and this should be fixed in the test
script.

During this investigation, I noticed that the test case nbd/002 handles the
ioctl interface and the netlink interface opposite. The first patch fixes this
wrong interface handling. The second patch addresses the too early partition
existence check issue.

Link to v1 patch: https://lore.kernel.org/linux-block/20240319085015.3901051-1-shinichiro.kawasaki@wdc.com/

Changes from v1:
* Added another patch to fix ioctl/netlink interface handling mistake
* Avoid the nbd/002 failure by repeating the partition existence check

Shin'ichiro Kawasaki (2):
  nbd/002: fix wrong -L/-nonetlink option usage
  nbd/002: repeat partition existence check for ioctl interface

 tests/nbd/002 | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

-- 
2.44.0

[PATCH blktests v2 1/2] nbd/002: fix wrong -L/-nonetlink option usage

[Shin'ichiro Kawasaki]

As the commit 3c014acd5171 ("nbd/001: use -L for nbd-client") explains,
the nbd-client command uses the netlink interface instead of the ioctl
interface. The default interface changed at nbd version 3.17 in March
2018. Before that, the default was ioctl. After the change, the
nbd-client command requires -L or -nonetlink option to use the ioctl
interface.

The commit 3c014acd5171 adjusted nbd/001 test script to the default
interface change. However, it is not reflected to nbd/002. This caused
mismatch between the comments in the test case and the actual test. The
comments describe the first half as "Do it with ioctls", and the last
half as "Do it with netlink". However, the test script does opposite. It
specifies no option for the first half, then tests with netlink
interface. It specifies -L option for the last half, then tests with the
ioctl interface.

This makes it difficult to debug the failure of the test case. Fix the
nbd-client command option to match the comments. Also, use the long
option -nonetlink instead of -L for easier reading.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 tests/nbd/002 | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/nbd/002 b/tests/nbd/002
index fd992a0..968c9fa 100755
--- a/tests/nbd/002
+++ b/tests/nbd/002
@@ -53,11 +53,11 @@ test() {
 
 	echo "Testing IOCTL path"
 
-	nbd-client -N export localhost /dev/nbd0 >> "$FULL" 2>&1
+	nbd-client -nonetlink -N export localhost /dev/nbd0 >> "$FULL" 2>&1
 
 	if ! _wait_for_nbd_connect; then
 		echo "Connect didn't happen?"
-		nbd-client -d /dev/nbd0 >> "$FULL" 2>&1
+		nbd-client -nonetlink -d /dev/nbd0 >> "$FULL" 2>&1
 		_stop_nbd_server
 		return 1
 	fi
@@ -66,12 +66,12 @@ test() {
 
 	if ! stat /dev/nbd0p1 >> "$FULL" 2>&1; then
 		echo "Didn't have partition on ioctl path"
-		nbd-client -d /dev/nbd0 >> "$FULL" 2>&1
+		nbd-client -nonetlink -d /dev/nbd0 >> "$FULL" 2>&1
 		_stop_nbd_server
 		return 1
 	fi
 
-	nbd-client -d /dev/nbd0 >> "$FULL" 2>&1
+	nbd-client -nonetlink -d /dev/nbd0 >> "$FULL" 2>&1
 
 	udevadm settle
 
@@ -83,7 +83,7 @@ test() {
 
 	# Do it with netlink
 	echo "Testing the netlink path"
-	nbd-client -L -N export localhost /dev/nbd0 >> "$FULL" 2>&1
+	nbd-client -N export localhost /dev/nbd0 >> "$FULL" 2>&1
 
 	if ! _wait_for_nbd_connect; then
 		echo "Connect didn't happen?"
@@ -96,12 +96,12 @@ test() {
 
 	if  ! stat /dev/nbd0p1 >/dev/null 2>&1; then
 		echo "Didn't have partition on the netlink path"
-		nbd-client -L -d /dev/nbd0 >> "$FULL" 2>&1
+		nbd-client -d /dev/nbd0 >> "$FULL" 2>&1
 		_stop_nbd_server
 		return 1
 	fi
 
-	nbd-client -L -d /dev/nbd0 >> "$FULL" 2>&1
+	nbd-client -d /dev/nbd0 >> "$FULL" 2>&1
 
 	if ! _wait_for_nbd_disconnect; then
 		echo "Disconnect didn't happen?"
-- 
2.44.0

[PATCH blktests v2 2/2] nbd/002: repeat partition existence check for ioctl interface

[Shin'ichiro Kawasaki]

When nbd-client is set up with the ioctl interface, it takes some time
for the nbd driver and the block layer to complete the partition read.
The test script calls stat command for the /dev/nbd0p1 device to check
the partition exists as expected. However, this stat command is often
called before the partition read completion, then causes the test case
failure.

To avoid the test case failure, repeat the partition check a few times
with one second wait.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 tests/nbd/002 | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tests/nbd/002 b/tests/nbd/002
index 968c9fa..8e4e062 100755
--- a/tests/nbd/002
+++ b/tests/nbd/002
@@ -21,6 +21,8 @@ requires() {
 }
 
 test() {
+	local pass i
+
 	echo "Running ${TEST_NAME}"
 	_start_nbd_server
 	{
@@ -64,7 +66,15 @@ test() {
 
 	udevadm settle
 
-	if ! stat /dev/nbd0p1 >> "$FULL" 2>&1; then
+	pass=false
+	for ((i = 0; i < 3; i++)); do
+		if stat /dev/nbd0p1 >> "$FULL" 2>&1; then
+			pass=true
+			break
+		fi
+		sleep 1
+	done
+	if [[ $pass != true ]]; then
 		echo "Didn't have partition on ioctl path"
 		nbd-client -nonetlink -d /dev/nbd0 >> "$FULL" 2>&1
 		_stop_nbd_server
-- 
2.44.0

Re: [PATCH blktests v2 0/2] fix nbd/002

[Yi Zhang]

Cannot reproduce the failure within 10000 cycles test now, thanks.

Tested-by: Yi Zhang <yi.zhang@redhat.com>




On Wed, Apr 17, 2024 at 6:42 PM Shin'ichiro Kawasaki
<shinichiro.kawasaki@wdc.com> wrote:
>
> Recently, CKI project found blktests nbd/002 failure. The test case sets up the
> connection to the nbd device, then checks partition existence in the device to
> confirm that the kernel reads the partition table in the device. Usually, this
> partition read by kernel is completed before the test script checks the
> partition existence. However, the partition read often completes after the
> partition existence check, then the test case fails. I think the test script
> checks the partition existence too early, and this should be fixed in the test
> script.
>
> During this investigation, I noticed that the test case nbd/002 handles the
> ioctl interface and the netlink interface opposite. The first patch fixes this
> wrong interface handling. The second patch addresses the too early partition
> existence check issue.
>
> Link to v1 patch: https://lore.kernel.org/linux-block/20240319085015.3901051-1-shinichiro.kawasaki@wdc.com/
>
> Changes from v1:
> * Added another patch to fix ioctl/netlink interface handling mistake
> * Avoid the nbd/002 failure by repeating the partition existence check
>
> Shin'ichiro Kawasaki (2):
>   nbd/002: fix wrong -L/-nonetlink option usage
>   nbd/002: repeat partition existence check for ioctl interface
>
>  tests/nbd/002 | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
>
> --
> 2.44.0
>


--
Best Regards,
  Yi Zhang

Re: [PATCH blktests v2 0/2] fix nbd/002

[Shinichiro Kawasaki]

On Apr 21, 2024 / 20:29, Yi Zhang wrote:
> Cannot reproduce the failure within 10000 cycles test now, thanks.
> 
> Tested-by: Yi Zhang <yi.zhang@redhat.com>

Thanks for the test. I've applied the patches.