From b87e14495d5b07e1b247218a72329f10ecb3da7f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 31 Aug 2011 13:58:15 +0930 Subject: [PATCH 1/1] tdb2: add TDB_RDONLY flag, allow setting/unsetting it. You can only unset it if the TDB was originally opened O_RDWR. Also, cleaned up error handling in tdb_allrecord_lock() so we only get one log message on a r/o database. --- ccan/tdb2/io.c | 10 +-- ccan/tdb2/lock.c | 7 +- ccan/tdb2/open.c | 13 ++- ccan/tdb2/private.h | 3 - ccan/tdb2/tdb.c | 39 ++++++++ ccan/tdb2/tdb2.h | 1 + ccan/tdb2/test/run-92-get-set-readonly.c | 109 +++++++++++++++++++++++ ccan/tdb2/transaction.c | 4 +- 8 files changed, 170 insertions(+), 16 deletions(-) create mode 100644 ccan/tdb2/test/run-92-get-set-readonly.c diff --git a/ccan/tdb2/io.c b/ccan/tdb2/io.c index d422d963..24c70084 100644 --- a/ccan/tdb2/io.c +++ b/ccan/tdb2/io.c @@ -200,7 +200,7 @@ enum TDB_ERROR zero_out(struct tdb_context *tdb, tdb_off_t off, tdb_len_t len) void *p = tdb->methods->direct(tdb, off, len, true); enum TDB_ERROR ecode = TDB_SUCCESS; - assert(!tdb->read_only); + assert(!(tdb->flags & TDB_RDONLY)); if (TDB_PTR_IS_ERR(p)) { return TDB_PTR_ERR(p); } @@ -248,7 +248,7 @@ static enum TDB_ERROR tdb_write(struct tdb_context *tdb, tdb_off_t off, { enum TDB_ERROR ecode; - if (tdb->read_only) { + if (tdb->flags & TDB_RDONLY) { return tdb_logerr(tdb, TDB_ERR_RDONLY, TDB_LOG_USE_ERROR, "Write to read-only database"); } @@ -337,7 +337,7 @@ enum TDB_ERROR tdb_read_convert(struct tdb_context *tdb, tdb_off_t off, enum TDB_ERROR tdb_write_off(struct tdb_context *tdb, tdb_off_t off, tdb_off_t val) { - if (tdb->read_only) { + if (tdb->flags & TDB_RDONLY) { return tdb_logerr(tdb, TDB_ERR_RDONLY, TDB_LOG_USE_ERROR, "Write to read-only database"); } @@ -416,7 +416,7 @@ static enum TDB_ERROR tdb_expand_file(struct tdb_context *tdb, char buf[8192]; enum TDB_ERROR ecode; - if (tdb->read_only) { + if (tdb->flags & TDB_RDONLY) { return tdb_logerr(tdb, TDB_ERR_RDONLY, TDB_LOG_USE_ERROR, "Expand on read-only database"); } @@ -488,7 +488,7 @@ void *tdb_access_write(struct tdb_context *tdb, { void *ret = NULL; - if (tdb->read_only) { + if (tdb->flags & TDB_RDONLY) { tdb_logerr(tdb, TDB_ERR_RDONLY, TDB_LOG_USE_ERROR, "Write to read-only database"); return TDB_ERR_PTR(TDB_ERR_RDONLY); diff --git a/ccan/tdb2/lock.c b/ccan/tdb2/lock.c index 76b8bc31..957adf83 100644 --- a/ccan/tdb2/lock.c +++ b/ccan/tdb2/lock.c @@ -194,7 +194,7 @@ static enum TDB_ERROR tdb_brlock(struct tdb_context *tdb, return TDB_SUCCESS; } - if (rw_type == F_WRLCK && tdb->read_only) { + if (rw_type == F_WRLCK && (tdb->flags & TDB_RDONLY)) { return tdb_logerr(tdb, TDB_ERR_RDONLY, TDB_LOG_USE_ERROR, "Write lock attempted on read-only database"); } @@ -508,8 +508,9 @@ static enum TDB_ERROR tdb_lock_gradual(struct tdb_context *tdb, } /* First we try non-blocking. */ - if (tdb_brlock(tdb, ltype, off, len, nb_flags) == TDB_SUCCESS) { - return TDB_SUCCESS; + ecode = tdb_brlock(tdb, ltype, off, len, nb_flags); + if (ecode != TDB_ERR_LOCK) { + return ecode; } /* Try locking first half, then second. */ diff --git a/ccan/tdb2/open.c b/ccan/tdb2/open.c index 83a83e0b..68f707b9 100644 --- a/ccan/tdb2/open.c +++ b/ccan/tdb2/open.c @@ -402,7 +402,8 @@ struct tdb_context *tdb_open(const char *name, int tdb_flags, } if (tdb_flags & ~(TDB_INTERNAL | TDB_NOLOCK | TDB_NOMMAP | TDB_CONVERT - | TDB_NOSYNC | TDB_SEQNUM | TDB_ALLOW_NESTING)) { + | TDB_NOSYNC | TDB_SEQNUM | TDB_ALLOW_NESTING + | TDB_RDONLY)) { ecode = tdb_logerr(tdb, TDB_ERR_EINVAL, TDB_LOG_USE_ERROR, "tdb_open: unknown flags %u", tdb_flags); goto fail; @@ -416,10 +417,16 @@ struct tdb_context *tdb_open(const char *name, int tdb_flags, } if ((open_flags & O_ACCMODE) == O_RDONLY) { - tdb->read_only = true; openlock = F_RDLCK; + tdb->flags |= TDB_RDONLY; } else { - tdb->read_only = false; + if (tdb_flags & TDB_RDONLY) { + ecode = tdb_logerr(tdb, TDB_ERR_EINVAL, + TDB_LOG_USE_ERROR, + "tdb_open: can't use TDB_RDONLY" + " without O_RDONLY"); + goto fail; + } openlock = F_WRLCK; } diff --git a/ccan/tdb2/private.h b/ccan/tdb2/private.h index 39044425..efc3a98d 100644 --- a/ccan/tdb2/private.h +++ b/ccan/tdb2/private.h @@ -331,9 +331,6 @@ struct tdb_context { /* Are we accessing directly? (debugging check). */ int direct_access; - /* Operating read-only? (Opened O_RDONLY, or in traverse_read) */ - bool read_only; - /* Open flags passed to tdb_open. */ int open_flags; diff --git a/ccan/tdb2/tdb.c b/ccan/tdb2/tdb.c index 7484695d..bb181732 100644 --- a/ccan/tdb2/tdb.c +++ b/ccan/tdb2/tdb.c @@ -324,6 +324,29 @@ unsigned int tdb_get_flags(struct tdb_context *tdb) return tdb->flags; } +static bool readonly_changable(struct tdb_context *tdb, const char *caller) +{ + if (tdb->transaction) { + tdb->last_error = tdb_logerr(tdb, TDB_ERR_EINVAL, + TDB_LOG_USE_ERROR, + "%s: can't change" + " TDB_RDONLY inside transaction", + caller); + return false; + } + + if (tdb->file->allrecord_lock.count != 0 + || tdb->file->num_lockrecs != 0) { + tdb->last_error = tdb_logerr(tdb, TDB_ERR_EINVAL, + TDB_LOG_USE_ERROR, + "%s: can't change" + " TDB_RDONLY holding locks", + caller); + return false; + } + return true; +} + void tdb_add_flag(struct tdb_context *tdb, unsigned flag) { if (tdb->flags & TDB_INTERNAL) { @@ -349,6 +372,10 @@ void tdb_add_flag(struct tdb_context *tdb, unsigned flag) case TDB_ALLOW_NESTING: tdb->flags |= TDB_ALLOW_NESTING; break; + case TDB_RDONLY: + if (readonly_changable(tdb, "tdb_add_flag")) + tdb->flags |= TDB_RDONLY; + break; default: tdb->last_error = tdb_logerr(tdb, TDB_ERR_EINVAL, TDB_LOG_USE_ERROR, @@ -382,6 +409,18 @@ void tdb_remove_flag(struct tdb_context *tdb, unsigned flag) case TDB_ALLOW_NESTING: tdb->flags &= ~TDB_ALLOW_NESTING; break; + case TDB_RDONLY: + if ((tdb->open_flags & O_ACCMODE) == O_RDONLY) { + tdb->last_error = tdb_logerr(tdb, TDB_ERR_EINVAL, + TDB_LOG_USE_ERROR, + "tdb_remove_flag: can't" + " remove TDB_RDONLY on tdb" + " opened with O_RDONLY"); + break; + } + if (readonly_changable(tdb, "tdb_remove_flag")) + tdb->flags &= ~TDB_RDONLY; + break; default: tdb->last_error = tdb_logerr(tdb, TDB_ERR_EINVAL, TDB_LOG_USE_ERROR, diff --git a/ccan/tdb2/tdb2.h b/ccan/tdb2/tdb2.h index 5643ef4f..051d20a4 100644 --- a/ccan/tdb2/tdb2.h +++ b/ccan/tdb2/tdb2.h @@ -85,6 +85,7 @@ struct tdb_context *tdb_open(const char *name, int tdb_flags, #define TDB_NOSYNC 64 /* don't use synchronous transactions */ #define TDB_SEQNUM 128 /* maintain a sequence number */ #define TDB_ALLOW_NESTING 256 /* fake nested transactions */ +#define TDB_RDONLY 512 /* implied by O_RDONLY */ /** * tdb_close - close and free a tdb. diff --git a/ccan/tdb2/test/run-92-get-set-readonly.c b/ccan/tdb2/test/run-92-get-set-readonly.c new file mode 100644 index 00000000..09a6010c --- /dev/null +++ b/ccan/tdb2/test/run-92-get-set-readonly.c @@ -0,0 +1,109 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "logging.h" + +int main(int argc, char *argv[]) +{ + unsigned int i; + struct tdb_context *tdb; + struct tdb_data key = tdb_mkdata("key", 3); + struct tdb_data data = tdb_mkdata("data", 4); + int flags[] = { TDB_DEFAULT, TDB_NOMMAP, + TDB_CONVERT, TDB_NOMMAP|TDB_CONVERT }; + + plan_tests(sizeof(flags) / sizeof(flags[0]) * 48); + + for (i = 0; i < sizeof(flags) / sizeof(flags[0]); i++) { + /* RW -> R0 */ + tdb = tdb_open("run-92-get-set-readonly.tdb", flags[i], + O_RDWR|O_CREAT|O_TRUNC, 0600, &tap_log_attr); + ok1(tdb); + ok1(!(tdb_get_flags(tdb) & TDB_RDONLY)); + + ok1(tdb_store(tdb, key, data, TDB_INSERT) == TDB_SUCCESS); + + tdb_add_flag(tdb, TDB_RDONLY); + ok1(tdb_get_flags(tdb) & TDB_RDONLY); + + /* Can't store, append, delete. */ + ok1(tdb_store(tdb, key, data, TDB_MODIFY) == TDB_ERR_RDONLY); + ok1(tap_log_messages == 1); + ok1(tdb_append(tdb, key, data) == TDB_ERR_RDONLY); + ok1(tap_log_messages == 2); + ok1(tdb_delete(tdb, key) == TDB_ERR_RDONLY); + ok1(tap_log_messages == 3); + + /* Can't start a transaction, or any write lock. */ + ok1(tdb_transaction_start(tdb) == TDB_ERR_RDONLY); + ok1(tap_log_messages == 4); + ok1(tdb_chainlock(tdb, key) == TDB_ERR_RDONLY); + ok1(tap_log_messages == 5); + ok1(tdb_lockall(tdb) == TDB_ERR_RDONLY); + ok1(tap_log_messages == 6); + ok1(tdb_wipe_all(tdb) == TDB_ERR_RDONLY); + ok1(tap_log_messages == 7); + + /* Back to RW. */ + tdb_remove_flag(tdb, TDB_RDONLY); + ok1(!(tdb_get_flags(tdb) & TDB_RDONLY)); + + ok1(tdb_store(tdb, key, data, TDB_MODIFY) == TDB_SUCCESS); + ok1(tdb_append(tdb, key, data) == TDB_SUCCESS); + ok1(tdb_delete(tdb, key) == TDB_SUCCESS); + + ok1(tdb_transaction_start(tdb) == TDB_SUCCESS); + ok1(tdb_store(tdb, key, data, TDB_INSERT) == TDB_SUCCESS); + ok1(tdb_transaction_commit(tdb) == TDB_SUCCESS); + + ok1(tdb_chainlock(tdb, key) == TDB_SUCCESS); + tdb_chainunlock(tdb, key); + ok1(tdb_lockall(tdb) == TDB_SUCCESS); + tdb_unlockall(tdb); + ok1(tdb_wipe_all(tdb) == TDB_SUCCESS); + ok1(tap_log_messages == 7); + + tdb_close(tdb); + + /* R0 -> RW */ + tdb = tdb_open("run-92-get-set-readonly.tdb", flags[i], + O_RDONLY, 0600, &tap_log_attr); + ok1(tdb); + ok1(tdb_get_flags(tdb) & TDB_RDONLY); + + /* Can't store, append, delete. */ + ok1(tdb_store(tdb, key, data, TDB_INSERT) == TDB_ERR_RDONLY); + ok1(tap_log_messages == 8); + ok1(tdb_append(tdb, key, data) == TDB_ERR_RDONLY); + ok1(tap_log_messages == 9); + ok1(tdb_delete(tdb, key) == TDB_ERR_RDONLY); + ok1(tap_log_messages == 10); + + /* Can't start a transaction, or any write lock. */ + ok1(tdb_transaction_start(tdb) == TDB_ERR_RDONLY); + ok1(tap_log_messages == 11); + ok1(tdb_chainlock(tdb, key) == TDB_ERR_RDONLY); + ok1(tap_log_messages == 12); + ok1(tdb_lockall(tdb) == TDB_ERR_RDONLY); + ok1(tap_log_messages == 13); + ok1(tdb_wipe_all(tdb) == TDB_ERR_RDONLY); + ok1(tap_log_messages == 14); + + /* Can't remove TDB_RDONLY since we opened with O_RDONLY */ + tdb_remove_flag(tdb, TDB_RDONLY); + ok1(tap_log_messages == 15); + ok1(tdb_get_flags(tdb) & TDB_RDONLY); + tdb_close(tdb); + + ok1(tap_log_messages == 15); + tap_log_messages = 0; + } + return exit_status(); +} diff --git a/ccan/tdb2/transaction.c b/ccan/tdb2/transaction.c index 2f588ef3..a29acf96 100644 --- a/ccan/tdb2/transaction.c +++ b/ccan/tdb2/transaction.c @@ -532,7 +532,7 @@ enum TDB_ERROR tdb_transaction_start(struct tdb_context *tdb) " internal tdb"); } - if (tdb->read_only) { + if (tdb->flags & TDB_RDONLY) { return tdb->last_error = tdb_logerr(tdb, TDB_ERR_RDONLY, TDB_LOG_USE_ERROR, "tdb_transaction_start:" @@ -1196,7 +1196,7 @@ enum TDB_ERROR tdb_transaction_recover(struct tdb_context *tdb) return TDB_SUCCESS; } - if (tdb->read_only) { + if (tdb->flags & TDB_RDONLY) { return tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR, "tdb_transaction_recover:" " attempt to recover read only database"); -- 2.39.2