multipath-tools tests: fix directio test with real device

Allow setting DIO_TEST_DEV during runtime, by reading the environment
variable. The test was fragile despite the delay, because the real
io_getevents() call isn't guaranteed to return the number of events
requested. Fix that. Moreover, allow reading DIO_TEST_DELAY (in us)
from the environment. With the io_getevents fix, for me the test
succeeded even with 0 us delay.

Change the README accordingly.

Signed-off-by: Martin Wilck <[email protected]>

tests/Makefile

@@ -21,12 +21,6 @@ valgrind:	$(TESTS:%=%.vgr)
 # test-specific compiler flags
 # XYZ-test_FLAGS: Additional compiler flags for this test
 
-ifneq ($(wildcard directio_test_dev),)
-DIO_TEST_DEV = $(shell sed -n -e 's/^[[:space:]]*DIO_TEST_DEV[[:space:]]*=[[:space:]]*\([^[:space:]\#]\+\).*/\1/p' < directio_test_dev)
-endif
-ifneq ($(DIO_TEST_DEV),)
-directio-test_FLAGS := -DDIO_TEST_DEV=\"$(DIO_TEST_DEV)\"
-endif
 mpathvalid-test_FLAGS := -I$(mpathvaliddir)
 features-test_FLAGS := -I$(multipathdir)/nvme
 
@@ -59,9 +53,7 @@ valid-test_LIBDEPS := -lmount -ludev -lpthread -ldl
 devt-test_LIBDEPS := -ludev
 mpathvalid-test_LIBDEPS := -ludev -lpthread -ldl
 mpathvalid-test_OBJDEPS := $(mpathvaliddir)/mpath_valid.o
-ifneq ($(DIO_TEST_DEV),)
 directio-test_LIBDEPS := -laio
-endif
 strbuf-test_OBJDEPS := $(mpathutildir)/strbuf.o
 sysfs-test_TESTDEPS := test-log.o
 sysfs-test_OBJDEPS := $(multipathdir)/sysfs.o $(mpathutildir)/util.o

tests/README.md

@@ -13,6 +13,17 @@ If valgrind detects a bad memory access or leak, the test will fail. The
 output of the test run, including valgrind output, is stored as
 `<testname>.vgr`.
 
+## Running tests manually
+
+`make test` or `make -C test "$TEST.out"` will only run the test program if
+the output files `$TEST.out` don't exist yet. To re-run the test, delete the
+output file first. In order to run a test outside `make`, set the library
+search path:
+
+    cd tests
+    export LD_LIBRARY_PATH=.:../libmpathutil:../libmpathcmd
+	./dmevents-test  # or whatever other test you want to run
+
 ## Controlling verbosity for unit tests
 
 Some test programs use the environment variable `MPATHTEST_VERBOSITY` to
@@ -37,15 +48,21 @@ This test includes test items that require a access to a block device. The
 device will be opened in read-only mode; you don't need to worry about data
 loss. However, the user needs to specify a device to be used. Set the
 environment variable `DIO_TEST_DEV` to the path of the device.
-Alternatively, create a file `directio_test_dev` under
-the `tests` directory containing a single line that sets this environment
-variable in Bourne Shell syntax, like this:
-
-    DIO_TEST_DEV=/dev/sdc3
-
 After that, run `make directio.out` as root in the `tests` directory to
 perform the test.
 
+With a real test device, the test results may note be 100% reproducible,
+and sporadic test failures may occur under certain circumstances.
+It may be necessary to introduce a certain delay between test
+operations. To do so, set the environment variable `DIO_TEST_DELAY` to a
+positive integer that determines the delay (in microseconds) after each
+`io_submit()` operation. The default delay is 10 microseconds.
+
+*Note:* `DIO_TEST_DEV` doesn't have to be set during compilation of
+`directio-test`. This used to be the case in previous versions of
+multipath-tools. Previously, it was possible to set `DIO_TEST_DEV` in a file
+`tests/directio_test_dev`. This is not supported any more.
+
 ## Adding tests
 
 The unit tests are based on the [cmocka test framework](https://cmocka.org/),

tests/directio.c

@@ -34,6 +34,8 @@ struct io_event mock_events[AIO_GROUP_SIZE]; /* same as the checker max */
 int ev_off = 0;
 struct timespec zero_timeout = { .tv_sec = 0 };
 struct timespec full_timeout = { .tv_sec = -1 };
+const char *test_dev = NULL;
+unsigned int test_delay = 10000;
 
 #ifdef __GLIBC__
 #define ioctl_request_t unsigned long
@@ -45,12 +47,13 @@ int REAL_IOCTL(int fd, ioctl_request_t request, void *argp);
 
 int WRAP_IOCTL(int fd, ioctl_request_t request, void *argp)
 {
-#ifdef DIO_TEST_DEV
-	mock_type(int);
-	return REAL_IOCTL(fd, request, argp);
-#else
 	int *blocksize = (int *)argp;
 
+	if (test_dev) {
+		mock_type(int);
+		return REAL_IOCTL(fd, request, argp);
+	}
+
 	assert_int_equal(fd, test_fd);
 	/*
 	 * On MUSL libc, the "request" arg is an int (glibc: unsigned long).
@@ -64,88 +67,80 @@ int WRAP_IOCTL(int fd, ioctl_request_t request, void *argp)
 	assert_non_null(blocksize);
 	*blocksize = mock_type(int);
 	return 0;
-#endif
 }
 
 int REAL_FCNTL (int fd, int cmd, long arg);
 
 int WRAP_FCNTL (int fd, int cmd, long arg)
 {
-#ifdef DIO_TEST_DEV
-	return REAL_FCNTL(fd, cmd, arg);
-#else
+	if (test_dev)
+		return REAL_FCNTL(fd, cmd, arg);
 	assert_int_equal(fd, test_fd);
 	assert_int_equal(cmd, F_GETFL);
 	return O_DIRECT;
-#endif
 }
 
 int __real___fxstat(int ver, int fd, struct stat *statbuf);
 
 int __wrap___fxstat(int ver, int fd, struct stat *statbuf)
 {
-#ifdef DIO_TEST_DEV
-	return __real___fxstat(ver, fd, statbuf);
-#else
+	if (test_dev)
+		return __real___fxstat(ver, fd, statbuf);
+
 	assert_int_equal(fd, test_fd);
 	assert_non_null(statbuf);
 	memset(statbuf, 0, sizeof(struct stat));
 	return 0;
-#endif
+
 }
 
 int __real_io_setup(int maxevents, io_context_t *ctxp);
 
 int __wrap_io_setup(int maxevents, io_context_t *ctxp)
 {
-	ioctx_count++;
-#ifdef DIO_TEST_DEV
 	int ret = mock_type(int);
-	assert_int_equal(ret, __real_io_setup(maxevents, ctxp));
+
+	if (test_dev)
+		assert_int_equal(ret, __real_io_setup(maxevents, ctxp));
+	ioctx_count++;
 	return ret;
-#else
-	return mock_type(int);
-#endif
 }
 
 int __real_io_destroy(io_context_t ctx);
 
 int __wrap_io_destroy(io_context_t ctx)
 {
-	ioctx_count--;
-#ifdef DIO_TEST_DEV
 	int ret = mock_type(int);
-	assert_int_equal(ret, __real_io_destroy(ctx));
+
+	ioctx_count--;
+	if (test_dev)
+		assert_int_equal(ret, __real_io_destroy(ctx));
+
 	return ret;
-#else
-	return mock_type(int);
-#endif
 }
 
 int __real_io_submit(io_context_t ctx, long nr, struct iocb *ios[]);
 
 int __wrap_io_submit(io_context_t ctx, long nr, struct iocb *ios[])
 {
-#ifdef DIO_TEST_DEV
-	struct timespec dev_delay = { .tv_nsec = 100000 };
 	int ret = mock_type(int);
-	assert_int_equal(ret, __real_io_submit(ctx, nr, ios));
-	nanosleep(&dev_delay, NULL);
+
+	if (test_dev) {
+		struct timespec dev_delay = { .tv_nsec = test_delay };
+		assert_int_equal(ret, __real_io_submit(ctx, nr, ios));
+		nanosleep(&dev_delay, NULL);
+	}
 	return ret;
-#else
-	return mock_type(int);
-#endif
 }
 
 int __real_io_cancel(io_context_t ctx, struct iocb *iocb, struct io_event *evt);
 
 int __wrap_io_cancel(io_context_t ctx, struct iocb *iocb, struct io_event *evt)
 {
-#ifdef DIO_TEST_DEV
-	return __real_io_cancel(ctx, iocb, evt);
-#else
-	return 0;
-#endif
+	if (test_dev)
+		return __real_io_cancel(ctx, iocb, evt);
+	else
+		return 0;
 }
 
 int REAL_IO_GETEVENTS(io_context_t ctx, long min_nr, long nr,
@@ -155,38 +150,43 @@ int WRAP_IO_GETEVENTS(io_context_t ctx, long min_nr, long nr,
 			struct io_event *events, struct timespec *timeout)
 {
 	int nr_evs;
-#ifndef DIO_TEST_DEV
 	struct timespec *sleep_tmo;
 	int i;
 	struct io_event *evs;
-#endif
 
 	assert_non_null(timeout);
 	nr_evs = mock_type(int);
 	assert_true(nr_evs <= nr);
 	if (!nr_evs)
 		return 0;
-#ifdef DIO_TEST_DEV
-	mock_ptr_type(struct timespec *);
-	mock_ptr_type(struct io_event *);
-	assert_int_equal(nr_evs, REAL_IO_GETEVENTS(ctx, min_nr, nr_evs,
-						   events, timeout));
-#else
-	sleep_tmo = mock_ptr_type(struct timespec *);
-	if (sleep_tmo) {
-		if (sleep_tmo->tv_sec < 0)
-			nanosleep(timeout, NULL);
-		else
-			nanosleep(sleep_tmo, NULL);
-	}
-	if (nr_evs < 0) {
-		errno = -nr_evs;
-		return -1;
+	if (test_dev) {
+		int n = 0;
+		mock_ptr_type(struct timespec *);
+		mock_ptr_type(struct io_event *);
+
+		condlog(2, "min_nr = %ld nr_evs = %d", min_nr, nr_evs);
+		while (n < nr_evs) {
+			min_nr = min_nr <= nr_evs - n ? min_nr : nr_evs - n;
+			n += REAL_IO_GETEVENTS(ctx, min_nr, nr_evs - n,
+					       events + n, timeout);
+		}
+		assert_int_equal(nr_evs, n);
+	} else {
+		sleep_tmo = mock_ptr_type(struct timespec *);
+		if (sleep_tmo) {
+			if (sleep_tmo->tv_sec < 0)
+				nanosleep(timeout, NULL);
+			else
+				nanosleep(sleep_tmo, NULL);
+		}
+		if (nr_evs < 0) {
+			errno = -nr_evs;
+			return -1;
+		}
+		evs = mock_ptr_type(struct io_event *);
+		for (i = 0; i < nr_evs; i++)
+			events[i] = evs[i];
 	}
-	evs = mock_ptr_type(struct io_event *);
-	for (i = 0; i < nr_evs; i++)
-		events[i] = evs[i];
-#endif
 	ev_off -= nr_evs;
 	return nr_evs;
 }
@@ -259,10 +259,9 @@ static void do_libcheck_init(struct checker *c, int blocksize,
 	assert_non_null(ct->req);
 	if (req)
 		*req = ct->req;
-#ifndef DIO_TEST_DEV
-	/* don't check fake blocksize on real devices */
-	assert_int_equal(ct->req->blksize, blocksize);
-#endif
+	if (!test_dev)
+		/* don't check fake blocksize on real devices */
+		assert_int_equal(ct->req->blksize, blocksize);
 }
 
 static int is_checker_running(struct checker *c)
@@ -583,11 +582,11 @@ static void test_async_timeout_cancel_failed(void **state)
 	do_check_state(&c[1], 0, 2, PATH_PENDING);
 	return_io_getevents_none();
 	do_check_state(&c[0], 0, 2, PATH_DOWN);
-#ifndef DIO_TEST_DEV
-	/* can't pick which even gets returned on real devices */
-	return_io_getevents_nr(NULL, 1, &reqs[1], &res[1]);
-	do_check_state(&c[1], 0, 2, PATH_UP);
-#endif
+	if (!test_dev) {
+		/* can't pick which even gets returned on real devices */
+		return_io_getevents_nr(NULL, 1, &reqs[1], &res[1]);
+		do_check_state(&c[1], 0, 2, PATH_UP);
+	}
 	return_io_getevents_none();
 	do_check_state(&c[0], 0, 2, PATH_DOWN);
 	assert_true(is_checker_running(&c[0]));
@@ -663,12 +662,11 @@ static void test_check_state_blksize(void **state)
 	int blksize[] = {4096, 1024, 512};
 	struct async_req *reqs[3];
 	int res[] = {0,1,0};
-#ifdef DIO_TEST_DEV
-	/* can't pick event return state on real devices */
 	int chk_state[] = {PATH_UP, PATH_UP, PATH_UP};
-#else
-	int chk_state[] = {PATH_UP, PATH_DOWN, PATH_UP};
-#endif
+
+	/* can't pick event return state on real devices */
+	if (!test_dev)
+		chk_state[1] = PATH_DOWN;
 
 	assert_true(list_empty(&aio_grp_list));
 	will_return(__wrap_io_setup, 0);
@@ -718,20 +716,38 @@ static void test_check_state_async(void **state)
 
 static int setup(void **state)
 {
-#ifdef DIO_TEST_DEV
-	test_fd = open(DIO_TEST_DEV, O_RDONLY);
-	if (test_fd < 0)
-		fail_msg("cannot open %s: %m", DIO_TEST_DEV);
-#endif
+	char *dl = getenv("DIO_TEST_DELAY");
+	test_dev = getenv("DIO_TEST_DEV");
+
+	if (test_dev) {
+		condlog(2, "%s: opening %s", __func__, test_dev);
+		test_fd = open(test_dev, O_RDONLY);
+		if (dl) {
+			char *e;
+			long int d = strtol(dl, &e, 10);
+
+			if (*e == '\0' && d >= 0 && (d * 1000) < (long)UINT_MAX)
+				test_delay = d * 1000;
+			else {
+				condlog(1, "DIO_TEST_DELAY=%s is invalid", dl);
+				return 1;
+			}
+		}
+		condlog(2, "%s: delay = %u us", __func__, test_delay / 1000);
+	}
+	if (test_fd < 0) {
+		fail_msg("cannot open %s: %m", test_dev);
+		return 1;
+	}
 	return 0;
 }
 
 static int teardown(void **state)
 {
-#ifdef DIO_TEST_DEV
-	assert_true(test_fd > 0);
-	assert_int_equal(close(test_fd), 0);
-#endif
+	if (test_dev) {
+		assert_true(test_fd > 0);
+		assert_int_equal(close(test_fd), 0);
+	}
 	return 0;
 }
 
@@ -762,7 +778,7 @@ int main(void)
 {
 	int ret = 0;
 
-	init_test_verbosity(5);
+	init_test_verbosity(2);
 	ret += test_directio();
 	return ret;
 }