Hi Eugene, 

On Wed, May 25, 2005 at 11:26:03PM +0800, Eugene Konev wrote:
 
> The attached patch adds calling db4.2_recover to slapd.init on every
> slapd startup.

Thanks again for the patch. I am currently working to apply your
changes. A few remarks while I am working on it:

> diff -Nru openldap2.2-2.2.23/debian/slapd.init 
> openldap2.2-hack/debian/slapd.init
> --- openldap2.2-2.2.23/debian/slapd.init      2005-05-24 19:42:21.000000000 
> +0800
> +++ openldap2.2-hack/debian/slapd.init        2005-05-25 23:15:35.000000000 
> +0800
> @@ -48,6 +48,10 @@
>               "$SLAPD_CONF"`
>  fi
>  
> +# Find out slapd db directories
> +SLAPD_DBDIRS=`sed -ne 's/^directory[[:space:]]\+"*\([^"]\+\).*/\1/p' \
> +                "$SLAPD_CONF" `
> +     

I'd rather gather this list at the time when it is needed. Apart from
that I don't really grok that sed expression :)

> +# Try to recover slapd database
> +try_fix_db() {
...
> +     if [ "$SLAPD_TRYFIXDB" != yes -o \
That switch makes no sense inside fix_db I'd say. I used to do it like
this but stumbled across it later because I was using that function
later and it did not do anything because it checked some magic variable.
Not going to happen here I'd say, but for consistency I'll move that
check.

> +     echo -n " (possibly) fixing db,"
I don't like that message - as a user I am not going to understand
what's going on there. Changed to "running BDB recovery".

> +     for DBDIR in $SLAPD_DBDIRS; do
> +         if [ -d "$DBDIR" -a -f "$DBDIR/objectClass.bdb" ]; then
Did not see that check at the first glance. Moving it to the point where
the list of BDB directories is collected. 

> +             db4.2_recover -eh $DBDIR 2>&1
What is that -e option about? I do not really understand the meaning.
Guess I'll keep it enabled. Perhaps it would be a good idea to install
a DB_CONFIG during upgrade and run db_recover without -e to get the new
settings!?

@Steve: What would you think about such a change?

> +             if [ $? -ne 0 ]; then
That will not work because of the "set -e" up in that file. The script
will bail out if db_recover fails and will never get to that check. It's
better to run 

        db4.2_recover ... || do stuff in case of failure

or run db_recover inside an if but that will hide the real functionality
of db_recover as an side effect of the if. 


I attached my current patch (currently building slapd to test it).
Suggestions welcome :)

Greetings

        Torsten
=== debian/slapd.default
==================================================================
--- debian/slapd.default  (revision 2061)
+++ debian/slapd.default  (revision 2063)
@@ -13,6 +13,9 @@
 # will try to figure it out from $SLAPD_CONF (/etc/ldap/slapd.conf)
 SLAPD_PIDFILE=
 
+# Confiure if db_recover should be called before starting slapd
+TRY_BDB_RECOVERY=yes
+
 # Configure if the slurpd daemon should be started. Possible values: 
 # - yes:   Always start slurpd
 # - no:    Never start slurpd
=== debian/changelog
==================================================================
--- debian/changelog  (revision 2061)
+++ debian/changelog  (revision 2063)
@@ -9,6 +9,10 @@
     (closes: #308234).
   * debian/slapd.postinst: Make sure the debhelper commands are executed 
     in all cases (closes: #310422).
+  * Merged suggested changes by Eugene Konev to automatically run 
+    db_recover before starting slapd (closes: #255276).
+    + debian/slapd.init: Run db_recover if enabled and available.
+    + debian/slapd.default: Add configuration option to disable it.
 
   Steve Langasek <[EMAIL PROTECTED]>:
   * libraries/libldap_r/Makefile.in: make sure the ximian-connector ntlm
@@ -17,7 +21,7 @@
     two versions of this library around is more trouble than it's worth,
     and can cause glorious segfaults down the line
 
- -- 
+ -- Torsten Landschoff <[EMAIL PROTECTED]>  Mon, 23 May 2005 17:03:53 +0200
 
 openldap2.2 (2.2.23-5) unstable; urgency=low
 
=== debian/slapd.init
==================================================================
--- debian/slapd.init  (revision 2061)
+++ debian/slapd.init  (revision 2063)
@@ -6,6 +6,9 @@
 # Stop processing if slapd is not there
 [ -x /usr/sbin/slapd ] || exit 0
 
+# Set default values
+DB_RECOVER_CMD=db4.2_recover
+
 # Source the init script configuration
 if [ -f "/etc/default/slapd" ]; then
        . /etc/default/slapd
@@ -106,7 +109,56 @@
        fi
 }
 
+# Try to recover slapd database
+try_fix_db() {
+       local dbdir failed bdb_envs
 
+       # db4.2-util is just recommended by slapd, so make sure it is
+       # available before trying to use it
+       if ! command -v $DB_RECOVER_CMD; then
+               echo -n " ($DB_RECOVER_CMD not found), "
+               return 0
+       fi
+
+       bdb_envs=`find_bdb_envs`
+
+       # We care only about BDB environments
+       if [ -z "$bdb_envs" ]; then
+               return 0
+       fi
+
+       echo -n " running BDB recovery,"
+       for dbdir in $bdb_envs; do
+               $DB_RECOVER_CMD -eh $dbdir || db_recover_failed $dbdir
+       done
+}
+
+# Find bdb environment dirs
+find_bdb_envs() {
+       local d
+       for d in `awk '/directory/ {print $2}' < "$SLAPD_CONF"`; do
+               if [ -d "$d" -a -f "$d/objectClass.bdb" ]; then
+                       echo $d
+               fi
+       done
+}
+
+# Inform the user that BDB recovery failed
+db_recover_failed() {
+       local dbdir
+       dbdir="$1"
+
+       reason="`cat <<EOF`"
+Automatic recovery of the OpenLDAP directory database in
+
+       $dbdir
+
+failed. You will need to perform a manual recovery, possibly from backup.
+The failed command was $DB_RECOVER_CMD -eh $dbdir. 
+EOF
+       exit 1
+}
+
 # Start the slapd daemon and capture the error message if any to 
 # $reason.
 start_slapd() {
@@ -157,6 +209,9 @@
 start() {
        echo -n "Starting OpenLDAP:"
        trap 'report_failure' 0
+       if [ "$TRY_BDB_RECOVERY" = "yes" ]; then
+               try_fix_db
+       fi
        start_slapd
        start_slurpd
        trap "-" 0

Attachment: signature.asc
Description: Digital signature

Reply via email to