[purposefully on -devel for discussion]
Any opinions on:
--- init.c~ 2008-03-12 10:02:44.000000000 -0400 +++ init.c 2008-03-12 10:03:16.000000000 -0400 @@ -712,7 +712,7 @@ return -1; }
- Debug( LDAP_DEBUG_TRACE, LDAP_XSTRING(bdb_back_initialize) + Debug( LDAP_DEBUG_ANY, LDAP_XSTRING(bdb_back_initialize) ": %s\n", version, 0, 0 ); }
I'm definitely one for 'quiet' software, but knowing the version of libdb is a really nice detail when assisting users of OpenLDAP. This would make it that much easier to have them find it. I'd argue there's a bit of precedent:
main.c: Debug( LDAP_DEBUG_ANY, "%s", Versionstr, 0, 0 );
The negative, of course, is the noise. But what's ~60 bytes in this day and age?
Aaron Richton writes:
Debug( LDAP_DEBUG_TRACE, LDAP_XSTRING(bdb_back_initialize)
Debug( LDAP_DEBUG_ANY, LDAP_XSTRING(bdb_back_initialize) ": %s\n", version, 0, 0 ); }
I'm definitely one for 'quiet' software, but knowing the version of libdb is a really nice detail when assisting users of OpenLDAP. This would make it that much easier to have them find it. I'd argue there's a bit of precedent:
main.c: Debug( LDAP_DEBUG_ANY, "%s", Versionstr, 0, 0 );
That's not done in slap tools though. Which I like. No need to filter out "friendly" output from slap tools in in cron jobs &co.
How about (slapMode & SLAP_TOOL_MODE) ? LDAP_DEBUG_TRACE : LDAP_DEBUG_ANY?
Also, is it a good or bad idea to move this and Sleepycat initialization to the first time '<database/backend> <bdb/hdb>' is used?
Hi, I've charged with designing a new backend to slapd for a client, and this is the first time I've worked on OpenLDAP.
I've got the API documentation, and the setup type documentation from the website. None of that covers the backends much however, so I'm currently going through the various call hooks provided by the system using debug to see what is going on.
Is there any other documentation/information that would be relevant that I should know about?
Malcolm
Malcolm Cowen, Software Developer, Cowen Software Ltd: 21-23 Bristol Ave, Levenshulme, Manchester, GB- M19 3NU, UK +44 (0) 161 225 4674 (office) +44 (0) 7973 950 597 (mobile) Registered in England No: 1828889, VAT Reg 403 4867 60 www.cowensw.co.uk, www.cowensw.com, email: mct@cowensw.co.uk
<quote who="Malcolm Cowen">
Hi, I've charged with designing a new backend to slapd for a client, and this is the first time I've worked on OpenLDAP.
My first question is why a new backend? What are you/they trying to do? Why are you doing it if you've never seen OpenLDAP code before is another one ;-)
I've got the API documentation, and the setup type documentation from the website. None of that covers the backends much however, so I'm currently going through the various call hooks provided by the system using debug to see what is going on.
Is there any other documentation/information that would be relevant that I should know about?
The source.
Malcolm Cowen writes:
I've got the API documentation, and the setup type documentation from the website. None of that covers the backends much however, so I'm currently going through the various call hooks provided by the system using debug to see what is going on.
slap.h contains quite a bit of comments about the API.
Here are some more notes in C-like format I've written while trying to figure the API out. Note that I haven't checked yet that I've gotten it straight. See also ITS#5328.
struct BackendInfo { ...;
/* * be->be_<operation> aka bi->bi_op_<operation>: * * Handle one LDAP operation, except what auxiliary function below handle. * Set the SlapReply, in some cases send it too. * Return an LDAP result code. * ? [What is the relation between sent and returned result code?] ? * * bind * If returning LDAP_SUCCESS, do not send it first - the frontend will. * Do send non-success responses before returning. * unbind, abandon: * These LDAP operations have no response, so don't send one. * search, modify, modrdn, add, delete: * Send response. * compare: * Send response, but return LDAP_SUCCESS after sending * LDAP_COMPARE_FALSE or LDAP_COMPARE_TRUE. * bi_extended (be->be_extended), bi_op_cancel (be->be_cancel): * Do not send response, just set up the SlapReply. * * Operations that can be abandoned may return SLAPD_ABANDON if * op->o_abandon is set. It is OK to "send" SLAPD_ABANDON with * send_ldap_response(); that function will catch it. */ typedef int BI_op_func( Operation *op, SlapReply *rs ); BI_op_func *bi_op_bind; BI_op_func *bi_op_unbind; BI_op_func *bi_op_search; BI_op_func *bi_op_compare; BI_op_func *bi_op_modify; BI_op_func *bi_op_modrdn; BI_op_func *bi_op_add; BI_op_func *bi_op_delete; BI_op_func *bi_op_abandon; /* Extended Operations Helper */ BI_op_func *bi_extended; BI_op_func *bi_op_cancel;
/* Auxilary Functions */
/* * Insert backend-specific operational attributes in * rs->sr_operational_attrs. The frontend handles entryDN and * subschemaSubentry, unless REP_NO_ENTRYDN and REP_NO_SUBSCHEMA are * set. Called from backend_operational(). */ BI_op_func *bi_operational; /* be->be_operational() */
/* * Called before LDAP operation handlers. * May send and return a referral if the operation should do so. * The existence of this function makes LDAP operations non-atomic. * * It should send and return LDAP_REFERRAL, or return but NOT send * another result code. With LDAP_SUCCESS, slapd will proceed to * the operation handler. With other result codes, slapd will put * them in rs->sr_err and send the error. * * Called from backend_check_referrals(). */ BI_op_func *bi_chk_referrals; /* be->be_chk_referrals() */
/* * bi->bi_chk_controls() aka be->be_chk_controls(): * May react to controls in op->o_ctrlflag from the LDAP operation, * before slapd checks bi->bi_controls and globally handled controls. * Called before LDAP operation handlers. * * Do not send results. * Return SLAP_CB_CONTINUE for slapd to check other supported controls. * Return LDAP_SUCCESS to proceed to the operation without checking * other controls. Otherwise return an error code which slapd will * send to fail the operation. * * Called from backend_check_restrictions(). * * See overlays/pcache.c for an example. * Presumably it returns LDAP_SUCCESS, it must first do the work * normally done by backend_check_controls(). */ BI_op_func *bi_chk_controls; /* be->be_chk_controls() */
/* * bi->bi_entry_get_rw() aka be->be_fetch(): * * Fetch an entry for operation 'op' with normalized DN 'ndn', for * read operations if 'rw' == 0, for write operations if 'rw' == 1. * If 'oc' and/or 'at' are non-NULL but the entry does not contain * them, the function may fail with LDAP_NO_SUCH_ATTRIBUTE. * * Return an LDAP result code. *e is NULL on entry, and must be * non-NULL on exit if and only if LDAP_SUCCESS is returned. I think. * * Slapd returns the entry to the backend with bi_entry_release_rw() * if that is set, otherwise hopefully frees it with entry_free(). * ? [See below.] * * Called from be_entry_get_rw(). * * ? [What can the caller and the backend do with the entry before * it is released? Can it be modified? No current callers * use rw!=0, can it be modified in that case? ] * * ? [Callers of entry_release seem to choose rw based on which * LDAP operation is being used, which maybe means that * be_fetch() callers should do the same - i.e. set rw for * the Add operation even for entries which the caller will * only read. Need to check. Except, do any callers use * * rw != 0 anyway? ] */ int (*bi_entry_get_rw)( Operation *op, struct berval *ndn, ObjectClass *oc, AttributeDescription *at, int rw, Entry **e );
/* * bi->bi_entry_release_rw() aka be->be_release(): * * Returns an entry to the backend, possibly fetched with be_fetch. * * Slapd will also call it with entries that did not originate * with the backend, I do not know if that is intentional. * ? [After some overlay did entry_dup(), maybe. Must check.] * * Called from be_entry_release_rw(), which frees the entry with * entry_free() if there is no be_entry_release_rw function. * ? [However frontendDB->be_release() also calls this function, * but does not free the entry if no function is found. Then * it just returns LDAP_NO_SUCH_OBJECT.] * * Return an LDAP result code, which is hardly ever used. * Only SLAP_CB_CONTINUE seems to matter: If there are overlays, * this means other bi_entry_release_rw() functions are called, * or entry_free() if all else fails. * * ? [Maybe back-relay has a memory leak and should call entry_free * if there is no backend function. frontendDB does not, overlay * translucent does, backglue does but has a FIXME comment about * it. Fix what?] */ int (*bi_entry_release_rw)( Operation *op, Entry *e, int rw );
/* * bi->bi_has_subordinates() aka be->be_has_subordinates(): * * Generate the value for compare(entry, "hasSubordinates=TRUE"). * Returns an LDAP result code. When it returns LDAP_SUCCESS, it * must set *hasSubs to LDAP_COMPARE_TRUE or LDAP_COMPARE_FALSE. * * This function really should succeed except on internal errors: * test_filter() with the presence filter (hasSubordinates=*) does * not call this function, it assumes attribute hasSubordinates will * exist if this function does. And for equality match it turns * other result codes than LDAP_SUCCESS into LDAP_OTHER. (Though it * can also be used for the Compare operation, in that case a result * code of LDAP_NO_SUCH_ATTRIBUTE also makes sense.) * * back-relay sets this function, which is wrong when relaying to a * backend which does not. OTOH slapi_entry_has_children() exists * even though slapi does not set bi_has_subordinates. * * bi->bi_entry_get_rw must exist if this function does; Compare * will call it if !defined(SLAP_COMPARE_IN_FRONTEND). */ int (*bi_has_subordinates)( Operation *op, Entry *e, int *hasSubs );
...;
/* * Connection management, called when a connection is opened or closed. * Return an LDAP result code. * * Overlays must normally return SLAP_CB_CONTINUE, so the backend * also gets a chance to prepare for and clean up after connections. * * Slapd discards other result codes, which is just as well since the * overlay code turns SLAP_CB_CONTINUE into LDAP_UNWILLING_TO_PERFORM * when the backend has no connection handler. * * Slapi can set up a fake connection and call backend_connection_init, * but I see no matching call to backend_connection_destroy. Maybe * the connection is given to slapd which destroys it? */ typedef int BI_conn_func( BackendDB *bd, Connection *c ); BI_conn_func *bi_connection_init; /* be->be_connection_init */ BI_conn_func *bi_connection_destroy; /* be->be_connection_destroy */
/* Hooks for slap tools: bi->bi_tool_<act> aka be->be_<act> */ int (*bi_tool_entry_open)( BackendDB *be, int mode ); int (*bi_tool_entry_close)( BackendDB *be ); ...; };
Hallvard B Furuseth wrote:
slap.h contains quite a bit of comments about the API.
Here are some more notes in C-like format I've written while trying to figure the API out. Note that I haven't checked yet that I've gotten it straight. See also ITS#5328.
struct BackendInfo { ...;
/* * be->be_<operation> aka bi->bi_op_<operation>: * * Handle one LDAP operation, except what auxiliary function below handle. * Set the SlapReply, in some cases send it too. * Return an LDAP result code. * ? [What is the relation between sent and returned result code?] ? * * bind * If returning LDAP_SUCCESS, do not send it first - the frontend will. * Do send non-success responses before returning.
This inconsistency has always bothered me, and with the current callback layering there's no longer any reason for it. We should change Bind to behave like all the other ops and just let backends send responses for either success or failure.
* unbind, abandon: * These LDAP operations have no response, so don't send one. * search, modify, modrdn, add, delete: * Send response. * compare: * Send response, but return LDAP_SUCCESS after sending * LDAP_COMPARE_FALSE or LDAP_COMPARE_TRUE. * bi_extended (be->be_extended), bi_op_cancel (be->be_cancel): * Do not send response, just set up the SlapReply. * * Operations that can be abandoned may return SLAPD_ABANDON if * op->o_abandon is set. It is OK to "send" SLAPD_ABANDON with * send_ldap_response(); that function will catch it. */ typedef int BI_op_func( Operation *op, SlapReply *rs ); BI_op_func *bi_op_bind; BI_op_func *bi_op_unbind; BI_op_func *bi_op_search; BI_op_func *bi_op_compare; BI_op_func *bi_op_modify; BI_op_func *bi_op_modrdn; BI_op_func *bi_op_add; BI_op_func *bi_op_delete; BI_op_func *bi_op_abandon; /* Extended Operations Helper */ BI_op_func *bi_extended; BI_op_func *bi_op_cancel; /* Auxilary Functions */ /* * Insert backend-specific operational attributes in * rs->sr_operational_attrs. The frontend handles entryDN and * subschemaSubentry, unless REP_NO_ENTRYDN and REP_NO_SUBSCHEMA are * set. Called from backend_operational(). */ BI_op_func *bi_operational; /* be->be_operational() */ /* * Called before LDAP operation handlers. * May send and return a referral if the operation should do so. * The existence of this function makes LDAP operations non-atomic. * * It should send and return LDAP_REFERRAL, or return but NOT send * another result code. With LDAP_SUCCESS, slapd will proceed to * the operation handler. With other result codes, slapd will put * them in rs->sr_err and send the error. * * Called from backend_check_referrals(). */ BI_op_func *bi_chk_referrals; /* be->be_chk_referrals() */
I think this handler needs to go away. As noted, it makes operations non-atomic, and at least in the case of back-bdb it gets checked redundantly.
/* * bi->bi_chk_controls() aka be->be_chk_controls(): * May react to controls in op->o_ctrlflag from the LDAP operation, * before slapd checks bi->bi_controls and globally handled controls. * Called before LDAP operation handlers. * * Do not send results. * Return SLAP_CB_CONTINUE for slapd to check other supported controls. * Return LDAP_SUCCESS to proceed to the operation without checking * other controls. Otherwise return an error code which slapd will * send to fail the operation. * * Called from backend_check_restrictions(). * * See overlays/pcache.c for an example. * Presumably it returns LDAP_SUCCESS, it must first do the work * normally done by backend_check_controls(). */ BI_op_func *bi_chk_controls; /* be->be_chk_controls() */ /* * bi->bi_entry_get_rw() aka be->be_fetch(): * * Fetch an entry for operation 'op' with normalized DN 'ndn', for * read operations if 'rw' == 0, for write operations if 'rw' == 1. * If 'oc' and/or 'at' are non-NULL but the entry does not contain * them, the function may fail with LDAP_NO_SUCH_ATTRIBUTE. * * Return an LDAP result code. *e is NULL on entry, and must be * non-NULL on exit if and only if LDAP_SUCCESS is returned. I think. * * Slapd returns the entry to the backend with bi_entry_release_rw() * if that is set, otherwise hopefully frees it with entry_free(). * ? [See below.] * * Called from be_entry_get_rw(). * * ? [What can the caller and the backend do with the entry before * it is released? Can it be modified? No current callers * use rw!=0, can it be modified in that case? ] * * ? [Callers of entry_release seem to choose rw based on which * LDAP operation is being used, which maybe means that * be_fetch() callers should do the same - i.e. set rw for * the Add operation even for entries which the caller will * only read. Need to check. Except, do any callers use * * rw != 0 anyway? ]
For backends that use an entry cache, the returned entry is the one held in the cache. The idea is that you should be able to call with rw = 1 to cause the cached entry to be writelocked until you call entry_release. In practice, no code thus far has needed to modify the entry that is being fetched. In general, it would be a bad idea anyway, since index updates may also be needed as a consequence of the modification. So, generally, the be_modify function is used if you actually need to modify an entry, and this function ends up only being called for read accesses.
*/ int (*bi_entry_get_rw)( Operation *op, struct berval *ndn, ObjectClass *oc, AttributeDescription *at, int rw, Entry **e ); /* * bi->bi_entry_release_rw() aka be->be_release(): * * Returns an entry to the backend, possibly fetched with be_fetch. * * Slapd will also call it with entries that did not originate * with the backend, I do not know if that is intentional. * ? [After some overlay did entry_dup(), maybe. Must check.] * * Called from be_entry_release_rw(), which frees the entry with * entry_free() if there is no be_entry_release_rw function. * ? [However frontendDB->be_release() also calls this function, * but does not free the entry if no function is found. Then * it just returns LDAP_NO_SUCH_OBJECT.] * * Return an LDAP result code, which is hardly ever used. * Only SLAP_CB_CONTINUE seems to matter: If there are overlays, * this means other bi_entry_release_rw() functions are called, * or entry_free() if all else fails. * * ? [Maybe back-relay has a memory leak and should call entry_free * if there is no backend function. frontendDB does not, overlay * translucent does, backglue does but has a FIXME comment about * it. Fix what?] */ int (*bi_entry_release_rw)( Operation *op, Entry *e, int rw );
Hm, yeah, it should always fallback to entry_free if nothing else.
/* * bi->bi_has_subordinates() aka be->be_has_subordinates(): * * Generate the value for compare(entry, "hasSubordinates=TRUE"). * Returns an LDAP result code. When it returns LDAP_SUCCESS, it * must set *hasSubs to LDAP_COMPARE_TRUE or LDAP_COMPARE_FALSE. * * This function really should succeed except on internal errors: * test_filter() with the presence filter (hasSubordinates=*) does * not call this function, it assumes attribute hasSubordinates will * exist if this function does. And for equality match it turns * other result codes than LDAP_SUCCESS into LDAP_OTHER. (Though it * can also be used for the Compare operation, in that case a result * code of LDAP_NO_SUCH_ATTRIBUTE also makes sense.) * * back-relay sets this function, which is wrong when relaying to a * backend which does not. OTOH slapi_entry_has_children() exists * even though slapi does not set bi_has_subordinates. * * bi->bi_entry_get_rw must exist if this function does; Compare * will call it if !defined(SLAP_COMPARE_IN_FRONTEND). */ int (*bi_has_subordinates)( Operation *op, Entry *e, int *hasSubs ); ...; /* * Connection management, called when a connection is opened or closed. * Return an LDAP result code. * * Overlays must normally return SLAP_CB_CONTINUE, so the backend * also gets a chance to prepare for and clean up after connections. * * Slapd discards other result codes, which is just as well since the * overlay code turns SLAP_CB_CONTINUE into LDAP_UNWILLING_TO_PERFORM * when the backend has no connection handler. * * Slapi can set up a fake connection and call backend_connection_init, * but I see no matching call to backend_connection_destroy. Maybe * the connection is given to slapd which destroys it? */ typedef int BI_conn_func( BackendDB *bd, Connection *c ); BI_conn_func *bi_connection_init; /* be->be_connection_init */ BI_conn_func *bi_connection_destroy; /* be->be_connection_destroy */ /* Hooks for slap tools: bi->bi_tool_<act> aka be->be_<act> */ int (*bi_tool_entry_open)( BackendDB *be, int mode ); int (*bi_tool_entry_close)( BackendDB *be ); ...;
};
Howard Chu writes:
Hallvard B Furuseth wrote:
- bind
- If returning LDAP_SUCCESS, do not send it first - the frontend will.
- Do send non-success responses before returning.
This inconsistency has always bothered me, and with the current callback layering there's no longer any reason for it. We should change Bind to behave like all the other ops and just let backends send responses for either success or failure.
For the transition, we can set a "response has been sent"-flag in the SlapReply or Operation, so the frontend will know whether to send.
- bi->bi_entry_release_rw() aka be->be_release():
- (...)
- ? [However frontendDB->be_release() also calls this function,
- but does not free the entry if no function is found. Then
- it just returns LDAP_NO_SUCH_OBJECT.]
- (...)
- ? [Maybe back-relay has a memory leak and should call entry_free
- if there is no backend function. frontendDB does not, overlay
- translucent does, backglue does but has a FIXME comment about
- it. Fix what?]
Hm, yeah, it should always fallback to entry_free if nothing else.
See ITS#5340 "REP_ENTRY_MODIFIABLE bug in dynlist" first. (Poorly named ITS now that I think of it, it seems a more general problem.)
<quote who="Aaron Richton">
[purposefully on -devel for discussion]
Any opinions on:
--- init.c~ 2008-03-12 10:02:44.000000000 -0400 +++ init.c 2008-03-12 10:03:16.000000000 -0400 @@ -712,7 +712,7 @@ return -1; }
Debug( LDAP_DEBUG_TRACE, LDAP_XSTRING(bdb_back_initialize)
Debug( LDAP_DEBUG_ANY, LDAP_XSTRING(bdb_back_initialize) ": %s\n", version, 0, 0 ); }
I'm definitely one for 'quiet' software, but knowing the version of libdb is a really nice detail when assisting users of OpenLDAP. This would make it that much easier to have them find it. I'd argue there's a bit of precedent:
main.c: Debug( LDAP_DEBUG_ANY, "%s", Versionstr, 0, 0 );
The negative, of course, is the noise. But what's ~60 bytes in this day and age?
Looks good:
[ghenry@suretec openldap-2.4.8]$ sudo /usr/local/libexec/slapd -d 256 -h ldap://:6001/ @(#) $OpenLDAP: slapd 2.4.8 (Mar 12 2008 14:47:23) $ ghenry@suretec:/home/ghenry/Download/openldap-2.4.8/servers/slapd bdb_back_initialize: Berkeley DB 4.6.21: (October 11, 2007) hdb_back_initialize: Berkeley DB 4.6.21: (October 11, 2007) bdb_monitor_db_open: monitoring disabled; configure monitor database to enable slapd starting