From e87f58129506ac92425b0240a7863e680dcd56e4 Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: Feb 08 2011 01:24:53 +0000 Subject: Bug 675265 - preventryusn gets added to entries on a failed delete https://bugzilla.redhat.com/show_bug.cgi?id=675265 Description: When an entry is deleted with Entry USN plugin enabled, an operational attribute preventryusn is added to handle indexes and entryusn tombstone. The attribute must have been added only when the delete was successful, but it was added regardless of the result from the operation. This patch checks the delete result in the newly added entryusn delete bepost plugin (usn_bepostop_delete). If it is not successful, the bepost plugin cleans up the attribute. --- diff --git a/ldap/servers/plugins/usn/usn.c b/ldap/servers/plugins/usn/usn.c index 4ad9e66..792ea47 100644 --- a/ldap/servers/plugins/usn/usn.c +++ b/ldap/servers/plugins/usn/usn.c @@ -59,6 +59,7 @@ static int usn_bepreop_add(Slapi_PBlock *pb); static int usn_bepreop_delete(Slapi_PBlock *pb); static int usn_bepreop_modify(Slapi_PBlock *pb); static int usn_bepostop(Slapi_PBlock *pb); +static int usn_bepostop_delete (Slapi_PBlock *pb); static int usn_start(Slapi_PBlock *pb); static int usn_close(Slapi_PBlock *pb); static int usn_get_attr(Slapi_PBlock *pb, const char* type, void *value); @@ -177,7 +178,7 @@ usn_bepostop_init(Slapi_PBlock *pb) if (slapi_pblock_set(pb, SLAPI_PLUGIN_BE_POST_ADD_FN, (void *)usn_bepostop) != 0 || slapi_pblock_set(pb, SLAPI_PLUGIN_BE_POST_DELETE_FN, - (void *)usn_bepostop) != 0 || + (void *)usn_bepostop_delete) != 0 || slapi_pblock_set(pb, SLAPI_PLUGIN_BE_POST_MODIFY_FN, (void *)usn_bepostop) != 0 || slapi_pblock_set(pb, SLAPI_PLUGIN_BE_POST_MODRDN_FN, @@ -516,6 +517,47 @@ bail: return rc; } +/* count up the counter */ +/* if the op is delete and the op was not successful, remove preventryusn */ +static int +usn_bepostop_delete (Slapi_PBlock *pb) +{ + int rc = -1; + Slapi_Backend *be = NULL; + + slapi_log_error(SLAPI_LOG_TRACE, USN_PLUGIN_SUBSYSTEM, + "--> usn_bepostop\n"); + + /* if op is not successful, don't increment the counter */ + slapi_pblock_get(pb, SLAPI_RESULT_CODE, &rc); + if (LDAP_SUCCESS != rc) { + Slapi_Entry *e = NULL; + + slapi_pblock_get(pb, SLAPI_DELETE_BEPOSTOP_ENTRY, &e); + if (NULL == e) { + rc = LDAP_NO_SUCH_OBJECT; + goto bail; + } + /* okay to return the rc from slapi_entry_delete_values */ + rc = slapi_entry_delete_values(e, SLAPI_ATTR_ENTRYUSN_PREV, NULL); + goto bail; + } + + slapi_pblock_get(pb, SLAPI_BACKEND, &be); + if (NULL == be) { + rc = LDAP_PARAM_ERROR; + goto bail; + } + + if (be->be_usn_counter) { + slapi_counter_increment(be->be_usn_counter); + } +bail: + slapi_log_error(SLAPI_LOG_TRACE, USN_PLUGIN_SUBSYSTEM, + "<-- usn_bepostop\n"); + return rc; +} + /* mimic replication to turn on create_tombstone_entry */ static int usn_get_attr(Slapi_PBlock *pb, const char* type, void *value) diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c index f2454be..dc759b4 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c @@ -116,6 +116,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) if (NULL == addr) { + /* retval is -1 */ goto error_return; } ldap_result_code = slapi_dn_syntax_check(pb, addr->dn, 1); @@ -123,6 +124,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) { ldap_result_code = LDAP_INVALID_DN_SYNTAX; slapi_pblock_get(pb, SLAPI_PB_RESULT_TEXT, &ldap_result_message); + /* retval is -1 */ goto error_return; } @@ -158,19 +160,22 @@ ldbm_back_delete( Slapi_PBlock *pb ) operation->o_status = SLAPI_OP_STATUS_WILL_COMPLETE; } if ( slapi_op_abandoned( pb ) ) { + /* retval is -1 */ goto error_return; } /* find and lock the entry we are about to modify */ if ( (e = find_entry2modify( pb, be, addr, NULL )) == NULL ) { - ldap_result_code= -1; + ldap_result_code= LDAP_NO_SUCH_OBJECT; + /* retval is -1 */ goto error_return; /* error result sent by find_entry2modify() */ } if ( slapi_entry_has_children( e->ep_entry ) ) { ldap_result_code= LDAP_NOT_ALLOWED_ON_NONLEAF; + /* retval is -1 */ goto error_return; } @@ -194,6 +199,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) { /* restore original entry so the front-end delete code can free it */ slapi_pblock_set( pb, SLAPI_DELETE_BEPREOP_ENTRY, orig_entry ); + /* retval is -1 */ goto error_return; } slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code); @@ -211,6 +217,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) slapi_pblock_get(pb, SLAPI_RESULT_CODE, &ldap_result_code); /* restore original entry so the front-end delete code can free it */ slapi_pblock_set( pb, SLAPI_DELETE_BEPREOP_ENTRY, orig_entry ); + /* retval is -1 */ goto error_return; } /* the flag could be set in a preop plugin (e.g., USN) */ @@ -294,6 +301,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) if ( ldap_result_code != LDAP_SUCCESS ) { ldap_result_message= errbuf; + /* retval is -1 */ goto error_return; } @@ -766,8 +774,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) } if (entryrdn_get_switch()) /* subtree-rename: on */ { - retval = - entryrdn_index_entry(be, e, BE_INDEX_DEL, &txn); + retval = entryrdn_index_entry(be, e, BE_INDEX_DEL, &txn); if (DB_LOCK_DEADLOCK == retval) { LDAPDebug0Args( LDAP_DEBUG_ARGS, "delete (deleting entryrdn) DB_LOCK_DEADLOCK\n"); @@ -852,6 +859,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) /* Failed */ LDAPDebug( LDAP_DEBUG_ANY, "Retry count exceeded in delete\n", 0, 0, 0 ); ldap_result_code= LDAP_OPERATIONS_ERROR; + retval = -1; goto error_return; } @@ -885,6 +893,7 @@ ldbm_back_delete( Slapi_PBlock *pb ) ldap_result_code= LDAP_OPERATIONS_ERROR; LDAPDebug( LDAP_DEBUG_ANY, "ldbm_back_delete: modify_switch_entries failed\n", 0, 0, 0); + retval = -1; goto error_return; } } @@ -893,10 +902,6 @@ ldbm_back_delete( Slapi_PBlock *pb ) goto common_return; error_return: - if (e!=NULL) { - cache_unlock_entry( &inst->inst_cache, e ); - CACHE_RETURN( &inst->inst_cache, &e ); - } if (tombstone_in_cache) { CACHE_REMOVE( &inst->inst_cache, tombstone ); @@ -941,9 +946,27 @@ common_return: * but not if the operation is purging tombstones. */ if (!delete_tombstone_entry) { + if (e) { + /* set entry in case be-postop plugins need to work on it + * (e.g., USN) */ + slapi_pblock_get( pb, SLAPI_DELETE_BEPOSTOP_ENTRY, &orig_entry ); + slapi_pblock_set( pb, SLAPI_DELETE_BEPOSTOP_ENTRY, e->ep_entry ); + } plugin_call_plugins (pb, SLAPI_PLUGIN_BE_POST_DELETE_FN); + /* set original entry back */ + if (e) { + slapi_pblock_set( pb, SLAPI_DELETE_BEPOSTOP_ENTRY, orig_entry ); + } } + /* Need to return to cache after post op plugins are called */ + if (retval) { /* error case */ + if (e) { + cache_unlock_entry( &inst->inst_cache, e ); + CACHE_RETURN( &inst->inst_cache, &e ); + } + } + if (ruv_c_init) { modify_term(&ruv_c, be); } diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h index 81db71f..dd601ca 100644 --- a/ldap/servers/slapd/slapi-plugin.h +++ b/ldap/servers/slapd/slapi-plugin.h @@ -5931,6 +5931,7 @@ typedef struct slapi_plugindesc { #define SLAPI_DELETE_EXISTING_ENTRY SLAPI_ADD_EXISTING_DN_ENTRY #define SLAPI_DELETE_GLUE_PARENT_ENTRY SLAPI_ADD_PARENT_ENTRY #define SLAPI_DELETE_BEPREOP_ENTRY SLAPI_ENTRY_PRE_OP +#define SLAPI_DELETE_BEPOSTOP_ENTRY SLAPI_ENTRY_POST_OP /* modify arguments */ #define SLAPI_MODIFY_TARGET SLAPI_TARGET_DN