Re: [PATCH blktests v3] nbd/003:add mount and clear_sock test for nbd

[Omar Sandoval <osandov@osandov.com>]

On Sat, Nov 30, 2019 at 08:05:40PM +0800, Sun Ke wrote:
> Add the test case to check nbd device. This test case catches regressions
> fixed by commit 92b5c8f0063e4 "nbd: replace kill_bdev() with
> __invalidate_device() again".
> 
> Establish the nbd connection. Run two processes. The first one do mount
> and umount, and the other one do clear_sock ioctl.
> 
> Signed-off-by: Sun Ke <sunke32@huawei.com>
> ---
> v2 -> v3
> 1. Now only build nbd0 connection, not 15 connections.
> 2. Add the run_cnt to 225.
> 3. Modify some variable names.

mount_clear_sock still seems overly complicated, so I simplified it.
Additionally, we can loop from the C program instead of looping in the
shell script and execing a bunch of times. And, it seems like we can use
the existing _start_nbd_server/_stop_nbd_server helpers now that we're
only using one connection. I made those changes here:
https://github.com/osandov/blktests/commit/ab2472ee1c54f7fc69011815e15515846ae40eea
Would you mind taking a look and ensuring that it still does what you're
trying to do? I seem to have triggered some other sort of deadlock in
NBD with the optimized test.

> ---
>  src/Makefile           |  3 ++-
>  src/mount_clear_sock.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/nbd/003          | 55 ++++++++++++++++++++++++++++++++++++++++
>  tests/nbd/003.out      |  1 +
>  4 files changed, 126 insertions(+), 1 deletion(-)
>  create mode 100644 src/mount_clear_sock.c
>  create mode 100644 tests/nbd/003
>  create mode 100644 tests/nbd/003.out
> 
> diff --git a/src/Makefile b/src/Makefile
> index 917d6f4..acd7327 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -10,7 +10,8 @@ C_TARGETS := \
>  	sg/syzkaller1 \
>  	nbdsetsize \
>  	loop_change_fd \
> -	zbdioctl
> +	zbdioctl \
> +	mount_clear_sock
>  
>  CXX_TARGETS := \
>  	discontiguous-io
> diff --git a/src/mount_clear_sock.c b/src/mount_clear_sock.c
> new file mode 100644
> index 0000000..c76cbfe
> --- /dev/null
> +++ b/src/mount_clear_sock.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-3.0+
> +// Copyright (C) 2019 Sun Ke
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +
> +#include <linux/nbd.h>
> +#include <assert.h>
> +#include <sys/wait.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +#include <sys/mount.h>
> +#include <linux/fs.h>
> +
> +void clear_sock(int fd)
> +{
> +	int err;
> +
> +	err = ioctl(fd, NBD_CLEAR_SOCK, 0);
> +	if (err) {
> +		perror("ioctl");
> +	}
> +}
> +
> +void mount_nbd(char *dev, char *mp, char *fs)
> +{
> +	mount(dev, mp, fs, MS_NOSUID | MS_SYNCHRONOUS, 0);
> +	umount(mp);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	if (argc != 4) {
> +		fprintf(stderr, "usage: $0 MOUNTPOINT DEV FS");
> +		return EXIT_FAILURE;
> +	}
> +
> +	char *mp = argv[1];
> +	char *dev = argv[2];
> +	char *fs = argv[3];
> +
> +	static int fd = -1;
> +
> +	fd = open(dev, O_RDWR);
> +	if (fd < 0 ) {
> +		perror("open");
> +	}
> +
> +	if (fork() == 0) {
> +		mount_nbd(dev, mp, fs);
> +		exit(0);
> +	}
> +	if (fork() == 0) {
> +		clear_sock(fd);
> +		exit(0);
> +	}
> +	while(wait(NULL) > 0)
> +		continue;
> +
> +	close(fd);
> +
> +	return 0;
> +}
> diff --git a/tests/nbd/003 b/tests/nbd/003
> new file mode 100644
> index 0000000..738928e
> --- /dev/null
> +++ b/tests/nbd/003
> @@ -0,0 +1,55 @@
> +#!/bin/bash
> +
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2019 Sun Ke
> +#
> +# Test nbd device resizing. Regression test for patch
> +#
> +# 2b5c8f0063e4 ("nbd: replace kill_bdev() with __invalidate_device() again")
> +
> +
> +DESCRIPTION="resize a connected nbd device"
> +QUICK=1
> +
> +fs_type=ext4
> +disk_capacity=256M
> +run_cnt=225
> +
> +requires() {
> +	_have_nbd && _have_src_program mount_clear_sock
> +}
> +
> +_start_nbd_mount_server() {
> +
> +	fallocate -l $1 "${TMPDIR}/disk"
> +
> +	if [[ "$2"x = "ext4"x ]]; then
> +		mkfs.ext4 "${TMPDIR}/disk" >> "$FULL" 2>&1
> +	else
> +		mkdosfs "${TMPDIR}/disk" >> "$FULL" 2>&1
> +	fi
> +	nbd-server 8000 "${TMPDIR}/disk" >> "$FULL" 2>&1
> +
> +	mkdir -p "${TMPDIR}/mount_point"
> +}
> +
> +_stop_nbd_mount_server() {
> +	pkill -9 -f 8000
> +	rm -f "${TMPDIR}/disk"
> +	rm -rf "${TMPDIR}/mount_point"
> +}
> +
> +test() {
> +	echo "Running ${TEST_NAME}"
> +
> +	_start_nbd_mount_server  $disk_capacity $fs_type
> +	nbd-client localhost 8000 /dev/nbd0 >> "$FULL" 2>&1
> +
> +	for ((i = 0; i < $run_cnt; i++))
> +	do
> +		src/mount_clear_sock  "${TMPDIR}/mount_point" /dev/nbd0 $fs_type
> +	done
> +
> +	nbd-client -d /dev/nbd0
> +	_stop_nbd_mount_server
> +}
> diff --git a/tests/nbd/003.out b/tests/nbd/003.out
> new file mode 100644
> index 0000000..aa340db
> --- /dev/null
> +++ b/tests/nbd/003.out
> @@ -0,0 +1 @@
> +Running nbd/003
> -- 
> 2.13.6
>