| Message ID | 20230911163846.27197-29-mwilck@suse.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
| Series | multipath-tools: user-friendly names rework | expand |
On Mon, Sep 11, 2023 at 06:38:37PM +0200, mwilck@suse.com wrote: > From: Martin Wilck <mwilck@suse.com> > > If some test fails with a lock held, cmocka doesn't deal well with > pthread_cleanup_pop(). Such tests can cause deadlock with the locking > primitives in the alias code, because locks don't get properly unlocked. Just > mock the lock/unlock functions and generate an error if they weren't paired at > the end of the test. > Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com> > Signed-off-by: Martin Wilck <mwilck@suse.com> > --- > tests/Makefile | 1 + > tests/alias.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 47 insertions(+) > > diff --git a/tests/Makefile b/tests/Makefile > index c777d07..7dac8a8 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -52,6 +52,7 @@ blacklist-test_LIBDEPS := -ludev > vpd-test_OBJDEPS := $(multipathdir)/discovery.o > vpd-test_LIBDEPS := -ludev -lpthread -ldl > alias-test_TESTDEPS := test-log.o > +alias-test_OBJDEPS := $(mpathutildir)/util.o > alias-test_LIBDEPS := -lpthread -ldl > valid-test_OBJDEPS := $(multipathdir)/valid.o $(multipathdir)/discovery.o > valid-test_LIBDEPS := -lmount -ludev -lpthread -ldl > diff --git a/tests/alias.c b/tests/alias.c > index 872b1fc..962c158 100644 > --- a/tests/alias.c > +++ b/tests/alias.c > @@ -89,6 +89,47 @@ int __wrap_dm_get_uuid(const char *name, char *uuid, int uuid_len) > return ret; > } > > +static int lock_errors; > +static int bindings_locked; > +static int timestamp_locked; > +int __wrap_pthread_mutex_lock(pthread_mutex_t *mutex) > +{ > + if (mutex == &bindings_mutex) { > + if (bindings_locked) { > + fprintf(stderr, "%s: bindings_mutex LOCKED\n", __func__); > + lock_errors++; > + } > + bindings_locked = 1; > + } else if (mutex == ×tamp_mutex) { > + if (timestamp_locked) { > + fprintf(stderr, "%s: timestamp_mutex LOCKED\n", __func__); > + lock_errors++; > + } > + timestamp_locked = 1; > + } else > + fprintf(stderr, "%s called for unknown mutex %p\n", __func__, mutex); > + return 0; > +} > + > +int __wrap_pthread_mutex_unlock(pthread_mutex_t *mutex) > +{ > + if (mutex == &bindings_mutex) { > + if (!bindings_locked) { > + fprintf(stderr, "%s: bindings_mutex UNLOCKED\n", __func__); > + lock_errors++; > + } > + bindings_locked = 0; > + } else if (mutex == ×tamp_mutex) { > + if (!timestamp_locked) { > + fprintf(stderr, "%s: timestamp_mutex UNLOCKED\n", __func__); > + lock_errors++; > + } > + timestamp_locked = 0; > + } else > + fprintf(stderr, "%s called for unknown mutex %p\n", __func__, mutex); > + return 0; > +} > + > #define TEST_FDNO 1234 > #define TEST_FPTR ((FILE *) 0xaffe) > > @@ -1718,6 +1759,10 @@ static void gufa_old_nomatch_nowwidmatch(void **state) { > free(alias); > } > > +static void gufa_check_locking(void **state) { > + assert_int_equal(lock_errors, 0); > +} > + > static int test_get_user_friendly_alias() > { > const struct CMUnitTest tests[] = { > @@ -1743,6 +1788,7 @@ static int test_get_user_friendly_alias() > cmocka_unit_test_teardown(gufa_old_nomatch_wwidmatch, teardown_bindings), > cmocka_unit_test_teardown(gufa_old_nomatch_wwidmatch_used, teardown_bindings), > cmocka_unit_test_teardown(gufa_old_nomatch_nowwidmatch, teardown_bindings), > + cmocka_unit_test(gufa_check_locking), > }; > > return cmocka_run_group_tests(tests, NULL, NULL); > -- > 2.42.0 -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
diff --git a/tests/Makefile b/tests/Makefile index c777d07..7dac8a8 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -52,6 +52,7 @@ blacklist-test_LIBDEPS := -ludev vpd-test_OBJDEPS := $(multipathdir)/discovery.o vpd-test_LIBDEPS := -ludev -lpthread -ldl alias-test_TESTDEPS := test-log.o +alias-test_OBJDEPS := $(mpathutildir)/util.o alias-test_LIBDEPS := -lpthread -ldl valid-test_OBJDEPS := $(multipathdir)/valid.o $(multipathdir)/discovery.o valid-test_LIBDEPS := -lmount -ludev -lpthread -ldl diff --git a/tests/alias.c b/tests/alias.c index 872b1fc..962c158 100644 --- a/tests/alias.c +++ b/tests/alias.c @@ -89,6 +89,47 @@ int __wrap_dm_get_uuid(const char *name, char *uuid, int uuid_len) return ret; } +static int lock_errors; +static int bindings_locked; +static int timestamp_locked; +int __wrap_pthread_mutex_lock(pthread_mutex_t *mutex) +{ + if (mutex == &bindings_mutex) { + if (bindings_locked) { + fprintf(stderr, "%s: bindings_mutex LOCKED\n", __func__); + lock_errors++; + } + bindings_locked = 1; + } else if (mutex == ×tamp_mutex) { + if (timestamp_locked) { + fprintf(stderr, "%s: timestamp_mutex LOCKED\n", __func__); + lock_errors++; + } + timestamp_locked = 1; + } else + fprintf(stderr, "%s called for unknown mutex %p\n", __func__, mutex); + return 0; +} + +int __wrap_pthread_mutex_unlock(pthread_mutex_t *mutex) +{ + if (mutex == &bindings_mutex) { + if (!bindings_locked) { + fprintf(stderr, "%s: bindings_mutex UNLOCKED\n", __func__); + lock_errors++; + } + bindings_locked = 0; + } else if (mutex == ×tamp_mutex) { + if (!timestamp_locked) { + fprintf(stderr, "%s: timestamp_mutex UNLOCKED\n", __func__); + lock_errors++; + } + timestamp_locked = 0; + } else + fprintf(stderr, "%s called for unknown mutex %p\n", __func__, mutex); + return 0; +} + #define TEST_FDNO 1234 #define TEST_FPTR ((FILE *) 0xaffe) @@ -1718,6 +1759,10 @@ static void gufa_old_nomatch_nowwidmatch(void **state) { free(alias); } +static void gufa_check_locking(void **state) { + assert_int_equal(lock_errors, 0); +} + static int test_get_user_friendly_alias() { const struct CMUnitTest tests[] = { @@ -1743,6 +1788,7 @@ static int test_get_user_friendly_alias() cmocka_unit_test_teardown(gufa_old_nomatch_wwidmatch, teardown_bindings), cmocka_unit_test_teardown(gufa_old_nomatch_wwidmatch_used, teardown_bindings), cmocka_unit_test_teardown(gufa_old_nomatch_nowwidmatch, teardown_bindings), + cmocka_unit_test(gufa_check_locking), }; return cmocka_run_group_tests(tests, NULL, NULL);