[PATCH] Use idmap cache for all pdb backends.

[Alexander Bokovoy]

Hi,


On Fri, May 2, 2014 at 4:38 PM, Alexander Werth
<werth at linux.vnet.ibm.com> wrote:
> On Wed, 2014-04-30 at 15:46 -0700, Christof Schmitt wrote:
>> On Wed, Apr 30, 2014 at 04:15:38PM +0200, Alexander Werth wrote:
>> > On Wed, 2014-04-30 at 15:51 +0200, Volker Lendecke wrote:
>> > > On Wed, Apr 30, 2014 at 03:16:45PM +0200, Alexander Werth wrote:
>> > > > Currently only the passdb ldapsam backend uses the idmap cache.
>> > > > And even that only uses the cache when the option ldapsam:trusted is set.
>> > > > By moving the set cache entry calls up one layer all passdb backends
>> > > > will use the idmap cache.
>> > > > Without the idmap cache any id<->sid mapping request from ACLs for
>> > > > example would result in another set of ldap queries. Up to the point
>> > > > where the LDAP would be overloaded.
>> > >
>> > > Looks good to me, thanks. When compiling I now get
>> > >
>> > > [2299/4156] Compiling source3/passdb/pdb_ldap.c
>> > > ../source3/passdb/pdb_ldap.c: In function ‘ldapsam_uid_to_sid’:
>> > > ../source3/passdb/pdb_ldap.c:5014:16: warning: variable ‘id’ set but not used [-Wunused-but-set-variable]
>> > > ../source3/passdb/pdb_ldap.c: In function ‘ldapsam_gid_to_sid’:
>> > > ../source3/passdb/pdb_ldap.c:5087:16: warning: variable ‘id’ set but not used [-Wunused-but-set-variable]
>> > >
>> > > It looks that if the "id" variable can be removed in both
>> > > cases. Do you want me to do this, or do you want to send a
>> > > new patch?
>> >
>> > Thanks, for the quick review.
>> > And I'm adding a mental note to check for compiler warnings.
>> > I've removed the unused variable and updated the patch.
>> >
>> > Cheers,
>> > Alexander
>>
>> > >From ad56d1e18450237b906f41179f13ca42fd8d1aad Mon Sep 17 00:00:00 2001
>> > From: Alexander Werth <alexander.werth at de.ibm.com>
>> > Date: Fri, 25 Apr 2014 13:53:48 +0200
>> > Subject: [PATCH] s3: Always cache idmapping results of pdb backend.
>> >
>> > And don't cache in the pdb_ldap module on the id_to_sid calls.
>> >
>> > Signed-off-by: Alexander Werth <alexander.werth at de.ibm.com>
>> > ---
>> >  source3/passdb/pdb_interface.c | 19 ++++++++++++++++++-
>> >  source3/passdb/pdb_ldap.c      | 14 --------------
>> >  2 files changed, 18 insertions(+), 15 deletions(-)
>>
>> Why are you removing the caching in pdb_ldap? As far as i can see,
>> ldapsam_sid_to_id, ldapsam_uid_to_sid and ldapsam_gid_to_sid do not call
>> the default methods, so the cache calls should probably be kept in the
>> ldapsam functions.
>
> Right, I meant to move that call up one level but didn't.
> What about this version of the patch?
> I've added the cache call right into pdb_sid_to_id, pdb_uid_to_sid, and
> pdb_gid_to_sid which are calling the backend specific methods.
Looks good now.
You can add
Reviewed-by: Alexander Bokovoy <ab at samba.org>

-- 
/ Alexander Bokovoy