From 7acb0553ce4d14a235e82635a5c5f1345dd3c7bf Mon Sep 17 00:00:00 2001 From: Gurucharan Shetty Date: Thu, 29 May 2014 14:47:30 -0700 Subject: [PATCH] lockfile: Modify tests for Windows. As of now, when a process tries to reacquire a lockfile, we return EEXIST on Windows and print a different error message. This means that the unit tests need to look for different error messages too. Linux uses EDEADLK for the same. EDEADLK feels like a good error description for Windows too and this also lets us not change the tests too much. So use it. Some of the tests in test-lockfile.c uses fork to test a child's ability to acquire lock. We do not fork in Windows. We also do not support symlinks on Windows. So, comment out those tests. Signed-off-by: Gurucharan Shetty Acked-by: Ben Pfaff --- lib/lockfile.c | 5 +++-- tests/lockfile.at | 13 ++++++----- tests/test-lockfile.c | 51 ++++++++++++++++++++++++------------------- 3 files changed, 40 insertions(+), 29 deletions(-) diff --git a/lib/lockfile.c b/lib/lockfile.c index d157bc616..1ef625128 100644 --- a/lib/lockfile.c +++ b/lib/lockfile.c @@ -276,8 +276,9 @@ lockfile_try_lock(const char *name, pid_t *pidp, struct lockfile **lockfilep) retval = LockFileEx(lock_handle, LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, 0, 1, 0, &overl); if (!retval) { - VLOG_WARN("Failed to lock file : %s", ovs_lasterror_to_string()); - return EEXIST; + VLOG_DBG("Failed to lock file : %s", ovs_lasterror_to_string()); + *pidp = getpid(); + return EDEADLK; } lockfile = xmalloc(sizeof *lockfile); diff --git a/tests/lockfile.at b/tests/lockfile.at index 2e2a74b47..61920c3b0 100644 --- a/tests/lockfile.at +++ b/tests/lockfile.at @@ -1,7 +1,10 @@ AT_BANNER([lockfile unit tests]) +# CHECK_LOCKFILE([test-name], [number-children], [error-message] +# [skip-test-windows]) m4_define([CHECK_LOCKFILE], [AT_SETUP([m4_translit([$1], [_], [ ])]) + m4_if([$4], [yes], [AT_SKIP_IF([test "$IS_WIN32" = "yes"])]) AT_KEYWORDS([lockfile]) AT_CHECK([ovstest test-lockfile $1], [0], [$1: success (m4_if( [$2], [1], [$2 child], [$2 children])) @@ -25,15 +28,15 @@ lockfile|WARN|.file.~lock~: cannot lock file because this process has already lo CHECK_LOCKFILE([lock_blocks_other_process], [1], [lockfile|WARN|.file.~lock~: child does not inherit lock lockfile|WARN|.file.~lock~: cannot lock file because it is already locked by pid -]) +], [yes]) CHECK_LOCKFILE([lock_twice_blocks_other_process], [1], [lockfile|WARN|.file.~lock~: cannot lock file because this process has already locked it lockfile|WARN|.file.~lock~: child does not inherit lock lockfile|WARN|.file.~lock~: cannot lock file because it is already locked by pid -]) +], [yes]) -CHECK_LOCKFILE([lock_and_unlock_allows_other_process], [1]) +CHECK_LOCKFILE([lock_and_unlock_allows_other_process], [1], [], [yes]) CHECK_LOCKFILE([lock_multiple], [0], [lockfile|WARN|.a.~lock~: cannot lock file because this process has already locked it @@ -44,8 +47,8 @@ CHECK_LOCKFILE([lock_symlink], [0], lockfile|WARN|.b.~lock~: cannot lock file because this process has already locked it lockfile|WARN|.b.~lock~: cannot lock file because this process has already locked it lockfile|WARN|.a.~lock~: cannot lock file because this process has already locked it -]) +], [yes]) CHECK_LOCKFILE([lock_symlink_to_dir], [0], [lockfile|WARN|dir/.b.~lock~: cannot lock file because this process has already locked it -]) +], [yes]) diff --git a/tests/test-lockfile.c b/tests/test-lockfile.c index 0bf3fe6dd..c50543cbe 100644 --- a/tests/test-lockfile.c +++ b/tests/test-lockfile.c @@ -92,6 +92,7 @@ run_lock_blocks_same_process_twice(void) lockfile_unlock(lockfile); } +#ifndef _WIN32 static enum { PARENT, CHILD } do_fork(void) { @@ -153,27 +154,6 @@ run_lock_and_unlock_allows_other_process(void) } } -static void -run_lock_multiple(void) -{ - struct lockfile *a, *b, *c, *dummy; - - CHECK(lockfile_lock("a", &a), 0); - CHECK(lockfile_lock("b", &b), 0); - CHECK(lockfile_lock("c", &c), 0); - - lockfile_unlock(a); - CHECK(lockfile_lock("a", &a), 0); - CHECK(lockfile_lock("a", &dummy), EDEADLK); - lockfile_unlock(a); - - lockfile_unlock(b); - CHECK(lockfile_lock("a", &a), 0); - - lockfile_unlock(c); - lockfile_unlock(a); -} - /* Checks that locking a dangling symlink works OK. (It used to hang.) */ static void run_lock_symlink(void) @@ -236,6 +216,29 @@ run_lock_symlink_to_dir(void) lockfile_unlock(a); } +#endif /* _WIN32 */ + +static void +run_lock_multiple(void) +{ + struct lockfile *a, *b, *c, *dummy; + + CHECK(lockfile_lock("a", &a), 0); + CHECK(lockfile_lock("b", &b), 0); + CHECK(lockfile_lock("c", &c), 0); + + lockfile_unlock(a); + CHECK(lockfile_lock("a", &a), 0); + CHECK(lockfile_lock("a", &dummy), EDEADLK); + lockfile_unlock(a); + + lockfile_unlock(b); + CHECK(lockfile_lock("a", &a), 0); + + lockfile_unlock(c); + lockfile_unlock(a); +} + static const struct test tests[] = { #define TEST(NAME) { #NAME, run_##NAME } @@ -243,12 +246,14 @@ static const struct test tests[] = { TEST(lock_and_unlock_twice), TEST(lock_blocks_same_process), TEST(lock_blocks_same_process_twice), +#ifndef _WIN32 TEST(lock_blocks_other_process), TEST(lock_twice_blocks_other_process), TEST(lock_and_unlock_allows_other_process), - TEST(lock_multiple), TEST(lock_symlink), TEST(lock_symlink_to_dir), +#endif /* _WIN32 */ + TEST(lock_multiple), TEST(help), { NULL, NULL } #undef TEST @@ -289,6 +294,7 @@ test_lockfile_main(int argc, char *argv[]) (tests[i].function)(); n_children = 0; +#ifndef _WIN32 while (wait(&status) > 0) { if (WIFEXITED(status) && WEXITSTATUS(status) == 11) { n_children++; @@ -300,6 +306,7 @@ test_lockfile_main(int argc, char *argv[]) if (errno != ECHILD) { ovs_fatal(errno, "wait"); } +#endif /* _WIN32 */ printf("%s: success (%d child%s)\n", tests[i].name, n_children, n_children != 1 ? "ren" : ""); -- 2.39.2