Fix the chicken and egg problem with ctdb/samba and a registry smb.conf

This attempts to fix the problem of ctdb event scripts blocking due to
attempted access to the ctdb databases during recovery. The changes are:

  - now only the 'shutdown' and 'startrecovery' events can be called
    with the databases locked in recovery. The event scripts must ensure
    that for these two events no database access is attempted

  - the recovered, takeip and releaseip events could previously be called
    inside a recovery. The code now ensures that this doesn't happen, delaying
    the events till after recovery has finished

  - the 50.samba event script now avoids using testparm unless it is really
    needed

This needs extensive testing.

diff --git a/config/events.d/50.samba b/config/events.d/50.samba
index c67dbda684ab86c6c574c87e3c8c51935c5c4b49..784c059c499e48cbb8eed4fbf102344c3447b41f 100755 (executable)
--- a/config/events.d/50.samba
+++ b/config/events.d/50.samba
@@ -16,9 +16,9 @@ shift
     SAMBA_CLEANUP_PERIOD=10
 }
 
-
-# autodetect use of winbind if not set in config file
-[ -z "$CTDB_MANAGES_WINBIND" ] && {
+# function to see if ctdb manages winbind
+check_ctdb_manages_winbind() {
+  [ -z "$CTDB_MANAGES_WINBIND" ] && {
     secmode=`testparm -s --parameter-name=security 2> /dev/null`
     case $secmode in
        ADS|DOMAIN)
@@ -28,6 +28,7 @@ shift
            CTDB_MANAGES_WINBIND="no";
            ;;
     esac
+  }
 }
 
 ###########################
@@ -53,11 +54,12 @@ case $cmd in
        }
 
        # restart the winbind service
+       check_ctdb_manages_winbind
        [ "$CTDB_MANAGES_WINBIND" = "yes" ] && {
                service winbind stop > /dev/null 2>&1
                killall -0 -q winbindd && {
                    sleep 1
-                   # make absolutely sure winbindd is dead
+                   # make absolutely sure winbindd is dead
                    killall -q -9 winbindd
                }
                service winbind start
@@ -87,6 +89,7 @@ case $cmd in
        service smb stop
 
        # stop the winbind service
+       check_ctdb_manages_winbind
        [ "$CTDB_MANAGES_WINBIND" = "yes" ] && {
                service winbind stop
        }
@@ -116,6 +119,7 @@ case $cmd in
        ctdb_check_tcp_ports "Samba" $smb_ports
 
        # check winbind is OK
+       check_ctdb_manages_winbind
        [ "$CTDB_MANAGES_WINBIND" = "yes" ] && {
                ctdb_check_command "winbind" "wbinfo -p"
        }

diff --git a/config/events.d/README b/config/events.d/README
index bfa43726cde6dcc00988aacc012a2d964b5d57cc..a75da388a1420394fd8e7c273cd754cf4aa8b504 100644 (file)
--- a/config/events.d/README
+++ b/config/events.d/README
@@ -18,6 +18,9 @@ The eventscripts are called with varying number of arguments.
 The first argument is the "event" and the rest of the arguments depend
 on which event was triggered.
 
+All of the events except the 'shutdown' and 'startrecovery' events will be
+called with the ctdb daemon in NORMAL mode (ie. not in recovery)
+
 The events currently implemented are
 startup
        This event does not take any additional arguments.
@@ -74,7 +77,7 @@ takeip
 
        Before this event there will always be a 'startrecovery' event.
 
-       This event will always be followed by a 'recovered' event onse
+       This event will always be followed by a 'recovered' event once
        all ipaddresses have been reassigned to new nodes and the ctdb database
        has been recovered.
        If multiple ip addresses are reassigned during recovery it is

diff --git a/server/ctdb_recoverd.c b/server/ctdb_recoverd.c
index 7aca7cb4dd5cd1b47a7ae6d55470acf3a832efb3..5f8eb83cf9cf821a302caf87c884ab13f1e9c5eb 100644 (file)
--- a/server/ctdb_recoverd.c
+++ b/server/ctdb_recoverd.c
@@ -247,7 +247,8 @@ static int run_startrecovery_eventscript(struct ctdb_context *ctdb, struct ctdb_
 static void async_getcap_callback(struct ctdb_context *ctdb, uint32_t node_pnn, int32_t res, TDB_DATA outdata)
 {
        if ( (outdata.dsize != sizeof(uint32_t)) || (outdata.dptr == NULL) ) {
-               DEBUG(DEBUG_ERR, (__location__ " Invalid lenght/pointer for getcap callback : %d %p\n", outdata.dsize, outdata.dptr));
+               DEBUG(DEBUG_ERR, (__location__ " Invalid lenght/pointer for getcap callback : %u %p\n", 
+                                 (unsigned)outdata.dsize, outdata.dptr));
                return;
        }
        ctdb->nodes[node_pnn]->capabilities = *((uint32_t *)outdata.dptr);
@@ -1451,6 +1452,15 @@ static int do_recovery(struct ctdb_recoverd *rec,
        
        DEBUG(DEBUG_NOTICE, (__location__ " Recovery - updated flags\n"));
 
+       /* disable recovery mode */
+       ret = set_recovery_mode(ctdb, nodemap, CTDB_RECOVERY_NORMAL);
+       if (ret!=0) {
+               DEBUG(DEBUG_ERR, (__location__ " Unable to set recovery mode to normal on cluster\n"));
+               return -1;
+       }
+
+       DEBUG(DEBUG_NOTICE, (__location__ " Recovery - disabled recovery mode\n"));
+
        /*
          tell nodes to takeover their public IPs
         */
@@ -1471,15 +1481,6 @@ static int do_recovery(struct ctdb_recoverd *rec,
 
        DEBUG(DEBUG_NOTICE, (__location__ " Recovery - finished the recovered event\n"));
 
-       /* disable recovery mode */
-       ret = set_recovery_mode(ctdb, nodemap, CTDB_RECOVERY_NORMAL);
-       if (ret!=0) {
-               DEBUG(DEBUG_ERR, (__location__ " Unable to set recovery mode to normal on cluster\n"));
-               return -1;
-       }
-
-       DEBUG(DEBUG_NOTICE, (__location__ " Recovery - disabled recovery mode\n"));
-
        /* send a message to all clients telling them that the cluster 
           has been reconfigured */
        ctdb_send_message(ctdb, CTDB_BROADCAST_CONNECTED, CTDB_SRVID_RECONFIGURE, tdb_null);

diff --git a/server/eventscript.c b/server/eventscript.c
index f6afd47800de60154c67f27f99319db8ecdea251..f8a5077d478eafd507531a59d2e480471f049957 100644 (file)
--- a/server/eventscript.c
+++ b/server/eventscript.c
@@ -52,7 +52,6 @@ static int ctdb_event_script_v(struct ctdb_context *ctdb, const char *fmt, va_li
 {
        char *options, *cmdstr;
        int ret;
-       va_list ap2;
        struct stat st;
        TALLOC_CTX *tmp_ctx = talloc_new(ctdb);
        trbt_tree_t *tree;
@@ -60,6 +59,24 @@ static int ctdb_event_script_v(struct ctdb_context *ctdb, const char *fmt, va_li
        struct dirent *de;
        char *script;
 
+       options  = talloc_vasprintf(tmp_ctx, fmt, ap);
+       CTDB_NO_MEMORY(ctdb, options);
+
+       if (ctdb->recovery_mode != CTDB_RECOVERY_NORMAL) {
+               /* we guarantee that only some specifically allowed event scripts are run
+                  while in recovery */
+               const char *allowed_scripts[] = {"startrecovery", "shutdown" };
+               int i;
+               for (i=0;i<ARRAY_SIZE(allowed_scripts);i++) {
+                       if (strcmp(options, allowed_scripts[i]) == 0) break;
+               }
+               if (i == ARRAY_SIZE(allowed_scripts)) {
+                       DEBUG(0,("Refusing to run event scripts with option '%s' while in recovery\n",
+                                options));
+               }
+               return -1;
+       }
+
        if (setpgid(0,0) != 0) {
                DEBUG(DEBUG_ERR,("Failed to create process group for event scripts - %s\n",
                         strerror(errno)));
@@ -146,11 +163,6 @@ static int ctdb_event_script_v(struct ctdb_context *ctdb, const char *fmt, va_li
           them
         */
        while ((script=trbt_findfirstarray32(tree, 1)) != NULL) {
-               va_copy(ap2, ap);
-               options  = talloc_vasprintf(tmp_ctx, fmt, ap2);
-               va_end(ap2);
-               CTDB_NO_MEMORY(ctdb, options);
-
                cmdstr = talloc_asprintf(tmp_ctx, "%s/%s %s", 
                                ctdb->event_script_dir,
                                script, options);