From 74a8bf565853149810b709ad590db47abff90ae6 Mon Sep 17 00:00:00 2001 From: Frank Lahm Date: Thu, 21 Apr 2011 13:45:44 +0200 Subject: [PATCH] Reliable db locking --- etc/cnid_dbd/cmd_dbd.c | 118 +++++---------------------------- etc/cnid_dbd/cmd_dbd.h | 2 - etc/cnid_dbd/cmd_dbd_scanvol.c | 5 -- etc/cnid_dbd/dbif.c | 87 ++++++++++++++++++++++++ etc/cnid_dbd/dbif.h | 9 +++ etc/cnid_dbd/main.c | 98 +++++++-------------------- 6 files changed, 136 insertions(+), 183 deletions(-) diff --git a/etc/cnid_dbd/cmd_dbd.c b/etc/cnid_dbd/cmd_dbd.c index c34008fd..7fd2ce02 100644 --- a/etc/cnid_dbd/cmd_dbd.c +++ b/etc/cnid_dbd/cmd_dbd.c @@ -72,12 +72,13 @@ #include #include #include +#include + #include "cmd_dbd.h" #include "dbd.h" #include "dbif.h" #include "db_param.h" -#define LOCKFILENAME "lock" #define DBOPTIONS (DB_CREATE | DB_INIT_LOCK | DB_INIT_LOG | DB_INIT_MPOOL | DB_INIT_TXN) int nocniddb = 0; /* Dont open CNID database, only scan filesystem */ @@ -164,102 +165,6 @@ static void set_signal(void) } } - -/*! - * Get lock on db lock file - * - * @args cmd (r) lock command: - * LOCK_FREE: close lockfd - * LOCK_UNLOCK: unlock lockm keep lockfd open - * LOCK_EXCL: F_WRLCK on lockfd - * LOCK_SHRD: F_RDLCK on lockfd - * @args dbpath (r) path to lockfile, only used on first call, - * later the stored fd is used - * @returns LOCK_FREE/LOCK_UNLOCK return 0 on success, -1 on error - * LOCK_EXCL/LOCK_SHRD return 1 if lock was acquired, - * 0 if file is already locked, -1 on error - */ -int get_lock(lockcmd_t cmd, const char *dbpath) -{ - static int lockfd = -1; - char lockpath[PATH_MAX]; - struct flock lock; - struct stat st; - - - switch (cmd) { - case LOCK_FREE: - if (lockfd == -1) - return -1; - close(lockfd); - lockfd = -1; - return 0; - - case LOCK_UNLOCK: - if (lockfd == -1) - return -1; - - lock.l_start = 0; - lock.l_whence = SEEK_SET; - lock.l_len = 0; - lock.l_type = F_UNLCK; - fcntl(lockfd, F_SETLK, &lock); - return 0; - - case LOCK_EXCL: - case LOCK_SHRD: - if (lockfd == -1) { - if ( (strlen(dbpath) + strlen(LOCKFILENAME+1)) > (PATH_MAX - 1) ) { - dbd_log( LOGSTD, ".AppleDB pathname too long"); - return -1; - } - strncpy(lockpath, dbpath, PATH_MAX - 1); - strcat(lockpath, "/"); - strcat(lockpath, LOCKFILENAME); - - if ((lockfd = open(lockpath, O_RDWR | O_CREAT, 0644)) < 0) { - dbd_log( LOGSTD, "Error opening lockfile: %s", strerror(errno)); - return -1; - } - - if ((stat(dbpath, &st)) != 0) { - dbd_log( LOGSTD, "Error statting lockfile: %s", strerror(errno)); - return -1; - } - - if ((chown(lockpath, st.st_uid, st.st_gid)) != 0) { - dbd_log( LOGSTD, "Error inheriting lockfile permissions: %s", - strerror(errno)); - return -1; - } - } - - lock.l_start = 0; - lock.l_whence = SEEK_SET; - lock.l_len = 0; - if (cmd == LOCK_EXCL) - lock.l_type = F_WRLCK; - else - lock.l_type = F_RDLCK; - - if (fcntl(lockfd, F_SETLK, &lock) < 0) { - if (errno == EACCES || errno == EAGAIN) { - dbd_log(LOGDEBUG, "get_lock: couldn't lock"); - return 0; - } else { - dbd_log( LOGSTD, "Error getting fcntl F_WRLCK on lockfile: %s", - strerror(errno)); - return -1; - } - } - default: - return -1; - } /* switch(cmd) */ - - dbd_log(LOGDEBUG, "get_lock: got lock"); - return 1; -} - static void usage (void) { printf("Usage: dbd [-e|-t|-v|-x] -d [-i] | -s [-c|-n]| -r [-c|-f] | -u \n" @@ -444,17 +349,16 @@ int main(int argc, char **argv) /* Get db lock */ if ((db_locked = get_lock(LOCK_EXCL, dbpath)) == -1) goto exit_failure; - if (!db_locked) { + if (db_locked != LOCK_EXCL) { /* Couldn't get exclusive lock, try shared lock if -e wasn't requested */ if (exclusive) { dbd_log(LOGSTD, "Database is in use and exlusive was requested"); goto exit_failure; } - if ((db_locked = get_lock(LOCK_SHRD, dbpath)) != 1) + if ((db_locked = get_lock(LOCK_SHRD, NULL)) != LOCK_SHRD) goto exit_failure; } - /* Prepare upgrade ? */ if (prep_upgrade) { if (dbif_prep_upgrade(dbpath)) @@ -471,7 +375,7 @@ int main(int argc, char **argv) dbd_log( LOGDEBUG, "Removing old database of volume: '%s'", volpath); system(cmd); dbd_log( LOGDEBUG, "Removed old database."); - if ((db_locked = get_lock(LOCK_EXCL, NULL)) == -1) + if ((db_locked = get_lock(LOCK_EXCL, dbpath)) == -1) goto exit_failure; } @@ -484,12 +388,12 @@ int main(int argc, char **argv) if (dbif_env_open(dbd, &db_param, - exclusive ? (DBOPTIONS | DB_RECOVER) : DBOPTIONS) < 0) { + (db_locked == LOCK_EXCL) ? (DBOPTIONS | DB_RECOVER) : DBOPTIONS) < 0) { dbd_log( LOGSTD, "error opening database!"); goto exit_failure; } - if (exclusive) + if (db_locked == LOCK_EXCL) dbd_log( LOGDEBUG, "Finished recovery."); if (dbif_open(dbd, NULL, rebuildindexes) < 0) { @@ -503,6 +407,14 @@ int main(int argc, char **argv) } } + /* Downgrade db lock if not running exclusive */ + if (!exclusive && (db_locked == LOCK_EXCL)) { + if (get_lock(LOCK_UNLOCK, NULL) != 0) + goto exit_failure; + if (get_lock(LOCK_SHRD, NULL) != LOCK_SHRD) + goto exit_failure; + } + /* Now execute given command scan|rebuild|dump */ if (dump && ! nocniddb) { if (dbif_dump(dbd, dumpindexes) < 0) { diff --git a/etc/cnid_dbd/cmd_dbd.h b/etc/cnid_dbd/cmd_dbd.h index 654c372b..b2ee649f 100644 --- a/etc/cnid_dbd/cmd_dbd.h +++ b/etc/cnid_dbd/cmd_dbd.h @@ -9,7 +9,6 @@ enum logtype {LOGSTD, LOGDEBUG}; typedef unsigned int dbd_flags_t; -typedef enum {LOCK_FREE, LOCK_UNLOCK, LOCK_EXCL, LOCK_SHRD} lockcmd_t; #define DBD_FLAGS_SCAN (1 << 0) #define DBD_FLAGS_FORCE (1 << 1) @@ -33,7 +32,6 @@ extern char cwdbuf[MAXPATHLEN+1]; extern void dbd_log(enum logtype lt, char *fmt, ...); extern int cmd_dbd_scanvol(DBD *dbd, struct volinfo *volinfo, dbd_flags_t flags); -extern int get_lock(lockcmd_t cmd, const char *dbpath); /* Functions for querying the database which couldn't be reused from the existing diff --git a/etc/cnid_dbd/cmd_dbd_scanvol.c b/etc/cnid_dbd/cmd_dbd_scanvol.c index 1f6c853a..abe9688a 100644 --- a/etc/cnid_dbd/cmd_dbd_scanvol.c +++ b/etc/cnid_dbd/cmd_dbd_scanvol.c @@ -855,11 +855,6 @@ static int dbd_readdir(int volroot, cnid_t did) struct dirent *ep; static struct stat st; /* Save some stack space */ - /* keep trying to get the lock */ - if (!db_locked) - if ((db_locked = get_lock(1, NULL)) == -1) - return -1; - /* Check again for .AppleDouble folder, check_adfile also checks/creates it */ if ((addir_ok = check_addir(volroot)) != 0) if ( ! (dbd_flags & DBD_FLAGS_SCAN)) diff --git a/etc/cnid_dbd/dbif.c b/etc/cnid_dbd/dbif.c index be9b568f..c79189c5 100644 --- a/etc/cnid_dbd/dbif.c +++ b/etc/cnid_dbd/dbif.c @@ -20,7 +20,9 @@ #include #include #include +#include #include + #include "db_param.h" #include "dbif.h" #include "pack.h" @@ -172,6 +174,91 @@ exit: return ret; } +/*! + * Get lock on db lock file + * + * @args cmd (r) lock command: + * LOCK_FREE: close lockfd + * LOCK_UNLOCK: unlock lockm keep lockfd open + * LOCK_EXCL: F_WRLCK on lockfd + * LOCK_SHRD: F_RDLCK on lockfd + * @args dbpath (r) path to lockfile, only used on first call, + * later the stored fd is used + * @returns LOCK_FREE/LOCK_UNLOCK return 0 on success, -1 on error + * LOCK_EXCL/LOCK_SHRD return LOCK_EXCL or LOCK_SHRD respectively on + * success, 0 if the lock couldn't be acquired, -1 on other errors + */ +int get_lock(int cmd, const char *dbpath) +{ + static int lockfd = -1; + int ret; + char lockpath[PATH_MAX]; + struct stat st; + + switch (cmd) { + case LOCK_FREE: + if (lockfd == -1) + return -1; + close(lockfd); + lockfd = -1; + return 0; + + case LOCK_UNLOCK: + if (lockfd == -1) + return -1; + return unlock(lockfd, 0, SEEK_SET, 0); + + case LOCK_EXCL: + case LOCK_SHRD: + if (lockfd == -1) { + if ( (strlen(dbpath) + strlen(LOCKFILENAME+1)) > (PATH_MAX - 1) ) { + LOG(log_error, logtype_cnid, ".AppleDB pathname too long"); + return -1; + } + strncpy(lockpath, dbpath, PATH_MAX - 1); + strcat(lockpath, "/"); + strcat(lockpath, LOCKFILENAME); + + if ((lockfd = open(lockpath, O_RDWR | O_CREAT, 0644)) < 0) { + LOG(log_error, logtype_cnid, "Error opening lockfile: %s", strerror(errno)); + return -1; + } + + if ((stat(dbpath, &st)) != 0) { + LOG(log_error, logtype_cnid, "Error statting lockfile: %s", strerror(errno)); + return -1; + } + + if ((chown(lockpath, st.st_uid, st.st_gid)) != 0) { + LOG(log_error, logtype_cnid, "Error inheriting lockfile permissions: %s", + strerror(errno)); + return -1; + } + } + + if (cmd == LOCK_EXCL) + ret = write_lock(lockfd, 0, SEEK_SET, 0); + else + ret = read_lock(lockfd, 0, SEEK_SET, 0); + + if (ret != 0) { + if (cmd == LOCK_SHRD) + LOG(log_error, logtype_cnid, "Volume CNID db is locked, try again..."); + return 0; + } + + LOG(log_debug, logtype_cnid, "get_lock: got %s lock", + cmd == LOCK_EXCL ? "LOCK_EXCL" : "LOCK_SHRD"); + return cmd; + + default: + return -1; + } /* switch(cmd) */ + + /* deadc0de, never get here */ + return -1; +} + /* --------------- */ int dbif_stamp(DBD *dbd, void *buffer, int size) { diff --git a/etc/cnid_dbd/dbif.h b/etc/cnid_dbd/dbif.h index c5f4f52c..1a4a0b8e 100644 --- a/etc/cnid_dbd/dbif.h +++ b/etc/cnid_dbd/dbif.h @@ -64,6 +64,13 @@ #define DBIF_IDX_DEVINO 1 #define DBIF_IDX_DIDNAME 2 +/* get_lock cmd and return value */ +#define LOCKFILENAME "lock" +#define LOCK_FREE 0 +#define LOCK_UNLOCK 1 +#define LOCK_EXCL 2 +#define LOCK_SHRD 3 + /* Structures */ typedef struct { char *name; @@ -85,6 +92,8 @@ typedef struct { } DBD; /* Functions */ +extern int get_lock(int cmd, const char *dbpath); + extern DBD *dbif_init(const char *envhome, const char *dbname); extern int dbif_env_open(DBD *dbd, struct db_param *dbp, uint32_t dbenv_oflags); extern int dbif_open(DBD *dbd, struct db_param *dbp, int reindex); diff --git a/etc/cnid_dbd/main.c b/etc/cnid_dbd/main.c index 5cafb2ef..bf71b72d 100644 --- a/etc/cnid_dbd/main.c +++ b/etc/cnid_dbd/main.c @@ -34,14 +34,13 @@ #include #include #include +#include #include "db_param.h" #include "dbif.h" #include "dbd.h" #include "comm.h" -#define LOCKFILENAME "lock" - /* Note: DB_INIT_LOCK is here so we can run the db_* utilities while netatalk is running. It's a likey performance hit, but it might we worth it. @@ -72,57 +71,6 @@ static void block_sigs_onoff(int block) return; } -/*! - * Get lock on db lock file - * - * @args cmd (r) !=0: lock, 0: unlock - * @returns 1 if lock was acquired, 0 if file is already locked, -1 on error - */ -static int get_lock(int cmd) -{ - static int lockfd = -1; - struct flock lock; - - if (cmd == 0) { - if (lockfd == -1) - return -1; - - lock.l_start = 0; - lock.l_whence = SEEK_SET; - lock.l_len = 0; - lock.l_type = F_UNLCK; - fcntl(lockfd, F_SETLK, &lock); - close(lockfd); - lockfd = -1; - return 0; - } - - if (lockfd == -1) { - if ((lockfd = open(LOCKFILENAME, O_RDWR | O_CREAT, 0644)) < 0) { - LOG(log_error, logtype_cnid, "get_lock: error opening lockfile: %s", strerror(errno)); - return -1; - } - } - - lock.l_start = 0; - lock.l_whence = SEEK_SET; - lock.l_len = 0; - lock.l_type = F_WRLCK; - - if (fcntl(lockfd, F_SETLK, &lock) < 0) { - if (errno == EACCES || errno == EAGAIN) { - LOG(log_debug, logtype_cnid, "get_lock: couldn't lock"); - return 0; - } else { - LOG(log_error, logtype_cnid, "get_lock: fcntl F_WRLCK lockfile: %s", strerror(errno)); - return -1; - } - } - - LOG(log_debug, logtype_cnid, "get_lock: got lock"); - return 1; -} - /* The dbd_XXX and comm_XXX functions all obey the same protocol for return values: @@ -169,20 +117,6 @@ static int loop(struct db_param *dbp) dbp->flush_interval, timebuf); while (1) { - /* - * If we haven't got the lock yet, get it now. - * Prevents a race with dbd: - * 1. no cnid_dbd running - * 2. dbd -r starts, gets the lock - * 3. cnid_dbd starts, doesn't get lock, doesn't run recovery, all is fine - * 4. dbd from (2) finishes, drops lock - * 5. anothet dbd but this time with -re is started which - * - succeeds getting the lock - * - runs recovery => this kills (3) - */ - if (!db_locked) - if ((db_locked = get_lock(1)) == -1) - return -1; timeout = min(time_next_flush, time_last_rqst +dbp->idle_timeout); if (timeout > now) timeout -= now; @@ -364,11 +298,18 @@ int main(int argc, char *argv[]) switch_to_user(dir); - /* Before we do anything else, check if there is an instance of cnid_dbd - running already and silently exit if yes. */ - if ((db_locked = get_lock(1)) == -1) { + /* Get db lock */ + if ((db_locked = get_lock(LOCK_EXCL, dir)) == -1) { + LOG(log_error, logtype_cnid, "main: fatal db lock error"); exit(1); } + if (db_locked != LOCK_EXCL) { + /* Couldn't get exclusive lock, try shared lock */ + if ((db_locked = get_lock(LOCK_SHRD, NULL)) != LOCK_SHRD) { + LOG(log_error, logtype_cnid, "main: fatal db lock error"); + exit(1); + } + } LOG(log_info, logtype_cnid, "Startup, DB dir %s", dir); @@ -387,7 +328,7 @@ int main(int argc, char *argv[]) /* Only recover if we got the lock */ if (dbif_env_open(dbd, dbp, - db_locked ? DBOPTIONS | DB_RECOVER : DBOPTIONS) < 0) + (db_locked == LOCK_EXCL) ? DBOPTIONS | DB_RECOVER : DBOPTIONS) < 0) exit(2); /* FIXME: same exit code as failure for dbif_open() */ LOG(log_debug, logtype_cnid, "Finished initializing BerkeleyDB environment"); @@ -397,6 +338,19 @@ int main(int argc, char *argv[]) } LOG(log_debug, logtype_cnid, "Finished opening BerkeleyDB databases"); + /* Downgrade db lock */ + if (db_locked == LOCK_EXCL) { + if (get_lock(LOCK_UNLOCK, NULL) != 0) { + dbif_close(dbd); + exit(2); + } + if (get_lock(LOCK_SHRD, NULL) != LOCK_SHRD) { + dbif_close(dbd); + exit(2); + } + } + + if (dbd_stamp(dbd) < 0) { dbif_close(dbd); exit(5); @@ -417,8 +371,6 @@ int main(int argc, char *argv[]) if (dbif_prep_upgrade(dir) < 0) err++; - (void)get_lock(0); - if (err) exit(4); else if (exit_sig) -- 2.39.2