Bug 207427

Created attachment 136799 [details]
cvs diffs

Files:
 ldap/admin/include/dsalib.h  ldap/admin/lib/dsalib_conf.c
 ldap/admin/lib/dsalib_location.c
 ldap/admin/lib/dsalib_updown.c
 ldap/admin/src/cfg_sspt.c
 ldap/admin/src/create_instance.c
 ldap/admin/src/create_instance.h
 ldap/admin/src/ds_newinst.c
 ldap/admin/src/ds_newinst.pl
 ldap/admin/src/makemccvlvindexes
 ldap/servers/slapd/auth.c
 ldap/servers/slapd/config.c

 ldap/servers/slapd/dse.c
 ldap/servers/slapd/fedse.c
 ldap/servers/slapd/libglobs.c
 ldap/servers/slapd/main.c
 ldap/servers/slapd/proto-slap.h
 ldap/servers/slapd/schema.c
 ldap/servers/slapd/slap.h
 ldap/servers/slapd/test-plugins/testbind.c
 ldap/servers/slapd/tools/pwenc.c

Changes:
0. this is just the first phase... :)
1. changed the option "-D <instancedir>" for ns-slapd to "-D <configdir>"
2. introduced FHS fields to the admin config structure and modified
create_instance using them.  (the job is not completed yet since the location
should be synchronized with the server side.  Such codes are found with the
comment "/* will go away */") 
+ * FHS description
+ * cf->prefix: %{_prefix}
+ * cf->sroot: %{_libdir}/BRAND_DS
+ * cf->localstatedir: %{_localstatedir}
+ * cf->sysconfdir: %{_sysconfdir}
+ * cf->bindir: %{_bindir}
+ * cf->datadir: %{_datadir}
+ * cf->docdir: %{_docdir}
+ * cf->inst_dir: <sroot>/slapd-<servid>
+ * cf->config_dir: <localstatedir>/lib/slapd-<servid>
+ * cf->schema_dir: <localstatedir>/lib/slapd-<servid>/schema
+ * cf->lock_dir: <localstatedir>/lock/slapd-<servid>
+ * cf->log_dir: <localstatedir>/log/slapd-<servid>
+ * cf->run_dir: <localstatedir>/run/slapd-<servid>
3. introduced "prefix".  this is the code in ds_newinst.pl.
+if ($table{General}->{prefix}) {
+        $cgiargs{prefix} = $table{General}->{prefix};
+}
And having this line in [General] section of the install info file
prefix=  /export/servers/ds72_core
the above FHS directories starts from the prefix directory.
4. I also cleaned up some dead looking codes especially in create_instance.c.

Installation:
$ cd <serverroot>
$ tar -xzvf <path_to_slapd.tar.gz>
$ perl ./bin/slapd/admin/bin/ds_newinst.pl <path_to_your_install_inf_file>
Success!  Your new directory server instance was created

Probably a copy/paste error on access and audit logs:
+#if 0
+    fprintf(f, "nsslapd-errorlog: %s/access\n", cf->log_dir);
+#else /* will go away */
and
+#if 0
+    fprintf(f, "nsslapd-errorlog: %s/audit\n", cf->log_dir);
+#else /* will go away */

Do we really want to get rid of the chown code during instance creation?

We still need that code that reads from ssusers.conf.  It is not used when doing
instance creation with ds_newinst, but it is used with ds_create.  So it's ok to
remove it, we just need to use it elsewhere.

Is it important to have instancedir still?  What uses it?

Thanks, Rich, for finding them out!  I replaced nsslapd-errorlog with the
correct directives...

#if 0
    fprintf(f, "nsslapd-accesslog: %s/access\n", cf->log_dir);
#else /* will go away */

#if 0
    fprintf(f, "nsslapd-auditlog: %s/audit\n", cf->log_dir);
#else /* will go away */

> Do we really want to get rid of the chown code during instance creation?
Could you tell me which code you are mentioning here?  Actually, I moved chown
dir code into create_instance_mkdir_p to apply recursively.  In my test, I
installed the DS as root with the server user "nhosoi".  I had to change not
just the bottom dir of the path, but in the middle as well, if the created
directory is not one level.

> We still need that code that reads from ssusers.conf.
I resurrected getSuiteSpotUserGroup and the calling points in create_server and
update_server.

> Is it important to have instancedir still?  What uses it?
Actually, unknown... :)  I was just thinking someone who opens the config file
and wants to find out where the instance is really located.  Maybe, no use. :)

Thanks!  (I'm attaching the revised diff...)

Created attachment 136899 [details]
revised cvs diffs

Created attachment 137191 [details]
cvs diffs (revised)

Files:
 ldap/admin/include/dsalib.h
 ldap/admin/lib/dsalib_conf.c
 ldap/admin/lib/dsalib_location.c
 ldap/admin/lib/dsalib_updown.c
 ldap/admin/src/cfg_sspt.c
 ldap/admin/src/create_instance.c
 ldap/admin/src/create_instance.h
 ldap/admin/src/ds_newinst.c
 ldap/admin/src/ds_newinst.pl
 ldap/admin/src/makemccvlvindexes
 ldap/cm/Makefile
 ldap/servers/slapd/auth.c
 ldap/servers/slapd/config.c
 ldap/servers/slapd/dse.c
 ldap/servers/slapd/fedse.c
 ldap/servers/slapd/libglobs.c
 ldap/servers/slapd/main.c
 ldap/servers/slapd/proto-slap.h
 ldap/servers/slapd/schema.c
 ldap/servers/slapd/slap.h
 ldap/servers/slapd/test-plugins/testbind.c
 ldap/servers/slapd/tools/pwenc.c

Changes:
In addition to Comment#1 and #5, fixed bugs found in the acceptance tests.
Now, it passes 89% (about the same as the nightly test)

The setenv() function is not supported on solaris.  Use putenv() instead for
maximal portability.

It seems as though CROOT is set in many places.  Is there a place we can just
set it once?

Otherwise, looks good.

I don't think that putenv will work either.  I believe putenv requires either
static (e.g. a static array) or global (a global variable or malloc) memory. 
That is, it requires the storage to persist after the call, and a stack variable
won't work.

(In reply to comment #10)
> I don't think that putenv will work either.  I believe putenv requires either
> static (e.g. a static array) or global (a global variable or malloc) memory. 
> That is, it requires the storage to persist after the call, and a stack variable
> won't work.

Oops...  I've added "static"...  Thanks!

DS_EXPORT_SYMBOL void
ds_set_config_dir(char *config_dir)
{
    static char env[PATH_MAX];
    PR_snprintf(env, sizeof(env), "DS_CONFIG_DIR=%s", config_dir);
    putenv(env);
}

Ok.

You should be declaring the ds_get_run_dir() function in dsalib.h.  Other than
that, the diffs in comment#6 look good.

Created attachment 137266 [details]
cvs diffs (ldapserver -- revised)

Final diffs including the fixes Rich and Nathan suggested.  (Thanks!)

Created attachment 137272 [details]
cvs commit message (ldapserver)

Thank you, reviewers!  Checked in into HEAD.