]> arthur.barton.de Git - netatalk.git/commitdiff
Fix dbd vs cnid_dbd locking
authorFrank Lahm <franklahm@googlemail.com>
Wed, 20 Apr 2011 10:51:43 +0000 (12:51 +0200)
committerFrank Lahm <franklahm@googlemail.com>
Wed, 20 Apr 2011 10:51:43 +0000 (12:51 +0200)
etc/cnid_dbd/cmd_dbd.c
etc/cnid_dbd/cmd_dbd.h
etc/cnid_dbd/cmd_dbd_scanvol.c
etc/cnid_dbd/main.c

index d9d5de7977b551d7bd8960ece7db2d1a3bdda534..ccc7eada645aa11416e1beceadc349bd775277f3 100644 (file)
@@ -81,7 +81,8 @@
 #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 */
-volatile sig_atomic_t alarmed;
+volatile sig_atomic_t alarmed;  /* flags for signals */
+int db_locked;                  /* have we got the fcntl lock on lockfile ? */
 
 static DBD *dbd;
 static int verbose;             /* Logging flag */
@@ -163,34 +164,59 @@ static void set_signal(void)
     }        
 }
 
-static int get_lock(const char *dbpath)
+
+/*!
+ * Get lock on db lock file
+ *
+ * @args cmd       (r) !=0: lock, 0: unlock
+ * @args dbpath    (r) path to lockfile, only used on first call,
+ *                     later the stored fd is used
+ * @returns            1 if lock was acquired, 0 if file is already locked, -1 on error
+ */
+int get_lock(int cmd, const char *dbpath)
 {
-    int lockfd;
+    static int lockfd = -1;
     char lockpath[PATH_MAX];
     struct flock lock;
     struct stat st;
 
-    if ( (strlen(dbpath) + strlen(LOCKFILENAME+1)) > (PATH_MAX - 1) ) {
-        dbd_log( LOGSTD, ".AppleDB pathname too long");
-        exit(EXIT_FAILURE);
-    }
-    strncpy(lockpath, dbpath, PATH_MAX - 1);
-    strcat(lockpath, "/");
-    strcat(lockpath, LOCKFILENAME);
+    if (cmd == 0) {
+        if (lockfd == -1)
+            return -1;
 
-    if ((lockfd = open(lockpath, O_RDWR | O_CREAT, 0644)) < 0) {
-        dbd_log( LOGSTD, "Error opening lockfile: %s", strerror(errno));
-        exit(EXIT_FAILURE);
+        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 ((stat(dbpath, &st)) != 0) {
-        dbd_log( LOGSTD, "Error statting lockfile: %s", strerror(errno));
-        exit(EXIT_FAILURE);
-    }
+    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 ((chown(lockpath, st.st_uid, st.st_gid)) != 0) {
-        dbd_log( LOGSTD, "Error inheriting lockfile permissions: %s", strerror(errno));
-        exit(EXIT_FAILURE);
+        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;
@@ -201,28 +227,19 @@ static int get_lock(const char *dbpath)
     if (fcntl(lockfd, F_SETLK, &lock) < 0) {
         if (errno == EACCES || errno == EAGAIN) {
             if (exclusive) {
-                dbd_log( LOGSTD, "Database is in use and exlusive was requested", strerror(errno));        
-                exit(EXIT_FAILURE);
+                dbd_log(LOGSTD, "Database is in use and exlusive was requested");
+                return -1;
             };
+            dbd_log(LOGDEBUG, "get_lock: couldn't lock");
+            return 0;
         } else {
             dbd_log( LOGSTD, "Error getting fcntl F_WRLCK on lockfile: %s", strerror(errno));
-            exit(EXIT_FAILURE);
+            return -1;
        }
     }
-    
-    return lockfd;
-}
 
-static void free_lock(int lockfd)
-{
-    struct flock lock;
-
-    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);
+    dbd_log(LOGDEBUG, "get_lock: got lock");    
+    return 1;
 }
 
 static void usage (void)
@@ -406,11 +423,9 @@ int main(int argc, char **argv)
         close(dbdirfd);
     }
 
-    /* 
-       Before we do anything else, check if there is an instance of cnid_dbd
-       running already and silently exit if yes.
-    */
-    lockfd = get_lock(dbpath);
+    /* Get db lock, which exits if exclusive was requested and it already is locked */
+    if ((db_locked = get_lock(1, dbpath)) == -1)
+        goto exit_failure;
 
     /* Prepare upgrade ? */
     if (prep_upgrade) {
@@ -422,12 +437,14 @@ int main(int argc, char **argv)
     /* Check if -f is requested and wipe db if yes */
     if ((flags & DBD_FLAGS_FORCE) && rebuild && (volinfo.v_flags & AFPVOL_CACHE)) {
         char cmd[8 + MAXPATHLEN];
-        close(lockfd);
+        if ((db_locked = get_lock(0, NULL)) != 0)
+            goto exit_failure;
         snprintf(cmd, 8 + MAXPATHLEN, "rm -f %s/*", dbpath);
         dbd_log( LOGDEBUG, "Removing old database of volume: '%s'", volpath);
         system(cmd);
         dbd_log( LOGDEBUG, "Removed old database.");
-        lockfd = get_lock(dbpath);
+        if ((db_locked = get_lock(1, dbpath)) == -1)
+            goto exit_failure;
     }
 
     /* 
@@ -437,7 +454,9 @@ int main(int argc, char **argv)
         if ((dbd = dbif_init(dbpath, "cnid2.db")) == NULL)
             goto exit_failure;
         
-        if (dbif_env_open(dbd, &db_param, exclusive ? (DBOPTIONS | DB_RECOVER) : DBOPTIONS) < 0) {
+        if (dbif_env_open(dbd,
+                          &db_param,
+                          exclusive ? (DBOPTIONS | DB_RECOVER) : DBOPTIONS) < 0) {
             dbd_log( LOGSTD, "error opening database!");
             goto exit_failure;
         }
@@ -480,7 +499,7 @@ exit_success:
     ret = 0;
 
 exit_failure:
-    free_lock(lockfd);
+    get_lock(0, NULL);
     
     if ((fchdir(cdir)) < 0)
         dbd_log(LOGSTD, "fchdir: %s", strerror(errno));
index 84b1191abc187384d91c89ab4c79743bae171965..0a4c2a8446eeb15b3db3cb26d3bf712f72d69a31 100644 (file)
@@ -25,12 +25,14 @@ typedef unsigned int dbd_flags_t;
         (strcmp(a,c) b 0)
 
 extern int nocniddb; /* Dont open CNID database, only scan filesystem */
+extern int db_locked; /* have we got the fcntl lock on lockfd ? */
 extern volatile sig_atomic_t alarmed;
 extern struct volinfo *volinfo;
 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(int cmd, const char *dbpath);
 
 /*
   Functions for querying the database which couldn't be reused from the existing
index fe58d8aae48467e43001dd753523d8dcb85b355a..2ec332690a2c01d800830bf87458c88236f99e8a 100644 (file)
@@ -855,6 +855,11 @@ 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))
index 42cf28489f14e61c055a73a73eb9a75a74742347..5cafb2ef12f9a0de4e70098892fd61ee073fddf6 100644 (file)
    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.
  */
-#define DBOPTIONS (DB_CREATE | DB_INIT_LOG | DB_INIT_MPOOL | DB_INIT_LOCK | DB_INIT_TXN | DB_RECOVER)
+#define DBOPTIONS (DB_CREATE | DB_INIT_LOG | DB_INIT_MPOOL | DB_INIT_LOCK | DB_INIT_TXN)
 
 static DBD *dbd;
-
 static int exit_sig = 0;
+static int db_locked;
 
 static void sig_exit(int signo)
 {
@@ -72,6 +72,57 @@ 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:
 
@@ -118,6 +169,20 @@ 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;
@@ -252,33 +317,6 @@ static void switch_to_user(char *dir)
     }
 }
 
-/* ------------------------ */
-static int get_lock(void)
-{
-    int lockfd;
-    struct flock lock;
-
-    if ((lockfd = open(LOCKFILENAME, O_RDWR | O_CREAT, 0644)) < 0) {
-        LOG(log_error, logtype_cnid, "main: error opening lockfile: %s", strerror(errno));
-        exit(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) {
-            exit(0);
-        } else {
-            LOG(log_error, logtype_cnid, "main: fcntl F_WRLCK lockfile: %s", strerror(errno));
-            exit(1);
-        }
-    }
-
-    return lockfd;
-}
 
 /* ----------------------- */
 static void set_signal(void)
@@ -302,25 +340,12 @@ static void set_signal(void)
     }
 }
 
-/* ----------------------- */
-static void free_lock(int lockfd)
-{
-    struct flock lock;
-
-    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);
-}
-
 /* ------------------------ */
 int main(int argc, char *argv[])
 {
     struct db_param *dbp;
     int err = 0;
-    int lockfd, ctrlfd, clntfd;
+    int ctrlfd, clntfd;
     char *dir, *logconfig;
 
     set_processname("cnid_dbd");
@@ -341,7 +366,9 @@ int main(int argc, char *argv[])
 
     /* Before we do anything else, check if there is an instance of cnid_dbd
        running already and silently exit if yes. */
-    lockfd = get_lock();
+    if ((db_locked = get_lock(1)) == -1) {
+        exit(1);
+    }
 
     LOG(log_info, logtype_cnid, "Startup, DB dir %s", dir);
 
@@ -357,7 +384,10 @@ int main(int argc, char *argv[])
     if (NULL == (dbd = dbif_init(".", "cnid2.db")))
         exit(2);
 
-    if (dbif_env_open(dbd, dbp, DBOPTIONS) < 0)
+    /* Only recover if we got the lock */
+    if (dbif_env_open(dbd,
+                      dbp,
+                      db_locked ? 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");
 
@@ -387,7 +417,7 @@ int main(int argc, char *argv[])
     if (dbif_prep_upgrade(dir) < 0)
         err++;
 
-    free_lock(lockfd);
+    (void)get_lock(0);
 
     if (err)
         exit(4);