eventscripts: Rewrite the smb.conf cache file handling

The background update is never guaranteed to complete before the cache
is used, so don't bother trying it at the beginning.  Instead, put a
timeout on a foreground update.

If the foreground update fails:

* If there's no available cache file then die.

* If there is a previous cache file then use it and log a warning.

* Do a background update at the end of the monitor event.

Also remove commas in the "smb ports" list before use, since (newer?)
testparm seem to insert commas into the default value.  Update the
associated test to add a comma.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Pair-programmed-with: Amitay Isaacs <amitay@gmail.com>

diff --git a/config/events.d/50.samba b/config/events.d/50.samba
index 117b45926575ead20c1eaf96922ab2415b775ca0..4b53cba01be6f9736a25a2509df692755fc7b73a 100755 (executable)
--- a/config/events.d/50.samba
+++ b/config/events.d/50.samba
@@ -68,74 +68,44 @@ service_stop ()
     fi
 }
 
-# we keep a cached copy of smb.conf here
+######################################################################
+# Show the testparm output using a cached smb.conf to avoid delays due
+# to registry access.
+
 smbconf_cache="$service_state_dir/smb.conf.cache"
 
+testparm_foreground_update ()
+{
+    _timeout="$1"
+
+    if ! _out=$(timeout $_timeout testparm -v -s 2>/dev/null) ; then
+       if [ -f "$smbconf_cache" ] ; then
+           echo "WARNING: smb.conf cache update failed - using old cache file"
+           return 1
+       else
+           die "ERROR: smb.conf cache create failed"
+       fi
+    fi
+
+    _tmpfile="${smbconf_cache}.$$"
+    # Patterns to exclude...
+    pat='^[[:space:]]+(registry[[:space:]]+shares|include|copy|winbind[[:space:]]+separator)[[:space:]]+='    
+    echo "$_out" | grep -Ev "$pat" >"$_tmpfile"
+    mv "$_tmpfile" "$smbconf_cache" # atomic
 
-#############################################
-# update the smb.conf cache in the foreground
-testparm_foreground_update() {
-    testparm -s 2> /dev/null | egrep -v 'registry.shares.=|include.=' > "$smbconf_cache"
+    return 0
 }
 
-#############################################
-# update the smb.conf cache in the background
-testparm_background_update() {
-    # if the cache doesn't exist, then update in the foreground
-    [ -f $smbconf_cache ] || {
-       testparm_foreground_update
-    }
-    # otherwise do a background update
-    (
-       tmpfile="${smbconf_cache}.$$"
-       testparm -s > $tmpfile 2> /dev/null &
-       # remember the pid of the teamparm process
-       pid="$!"
-       # give it 10 seconds to run
-       timeleft=10
-       while [ $timeleft -gt 0 ]; do
-           timeleft=$(($timeleft - 1))
-           # see if the process still exists
-           kill -0 $pid > /dev/null 2>&1 || {
-               # it doesn't exist, grab its exit status
-               wait $pid
-               [ $? = 0 ] || {
-                   echo "50.samba: smb.conf background update exited with status $?"
-                   rm -f "${tmpfile}"
-                   exit 1
-               }               
-               # put the new smb.conf contents in the cache (atomic rename)
-               # make sure we remove references to the registry while doing 
-               # this to ensure that running testparm on the cache does
-               # not use the registry
-               egrep -v 'registry.shares.=|include.=' < "$tmpfile" > "${tmpfile}.2"
-               rm -f "$tmpfile"
-               mv -f "${tmpfile}.2" "$smbconf_cache" || {
-                   echo "50.samba: failed to update background cache"
-                   rm -f "${tmpfile}.2"
-                   exit 1
-               }
-               exit 0
-           }
-           # keep waiting for testparm to finish
-           sleep 1
-       done
-       # it took more than 10 seconds - kill it off
-       rm -f "${tmpfile}"
-       kill -9 "$pid" > /dev/null 2>&1
-       echo "50.samba: timed out updating smbconf cache in background"
-       exit 1
-    ) &
+testparm_background_update ()
+{
+    _timeout="$1"
+
+    testparm_foreground_update $_timeout >/dev/null 2>&1 </dev/null &
 }
 
-##################################################
-# show the testparm output using a cached smb.conf 
-# to avoid registry access
-testparm_cat() {
-    [ -f $smbconf_cache ] || {
-       testparm_foreground_update
-    }
-    testparm -v -s "$smbconf_cache" "$@" 2>/dev/null
+testparm_cat ()
+{
+    testparm -s "$smbconf_cache" "$@" 2>/dev/null
 }
 
 list_samba_shares ()
@@ -145,6 +115,11 @@ list_samba_shares ()
     sed -e 's/"//g'
 }
 
+list_samba_ports ()
+{
+    testparm_cat --parameter-name="smb ports" |
+    sed -e 's@,@ @g'
+}
 
 ###########################
 
@@ -164,27 +139,23 @@ case "$1" in
        ;;
 
      monitor)
-       if [ "$CTDB_SAMBA_SKIP_SHARE_CHECK" != "yes" ] ; then
-           testparm_background_update
-
-           testparm_cat | egrep '^WARNING|^ERROR|^Unknown' && {
-               testparm_foreground_update
-               testparm_cat | egrep '^WARNING|^ERROR|^Unknown' && \
-                   die "ERROR: testparm shows smb.conf is not clean"
-           }
-
-           list_samba_shares | ctdb_check_directories_probe || {
-               testparm_foreground_update
-               list_samba_shares |
-               ctdb_check_directories
-           } || exit $?
-       fi
+       testparm_foreground_update 10
+       ret=$?
 
        smb_ports="$CTDB_SAMBA_CHECK_PORTS"
        if [ -z "$smb_ports" ] ; then
-           smb_ports=`testparm_cat --parameter-name="smb ports"`
+           smb_ports=$(list_samba_ports)
+           [ -n "$smb_ports" ] || die "Failed to set smb ports"
        fi
        ctdb_check_tcp_ports $smb_ports || exit $?
+
+       if [ "$CTDB_SAMBA_SKIP_SHARE_CHECK" != "yes" ] ; then
+           list_samba_shares | ctdb_check_directories || exit $?
+       fi
+
+       if [ $ret -ne 0 ] ; then
+           testparm_background_update 10
+       fi
        ;;
 
     *)

diff --git a/tests/eventscripts/stubs/testparm b/tests/eventscripts/stubs/testparm
index 9e3747f45c70d37abd4f008d80be3dabed98d907..aac5b18110540874599a4c00874cd29f78cb3a3a 100755 (executable)
--- a/tests/eventscripts/stubs/testparm
+++ b/tests/eventscripts/stubs/testparm
@@ -23,7 +23,7 @@ done
 # parsed out of the file.
 case "$parameter" in
     security) echo "ADS" ; exit 0 ;;
-    smb*ports) echo "445 139" ; exit 0 ;;
+    smb*ports) echo "445, 139" ; exit 0 ;;
     ?*) not_implemented "--parameter-name=$parameter" ;;
     # Fall through if $parameter not set
 esac