On Fri, 2013-01-04 at 16:15 +0100, Jakub Hrozek wrote:
On Fri, Jan 04, 2013 at 09:13:04AM -0500, Simo Sorce wrote:
> On Thu, 2012-12-20 at 08:28 -0500, Simo Sorce wrote:
> > On Thu, 2012-12-20 at 14:05 +0100, Ondrej Kos wrote:
> > >
> > >
> > > Here are two patches for the SYSDB:
> > >
> > >
> > > [PATCH 1/2] SYSDB: replace ghost users properly
> > >
> > > this fixes
https://fedorahosted.org/sssd/ticket/1714
> >
> > It seem that here you are basically scanning the whole attribute.
> > Instead of making potentially multiple deletes then wouldn't it be
> > easier to recompute the attribute and do a single replace omitting the
> > names you do not want in there?
> > I think you can significantly reduce the amount of linear searches as
> > the current patch introduces O(n^2) like behavior (nested linear loops).
>
> Ok scrap this suggestion, I did not understand the patch and Ondrej
> explained it to me in private.
>
> However now that I understood what problem we are trying to solve here I
> am afraid this work can be simply doen with adding a control named
> LDB_CONTROL_PERMISSIVE_MODIFY_OID available in ldb by default.
>
> It will cause a deletion of a missing value to not return errors.
>
You mean something like Samba had in dsdb_modify_permissive()?
In pseudocode it said:
int modify_permissive(ldb, msg)
{
ldb_request req;
ldb_build_mod_req(&req, ldb, msg);
ldb_request_add_control(req, LDB_CONTROL_PERMISSIVE_MODIFY_OID, false, NULL);
ret = ldb_request(ldb, req);
if (ret == LDB_SUCCESS) {
ret = ldb_wait(req->handle, LDB_WAIT_ALL);
}
}
Yes, I asked Ondrej in fact to build a sss_permissive_modify() function.
This is a perfect template.
I'm a little concerned that this approach would hide genuine
logic bugs,
though..but for a low level function such as sysdb_add_user that is
called a lot, this might be OK for performance reasons..
I think the compromise is justified, we also do create users in steps so
this modify would affect only a subset of attributes.
Simo.
--
Simo Sorce * Red Hat, Inc * New York