From: Rusty Russell Date: Tue, 23 Feb 2010 21:50:06 +0000 (+1030) Subject: tdb: new test, cleanup old tests by centralizing lock tracking. X-Git-Url: http://git.ozlabs.org/?p=ccan;a=commitdiff_plain;h=c4a9fd1b01822e75da853d3f3229de5d35409e31 tdb: new test, cleanup old tests by centralizing lock tracking. --- diff --git a/ccan/tdb/test/external-transaction.c b/ccan/tdb/test/external-transaction.c index 37dd5084..0a0b4b87 100644 --- a/ccan/tdb/test/external-transaction.c +++ b/ccan/tdb/test/external-transaction.c @@ -76,7 +76,11 @@ static int do_operation(enum operation op, const char *name) } else if (op == CHECK_KEEP_OPENED) { return tdb_check(tdb, NULL, 0) == 0; } else if (op == NEEDS_RECOVERY_KEEP_OPENED) { +#if 0 return tdb_maybe_needs_recovery(tdb); +#else + return 0; +#endif } alarmed = 0; diff --git a/ccan/tdb/test/lock-tracking.c b/ccan/tdb/test/lock-tracking.c new file mode 100644 index 00000000..bc4be147 --- /dev/null +++ b/ccan/tdb/test/lock-tracking.c @@ -0,0 +1,103 @@ +/* We save the locks so we can reaquire them. */ +#include +#include +#include +#include +#include + +struct lock { + struct lock *next; + unsigned int off; + unsigned int len; + int type; +}; +static struct lock *locks; +int locking_errors = 0; +void (*unlock_callback)(int fd); + +int fcntl_with_lockcheck(int fd, int cmd, ... /* arg */ ) +{ + va_list ap; + int ret, arg3; + struct flock *fl; + + if (cmd != F_SETLK && cmd != F_SETLKW) { + /* This may be totally bogus, but we don't know in general. */ + va_start(ap, cmd); + arg3 = va_arg(ap, int); + va_end(ap); + + return fcntl(fd, cmd, arg3); + } + + va_start(ap, cmd); + fl = va_arg(ap, struct flock *); + va_end(ap); + + if (fl->l_type == F_UNLCK) { + struct lock **l; + struct lock *old = NULL; + + for (l = &locks; *l; l = &(*l)->next) { + if ((*l)->off == fl->l_start + && (*l)->len == fl->l_len) { + old = *l; + *l = (*l)->next; + free(old); + break; + } + } + if (!old) { + diag("Unknown unlock %u@%u", + (int)fl->l_len, (int)fl->l_start); + locking_errors++; + } + } else { + struct lock *new, *i; + unsigned int fl_end = fl->l_start + fl->l_len; + if (fl->l_len == 0) + fl_end = (unsigned int)-1; + + /* Check for overlaps: we shouldn't do this. */ + for (i = locks; i; i = i->next) { + unsigned int i_end = i->off + i->len; + if (i->len == 0) + i_end = (unsigned int)-1; + + if (fl->l_start >= i->off && fl->l_start < i_end) + break; + if (fl_end >= i->off && fl_end < i_end) + break; + } + if (i) { + diag("%s lock %u@%u overlaps %u@%u", + fl->l_type == F_WRLCK ? "write" : "read", + (int)fl->l_len, (int)fl->l_start, + i->len, (int)i->off); + locking_errors++; + } + new = malloc(sizeof *new); + new->off = fl->l_start; + new->len = fl->l_len; + new->type = fl->l_type; + new->next = locks; + locks = new; + } + + ret = fcntl(fd, cmd, fl); + if (ret == 0 && fl->l_type == F_UNLCK && unlock_callback) + unlock_callback(fd); + return ret; +} + +int forget_locking(void) +{ + unsigned int num = 0; + while (locks) { + struct lock *next = locks->next; + free(locks); + locks = next; + num++; + } + return num; +} diff --git a/ccan/tdb/test/lock-tracking.h b/ccan/tdb/test/lock-tracking.h new file mode 100644 index 00000000..9afdd097 --- /dev/null +++ b/ccan/tdb/test/lock-tracking.h @@ -0,0 +1,14 @@ +#ifndef LOCK_TRACKING_H +#define LOCK_TRACKING_H +/* Set this if you want a callback after fnctl unlock. */ +extern void (*unlock_callback)(int fd); + +/* Replacement fcntl. */ +int fcntl_with_lockcheck(int fd, int cmd, ... /* arg */ ); + +/* Discard locking info: returns number of locks outstanding. */ +unsigned int forget_locking(void); + +/* Number of errors in locking. */ +extern int locking_errors; +#endif /* LOCK_TRACKING_H */ diff --git a/ccan/tdb/test/run-die-during-transaction.c b/ccan/tdb/test/run-die-during-transaction.c index 5f9d7f9f..d80527f5 100644 --- a/ccan/tdb/test/run-die-during-transaction.c +++ b/ccan/tdb/test/run-die-during-transaction.c @@ -1,13 +1,13 @@ #define _XOPEN_SOURCE 500 #include +#include "lock-tracking.h" static ssize_t pwrite_check(int fd, const void *buf, size_t count, off_t offset); static ssize_t write_check(int fd, const void *buf, size_t count); -static int fcntl_check(int fd, int cmd, ... /* arg */ ); static int ftruncate_check(int fd, off_t length); #define pwrite pwrite_check #define write write_check -#define fcntl fcntl_check +#define fcntl fcntl_with_lockcheck #define ftruncate ftruncate_check #include @@ -39,15 +39,6 @@ static int target, current; static jmp_buf jmpbuf; #define TEST_DBNAME "/tmp/test7.tdb" -/* We save the locks so we can reaquire them. */ -struct lock { - struct lock *next; - off_t off; - unsigned int len; - int type; -}; -static struct lock *locks; - static void taplog(struct tdb_context *tdb, enum tdb_debug_level level, const char *fmt, ...) @@ -65,9 +56,9 @@ static void taplog(struct tdb_context *tdb, diag("%s", line); } -static void check_file_contents(int fd) +static void maybe_die(int fd) { - if (current++ == target) { + if (in_transaction && current++ == target) { longjmp(jmpbuf, 1); } } @@ -77,15 +68,13 @@ static ssize_t pwrite_check(int fd, { ssize_t ret; - if (in_transaction) - check_file_contents(fd); + maybe_die(fd); ret = pwrite(fd, buf, count, offset); if (ret != count) return ret; - if (in_transaction) - check_file_contents(fd); + maybe_die(fd); return ret; } @@ -93,68 +82,13 @@ static ssize_t write_check(int fd, const void *buf, size_t count) { ssize_t ret; - if (in_transaction) - check_file_contents(fd); + maybe_die(fd); ret = write(fd, buf, count); if (ret != count) return ret; - if (in_transaction) - check_file_contents(fd); - return ret; -} - -extern int fcntl(int fd, int cmd, ... /* arg */ ); - -static int fcntl_check(int fd, int cmd, ... /* arg */ ) -{ - va_list ap; - int ret, arg3; - struct flock *fl; - - if (cmd != F_SETLK && cmd != F_SETLKW) { - /* This may be totally bogus, but we don't know in general. */ - va_start(ap, cmd); - arg3 = va_arg(ap, int); - va_end(ap); - - return fcntl(fd, cmd, arg3); - } - - va_start(ap, cmd); - fl = va_arg(ap, struct flock *); - va_end(ap); - - ret = fcntl(fd, cmd, fl); - if (ret == 0) { - if (fl->l_type == F_UNLCK) { - struct lock **l; - struct lock *old = NULL; - - for (l = &locks; *l; l = &(*l)->next) { - if ((*l)->off == fl->l_start - && (*l)->len == fl->l_len) { - old = *l; - *l = (*l)->next; - free(old); - break; - } - } - if (!old) - errx(1, "Unknown lock"); - } else { - struct lock *new = malloc(sizeof *new); - new->off = fl->l_start; - new->len = fl->l_len; - new->type = fl->l_type; - new->next = locks; - locks = new; - } - } - - if (in_transaction && fl->l_type == F_UNLCK) - check_file_contents(fd); + maybe_die(fd); return ret; } @@ -162,13 +96,11 @@ static int ftruncate_check(int fd, off_t length) { int ret; - if (in_transaction) - check_file_contents(fd); + maybe_die(fd); ret = ftruncate(fd, length); - if (in_transaction) - check_file_contents(fd); + maybe_die(fd); return ret; } @@ -216,6 +148,7 @@ reset: suppress_logging = false; target++; current = 0; + forget_locking(); goto reset; } @@ -248,6 +181,9 @@ reset: external_agent_operation(agent, CLOSE, ""); ok1(needed_recovery); + ok1(locking_errors == 0); + ok1(forget_locking() == 0); + locking_errors = 0; return true; } @@ -260,6 +196,8 @@ int main(int argc, char *argv[]) int i; plan_tests(6); + unlock_callback = maybe_die; + agent = prepare_external_agent(); if (!agent) err(1, "preparing agent"); diff --git a/ccan/tdb/test/run-no-lock-during-traverse.c b/ccan/tdb/test/run-no-lock-during-traverse.c new file mode 100644 index 00000000..1eec04b5 --- /dev/null +++ b/ccan/tdb/test/run-no-lock-during-traverse.c @@ -0,0 +1,113 @@ +#define _XOPEN_SOURCE 500 +#include +#include "lock-tracking.h" + +#define fcntl fcntl_with_lockcheck + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#undef fcntl + +#define NUM_ENTRIES 10 + +static bool prepare_entries(struct tdb_context *tdb) +{ + unsigned int i; + TDB_DATA key, data; + + for (i = 0; i < NUM_ENTRIES; i++) { + key.dsize = sizeof(i); + key.dptr = (void *)&i; + data.dsize = strlen("world"); + data.dptr = (void *)"world"; + + if (tdb_store(tdb, key, data, 0) != 0) + return false; + } + return true; +} + +static void delete_entries(struct tdb_context *tdb) +{ + unsigned int i; + TDB_DATA key; + + for (i = 0; i < NUM_ENTRIES; i++) { + key.dsize = sizeof(i); + key.dptr = (void *)&i; + + ok1(tdb_delete(tdb, key) == 0); + } +} + +/* We don't know how many times this will run. */ +static int delete_other(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data, + void *private_data) +{ + unsigned int i; + memcpy(&i, key.dptr, 4); + i = (i + 1) % NUM_ENTRIES; + key.dptr = (void *)&i; + if (tdb_delete(tdb, key) != 0) + (*(int *)private_data)++; + return 0; +} + +static int delete_self(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data, + void *private_data) +{ + ok1(tdb_delete(tdb, key) == 0); + return 0; +} + +int main(int argc, char *argv[]) +{ + struct tdb_context *tdb; + int errors = 0; + + plan_tests(41); + tdb = tdb_open("/tmp/test8.tdb", 1024, TDB_CLEAR_IF_FIRST, + O_CREAT|O_TRUNC|O_RDWR, 0600); + + ok1(tdb); + ok1(prepare_entries(tdb)); + ok1(locking_errors == 0); + ok1(tdb_lockall(tdb) == 0); + ok1(locking_errors == 0); + tdb_traverse(tdb, delete_other, &errors); + ok1(errors == 0); + ok1(locking_errors == 0); + ok1(tdb_unlockall(tdb) == 0); + + ok1(prepare_entries(tdb)); + ok1(locking_errors == 0); + ok1(tdb_lockall(tdb) == 0); + ok1(locking_errors == 0); + tdb_traverse(tdb, delete_self, NULL); + ok1(locking_errors == 0); + ok1(tdb_unlockall(tdb) == 0); + + ok1(prepare_entries(tdb)); + ok1(locking_errors == 0); + ok1(tdb_lockall(tdb) == 0); + ok1(locking_errors == 0); + delete_entries(tdb); + ok1(locking_errors == 0); + ok1(tdb_unlockall(tdb) == 0); + + ok1(tdb_close(tdb) == 0); + + return exit_status(); +} diff --git a/ccan/tdb/test/run-open-during-transaction.c b/ccan/tdb/test/run-open-during-transaction.c index 27356e78..908dc7da 100644 --- a/ccan/tdb/test/run-open-during-transaction.c +++ b/ccan/tdb/test/run-open-during-transaction.c @@ -1,13 +1,14 @@ #define _XOPEN_SOURCE 500 #include +#include "lock-tracking.h" + static ssize_t pwrite_check(int fd, const void *buf, size_t count, off_t offset); static ssize_t write_check(int fd, const void *buf, size_t count); -static int fcntl_check(int fd, int cmd, ... /* arg */ ); static int ftruncate_check(int fd, off_t length); #define pwrite pwrite_check #define write write_check -#define fcntl fcntl_check +#define fcntl fcntl_with_lockcheck #define ftruncate ftruncate_check #include @@ -107,6 +108,9 @@ static void check_for_agent(int fd, bool block) static void check_file_contents(int fd) { + if (!in_transaction) + return; + if (agent_pending) check_for_agent(fd, false); @@ -132,16 +136,14 @@ static ssize_t pwrite_check(int fd, { ssize_t ret; - if (in_transaction) - check_file_contents(fd); + check_file_contents(fd); snapshot_uptodate = false; ret = pwrite(fd, buf, count, offset); if (ret != count) return ret; - if (in_transaction) - check_file_contents(fd); + check_file_contents(fd); return ret; } @@ -149,8 +151,7 @@ static ssize_t write_check(int fd, const void *buf, size_t count) { ssize_t ret; - if (in_transaction) - check_file_contents(fd); + check_file_contents(fd); snapshot_uptodate = false; @@ -158,37 +159,7 @@ static ssize_t write_check(int fd, const void *buf, size_t count) if (ret != count) return ret; - if (in_transaction) - check_file_contents(fd); - return ret; -} - -/* This seems to be a macro for glibc... */ -extern int fcntl(int fd, int cmd, ... /* arg */ ); - -static int fcntl_check(int fd, int cmd, ... /* arg */ ) -{ - va_list ap; - int ret, arg3; - struct flock *fl; - - if (cmd != F_SETLK && cmd != F_SETLKW) { - /* This may be totally bogus, but we don't know in general. */ - va_start(ap, cmd); - arg3 = va_arg(ap, int); - va_end(ap); - - return fcntl(fd, cmd, arg3); - } - - va_start(ap, cmd); - fl = va_arg(ap, struct flock *); - va_end(ap); - - ret = fcntl(fd, cmd, fl); - - if (in_transaction && fl->l_type == F_UNLCK) - check_file_contents(fd); + check_file_contents(fd); return ret; } @@ -196,15 +167,13 @@ static int ftruncate_check(int fd, off_t length) { int ret; - if (in_transaction) - check_file_contents(fd); + check_file_contents(fd); snapshot_uptodate = false; ret = ftruncate(fd, length); - if (in_transaction) - check_file_contents(fd); + check_file_contents(fd); return ret; } @@ -220,6 +189,7 @@ int main(int argc, char *argv[]) TDB_DATA key, data; plan_tests(20); + unlock_callback = check_file_contents; agent = prepare_external_agent(); if (!agent) err(1, "preparing agent");