Bug 208672 - parameterizing the hardcoded paths (phase 2. db, log, lock, pid, cert, and instance dir)
parameterizing the hardcoded paths (phase 2. db, log, lock, pid, cert, and in...
Status: CLOSED CURRENTRELEASE
Product: 389
Classification: Community
Component: Directory Server (Show other bugs)
1.0.2
All Linux
medium Severity medium
: ---
: ---
Assigned To: Noriko Hosoi
Viktor Ashirov
:
Depends On:
Blocks: 152373 240316
  Show dependency treegraph
 
Reported: 2006-09-29 20:39 EDT by Noriko Hosoi
Modified: 2015-12-07 12:15 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2015-12-07 12:15:19 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
cvs diffs (ldapserver) (38.44 KB, patch)
2006-09-29 20:45 EDT, Noriko Hosoi
no flags Details | Diff
cvs diffs (ldapserver -- revised) (53.87 KB, patch)
2006-10-03 19:45 EDT, Noriko Hosoi
no flags Details | Diff
cvs diffs (ldapserver -revised) (189.38 KB, patch)
2006-10-10 20:11 EDT, Noriko Hosoi
no flags Details | Diff
cvs commit message (for comment #18) (7.58 KB, text/plain)
2006-10-12 21:12 EDT, Noriko Hosoi
no flags Details

  None (edit)
Description Noriko Hosoi 2006-09-29 20:39:45 EDT
Description of problem:
to prepare for FHS packaging.
Comment 1 Noriko Hosoi 2006-09-29 20:45:48 EDT
Created attachment 137456 [details]
cvs diffs (ldapserver)

Files:
  ldap/admin/include/dsalib.h
  ldap/admin/lib/dsalib_db.c
  ldap/admin/lib/dsalib_location.c
  ldap/admin/lib/dsalib_util.c
  ldap/admin/src/create_instance.c
  ldap/admin/src/create_instance.h
  ldap/admin/src/ds_remove.c
  ldap/servers/slapd/libglobs.c
  ldap/servers/slapd/protect_db.c
  ldap/servers/slapd/proto-slap.h
  ldap/servers/slapd/slap.h
  ldap/servers/slapd/snmp_collator.c
  ldap/servers/slapd/back-ldbm/ldbm_config.c

Changes:
1) introduced these 2 config attributes.
+#define CONFIG_LOCKDIR_ATTRIBUTE "nsslapd-lockdir"
+#define CONFIG_DBDIR_ATTRIBUTE "nsslapd-dbdir"
2) replaced the hardcoded paths with the corresponding attribute value in the
   server as well as in the create/remove instance codes.
Comment 4 Nathan Kinder 2006-10-02 16:40:42 EDT
The changes in comment#1 look good for the most part.  I have the following
comments:

In dsalib_util.c , a comment says " HFS changes the directory structure". It
should say that "FHS changes the ..." instead since the comment is referring to
the Filesystem Hierarchy Standard.

In libglobs.c, you are setting the getter and setter function pointers to NULL
for CONFIG_LDIFDIR_ATTRIBUTE.  Do we even need to keep CONFIG_LDIFDIR_ATTRIBUTE
around if there are no longer getter and setter functions?

I'm not sure of what we should be doing with regards to doing an internal search
for the dbdir in ldbm_config.c.  I believe that it's possible that the
nsslapd-dbdir attribute is different than the frontend global config setting, so
I guess it depends on if you want the currently used setting (which would be the
frontend config), or a setting which may be a pending change (which could be
different from the currently used setting).
Comment 5 Noriko Hosoi 2006-10-02 19:55:13 EDT
(In reply to comment #4)
> The changes in comment#1 look good for the most part.  I have the following
> comments:
> 
> In dsalib_util.c , a comment says " HFS changes the directory structure". It
> should say that "FHS changes the ..." instead since the comment is referring to
> the Filesystem Hierarchy Standard.

Ooooops. :p  Thanks for finding it out!

And thank you for summarizing the questionable codes, for me!  They are exactly
what I'd like to know...

> In libglobs.c, you are setting the getter and setter function pointers to NULL
> for CONFIG_LDIFDIR_ATTRIBUTE.  Do we even need to keep CONFIG_LDIFDIR_ATTRIBUTE
> around if there are no longer getter and setter functions?

My goal is keeping the attribute type and value in dse.ldif:
  nsslapd-ldapdir: <dir_to_ldif_dir>
without any overhead on the server.  This line is added by ds_newinst.  The
server does not need the info (for now).  But I thought it'd be handy to keep
the path info in the config file to share among tools.  (For now, the test tool
is getting the ldif path info from dse.ldif.)

> I'm not sure of what we should be doing with regards to doing an internal search
> for the dbdir in ldbm_config.c.  I believe that it's possible that the
> nsslapd-dbdir attribute is different than the frontend global config setting, so
> I guess it depends on if you want the currently used setting (which would be the
> frontend config), or a setting which may be a pending change (which could be
> different from the currently used setting).

Right...  I think there is no chance to change nsslapd-dbdir value.  The path is
more like a default location of the first db.  Each backend entry has its db
location attribute, e.g., "nsslapd-directory:
/export/servers/ds72_core/var/lib/slapd-laputa/db/NetscapeRoot", which could be
changed independent from dbdir.  

Hmmm, I guess I should not introduce nsslapd-dbdir to cn=config, but use
nsslapd-directory in the entry "cn=config,cn=ldbm
database,cn=plugins,cn=config".  And let the test tool get the db dir info from
the entry.  (Frankly, it makes the test tool more complicated since the same
attribute type "nsslapd-directory" is used in the entry as well as each backend
instance entry.  And the simple grep fails to work... :)  I'm going to take back
introducing nsslapd-dbdir, and use the one in "cn=config,cn=ldbm
database,cn=plugins,cn=config".  I'll repost the review request shortly...
Comment 6 Noriko Hosoi 2006-10-03 19:45:35 EDT
Created attachment 137712 [details]
cvs diffs (ldapserver -- revised)

Thanks for the comments and the discussion, Nathan.
Here's the additional changes:
1) I re-thought about the 2 issues and have decided to get rid of nsslapd-dbdir
and nsslapd-ldifdir from cn=config.  Instead of referring nsslapd-dbdir, we can
use nsslapd-directory in cn=config,cn=ldbm database,cn=plugins,cn=config.  And
for nsslapd-ldifdir, it's basically "defined" in db2ldif, where you can get the
location info.
2) In terms of db2ldif and db2ldif.pl, I added server_id prefix to the default
exported file name.  Before, each instance had its own ldif dir, but under HFS,
there is one ldif directory shared among servers on one host.  To make it
easier to know the origin of the ldif, I propose to add <server_id>- in front
of the generated ldif file name like this:
  <prefix>/share/fedora-ds/ldif/<serverID>-2006_10_03_162640.ldif
3) In the same token, when another instance is added to an existing server
sharing the same directory structure, default ldif files such as Example.ldif
and Example-roles.ldif files are overwritten.  So, I added a backup code to
keep the existing files.  (note that if the file having the extension ".bak"
already exists, the file is overridden...)

Files:
  ldap/admin/include/dsalib.h
  ldap/admin/lib/dsalib_db.c
  ldap/admin/lib/dsalib_location.c
  ldap/admin/lib/dsalib_util.c
  ldap/admin/src/create_instance.c
  ldap/admin/src/create_instance.h
  ldap/admin/src/ds_remove.c
  ldap/admin/src/scripts/template-db2ldif.pl
  ldap/servers/slapd/libglobs.c
  ldap/servers/slapd/protect_db.c
  ldap/servers/slapd/proto-slap.h
  ldap/servers/slapd/slap.h
  ldap/servers/slapd/snmp_collator.c
  ldap/servers/slapd/back-ldbm/ldbm_config.c
  ldap/servers/slapd/back-ldbm/ldbm_config.h
Comment 10 Rich Megginson 2006-10-04 18:07:01 EDT
In snmp_collator.c - do we really need to extract the log path from the access
log, or can we just use the logdir in the global config?

Other than that, looks good.
Comment 11 Nathan Kinder 2006-10-04 19:13:53 EDT
(In reply to comment #10)
> In snmp_collator.c - do we really need to extract the log path from the access
> log, or can we just use the logdir in the global config?

We may want to rethink where we are storing the snmp statistics file.  It's
really not a logfile, but more of a database of sorts.  As we move to a FHS
layout, placing this file in the log directory may make less sense that it does
with our current self-contained instance layout.

Comment 12 Rich Megginson 2006-10-04 19:16:56 EDT
Is the snmp stats file a standard thing i.e. is there any guidance in the
net-snmp package about where these types of files might go?  Where do other
packages put their snmp stats files?
Comment 13 Nathan Kinder 2006-10-04 19:28:05 EDT
The SNMP stats file is very much our own thing.  We basically have an im-memory
structuse that is used to store global statistics.  This is used for our
"cn=monitor" statistics as well as SNMP.  We basically just have a memory-mapped
file that gets written out by ns-slapd on some small regular interval (5 seconds
I think).  Our SNMP sub-agent then reads this file into memory to respond to
SNMP queries.

I'm not sure exactly what other packages do for SNMP.  I would guess that the
location of the values used to respond to SNMP queries varies widely across
different packages though (some may be in a db, some in a flat file, some
returned by a function call to a library, etc.).  Net-SNMP doesn't provide any
pointers on this.
Comment 14 Noriko Hosoi 2006-10-04 19:44:49 EDT
Thanks for the discussion and the background info.  So, we can/have to decide
the location to put the file?

For now, we have these paths in cn=config in addition to the 3 log paths:
  nsslapd-instancedir: <server-root>/slapd-<id> <== will be moved to
<server-root>/lib/fedora-ds/slapd-<id>
  nsslapd-schemadir: <server-root>/var/lib/slapd-<id>/schema
  nsslapd-lockdir: <server-root>/var/lock/slapd-<id>

Do you think any of these can be used to store slapd.stats?  If not, how about,
for instance, adding rundir (pid dir) <server-root>/run/slapd-<id> to cn=config
and using the directory for the file?
Comment 15 Rich Megginson 2006-10-04 20:25:20 EDT
Seems like the log dir or the db dir is the most appropriate place for the stats
file.  Do the contents of this file have to persist between server restarts?  If
not, then perhaps /tmp or /var/tmp, especially since it's only used by the
sub-agent.

Do we need instancedir anymore?
Comment 16 Noriko Hosoi 2006-10-04 21:03:49 EDT
(In reply to comment #15)
> Seems like the log dir or the db dir is the most appropriate place for the stats
> file.  Do the contents of this file have to persist between server restarts?  

Nope.  When the server is shutdown, the server even cleans it up!

> If
> not, then perhaps /tmp or /var/tmp, especially since it's only used by the
> sub-agent.

Okay, /var/tmp/slapd-<id>, then?

> Do we need instancedir anymore?

There are some places refered in slapd, but most of them are obsolete.  I've
found two more places which refer the old path in the search... :p  
One is slapd_nss_init in slapd/ssl.c.  If "nscertfile""nskeyfile" is set in
config, the files are searched in the instance dir.  (BTW, could these config
attributes be still alive???)
The second one is back-ldbm/archive.c.  It has some obsolete codes to convert
the relative path to absolute path based upon the assumption that the backup dir
is in the instancedir.

I'm going to fix these...
Comment 17 Rich Megginson 2006-10-04 22:06:49 EDT
We don't need nscertfile/nskeyfile we only need the key/cert directory, which in
our case will be the configdir.  I'm not sure what the plans are for the system
wide secmoddb, but we can probably assume for now that we can also use configdir
for that.
Comment 18 Noriko Hosoi 2006-10-10 20:11:33 EDT
Created attachment 138207 [details]
cvs diffs (ldapserver -revised)

Files:
  ldap/admin/include/dsalib.h
  ldap/admin/lib/dsalib_conf.c
  ldap/admin/lib/dsalib_db.c
  ldap/admin/lib/dsalib_location.c
  ldap/admin/lib/dsalib_util.c
  ldap/admin/src/create_instance.c
  ldap/admin/src/create_instance.h
  ldap/admin/src/ds_remove.c
  ldap/admin/src/scripts/template-bak2db.pl
  ldap/admin/src/scripts/template-db2bak.pl
  ldap/admin/src/scripts/template-db2index.pl
  ldap/admin/src/scripts/template-db2ldif.pl
  ldap/admin/src/scripts/template-ldif2db.pl
  ldap/admin/src/scripts/template-ns-accountstatus.pl
  ldap/admin/src/scripts/template-ns-activate.pl
  ldap/admin/src/scripts/template-ns-inactivate.pl
  ldap/servers/plugins/http/http_impl.c
  ldap/servers/plugins/rever/des.c
  ldap/servers/slapd/auth.c
  ldap/servers/slapd/ch_malloc.c
  ldap/servers/slapd/config.c
  ldap/servers/slapd/libglobs.c
  ldap/servers/slapd/libslapd.def
  ldap/servers/slapd/log.c
  ldap/servers/slapd/main.c
  ldap/servers/slapd/protect_db.c
  ldap/servers/slapd/proto-slap.h
  ldap/servers/slapd/slap.h
  ldap/servers/slapd/snmp_collator.c
  ldap/servers/slapd/ssl.c
  ldap/servers/slapd/back-ldbm/archive.c
  ldap/servers/slapd/back-ldbm/ldbm_config.c
  ldap/servers/slapd/back-ldbm/ldbm_config.h
  branding/fedora/brandver.dat
  branding/rhds/brandver.dat

Additional Changes:
1) parameterized the server instance directory
2) removed nsslapd-instancedir from the config
3) put slapd.stats (snmp stats file) in var/tmp/slapd-<id>
Comment 27 Noriko Hosoi 2006-10-11 19:23:50 EDT
Shame, shame...  I replaced "HFS" strings with "FHS".  (All were in comments...) :p

Thanks, Nathan!
Comment 32 Nathan Kinder 2006-10-12 19:02:41 EDT
The (many) changes in comment#18 look good.
Comment 33 Noriko Hosoi 2006-10-12 21:12:22 EDT
Created attachment 138397 [details]
cvs commit message (for comment #18)

Reviewed by Nathan (Thank you!!!)

Checked in into HEAD.

Note You need to log in before you can comment on or make changes to this bug.