Re: (ITS#5487) syncprov_findbase must search the backend from the syncrepl search

[hyc@symas.com]

rein@OpenLDAP.org wrote:
> Full_Name: Rein Tollevik
> Version: CVS head
> OS:
> URL: ftp://ftp.openldap.org/incoming/
> Submission from: (NULL) (81.93.160.250)
> Submitted by: rein
>
>
> syncprov_findbase() must search the backend saved with the syncrepl operation,
> not the one from the operation passed as argument.  The backend in the op
> argument can be a subordinate database, in which case the search for the base in
> the superior database will fail, and syncrepl consumers will be force to do a an
> unneccessary full refresh of the database.

OK.

>  The patch at the end should fix
> this.  Note that both fop.o_bd and fop.o_bd->bd_info can be changed by the
> overlay_op_walk() call, which is the reason for the long pointer traversal to
> find the correct bd_info to save and restore.

But the overlay_op_walk call is only appropriate when the DB to be searched is 
the current database, and the current DB is an overlay DB structure.

Your patch causes fc->fss->s_op->o_bd's bd_info pointer to change, which is 
not allowed. That's in the original backendDB, which must be treated as 
read-only since multiple threads may be accessing it. The correct approach 
here is to use a new local backendDB variable, copy the s_op->o_bd into it, 
and then just do a regular be_search invocation instead of using overlay_op_walk.

But, this patch must not take effect on the first call to syncprov_findbase 
(which occurred in syncprov_op_search) - in that case, the current code is 
correct. So, you need to tweak things based on whether (s_flags & 
PS_IS_REFRESHING) is true or not - if true, this is the first search, and it 
should use the original code. Else, it must use be_search.

> Rein Tollevik
> Basefarm AS
>
> diff -u OpenLDAP/servers/slapd/overlays/syncprov.c:1.1.1.18
> OpenLDAP/servers/slapd/overlays/syncprov.c:1.19
> --- OpenLDAP/servers/slapd/overlays/syncprov.c:1.1.1.18	Wed Apr 30 13:17:58
> 2008
> +++ OpenLDAP/servers/slapd/overlays/syncprov.c	Wed Apr 30 19:34:00 2008
> @@ -404,7 +404,7 @@
>   		slap_callback cb = {0};
>   		Operation fop;
>   		SlapReply frs = { REP_RESULT };
> -		BackendInfo *bi;
> +		BackendInfo *bi = fc->fss->s_op->o_bd->bd_info;
>   		int rc;
>
>   		fc->fss->s_flags ^= PS_FIND_BASE;
> @@ -413,10 +413,8 @@
>   		fop = *fc->fss->s_op;
>
>   		fop.o_hdr = op->o_hdr;
> -		fop.o_bd = op->o_bd;
>   		fop.o_time = op->o_time;
>   		fop.o_tincr = op->o_tincr;
> -		bi = op->o_bd->bd_info;
>
>   		cb.sc_response = findbase_cb;
>   		cb.sc_private = fc;
> @@ -435,7 +433,7 @@
>   		fop.ors_filterstr = generic_filterstr;
>
>   		rc = overlay_op_walk(&fop,&frs, op_search, on->on_info, on );
> -		op->o_bd->bd_info = bi;
> +		fc->fss->s_op->o_bd->bd_info = bi;
>   	} else {
>   		ldap_pvt_thread_mutex_unlock(&fc->fss->s_mutex );
>   		fc->fbase = 1;
>
>
>


-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/