On Fri, Jul 12, 2024 at 07:14:57PM +0200, Martin Wilck wrote: > 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. > Reviewed-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx> > Signed-off-by: Martin Wilck <mwilck@xxxxxxxx> > --- > tests/Makefile | 8 --- > tests/README.md | 29 ++++++-- > tests/directio.c | 182 ++++++++++++++++++++++++++--------------------- > 3 files changed, 122 insertions(+), 97 deletions(-) > > diff --git a/tests/Makefile b/tests/Makefile > index 55fbf0f..02580e7 100644 > --- a/tests/Makefile > +++ b/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 > diff --git a/tests/README.md b/tests/README.md > index fd36fc1..0cae057 100644 > --- a/tests/README.md > +++ b/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/), > diff --git a/tests/directio.c b/tests/directio.c > index d5f84f1..763929e 100644 > --- a/tests/directio.c > +++ b/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 (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]; > } > - 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]; > -#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; > } > -- > 2.45.2
