[SSSD] [PATCH] USe sysdb_serch_service() in sss_cacahe

Simo Sorce simo at redhat.com
Thu Jan 10 19:10:46 UTC 2013


On Thu, 2013-01-10 at 19:44 +0100, Michal Židek wrote:
> On 01/10/2013 06:04 PM, Simo Sorce wrote:
> > On Thu, 2013-01-10 at 17:28 +0100, Jakub Hrozek wrote:
> >> On Thu, Jan 10, 2013 at 11:13:05AM -0500, Simo Sorce wrote:
> >>> On Thu, 2013-01-10 at 11:06 -0500, Simo Sorce wrote:
> >>>> On Thu, 2013-01-10 at 11:22 +0100, Pavel Březina wrote:
> >>>>> On 01/08/2013 05:10 AM, Simo Sorce wrote:
> >>>>>> In the patches that fix the sdap_reinit functions I introduced a new
> >>>>>> service search helper sysdb_search_services()
> >>>>>>
> >>>>>> Use this helper in sss_cache instead of a home grown one with poor-man
> >>>>>> filter parser.
> >>>>>>
> >>>>>> Simo.
> >>>>>
> >>>>> Hi,
> >>>>> I don't think these functions are identical.
> >>>>>
> >>>>> -    if (strcmp(sub_filter, "*") == 0) {
> >>>>> -        /* All services */
> >>>>> -        ret = sysdb_enumservent(mem_ctx, sysdb, &res);
> >>>>> -    } else {
> >>>>> -        /* Get service by name */
> >>>>> -        ret = sysdb_getservbyname(mem_ctx, sysdb, sub_filter,
> >>>>> -                                  NULL, &res);
> >>>>> -    }
> >>>>>
> >>>>> The sub_filter is not a valid ldb filter, but '*' or name.
> >>>>
> >>>> Ahh true, I'll change this to be a minimal wrapper then.
> >>>>
> >>>> Thanks for catching it.
> >>>
> >>> Actually looking at is it would be more consistent to change the filter
> >>> in update_all_filters, however I noticed that all these filters only to
> >>> SYSDB_NAME=<value> and none take in account aliases, while the
> >>> getsrvbyname() function also takes in account aliases. Should the
> >>> update_all_filters also take in account aliases for other filters like
> >>> update_user_filter ?
> >>> If so I can simply change update_filter for all uniformly.
> >>>
> >>
> >> I think it would be a good idea.
> >>
> >>> Simo.
> >>>
> >>>> Btw why sss_cache is not tested in make check ?
> >>>>
> >>
> >> None of the tools is. It's just noone wrote the tests upstream, we're
> >> relying on Red Hat QE here.
> >
> > Ok I opened tickets both for alias support and tests, and another bug I
> > found.
> > I didn't do alias support because the additional bug may mean additional
> > changes are needed to that part of the code anyway, so I'll leave it to
> > someone to revisit exactly what the inputs/outputs and behavior need to
> > be before diving in the code.
> >
> > Attached find respun patch that instead makes the code generate a proper
> > filter for services too so that using sysdb_serach_service() is ok.
> > The only thing it really changes is that it drops support for searching
> > service aliases, but I think that is fine as alias support is
> > inconsistent throughout the tool and a decision will be made in the
> > ticket I opened, so by dropping alias support for services we are now
> > more consistent even if we later decide we need to readd alias support
> > to all calls that support aliases.
> >
> > Simo.
> >
> >
> 
> As you are already touching the update filters part, could you change 
> the fmt parameter to update_filter in (in autofs part) from 
> "(&(objectclass=%s)(%s=%s))"
> 
>   to
> 
> "(&(objectclass="SYSDB_AUTOFS_MAP_OC")(%s=%s))"
> 
> That should also fix https://fedorahosted.org/sssd/ticket/1754.

Ok.

rebased to add this fix.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Use-new-sysdb_search_service-in-sss_cache.patch
Type: text/x-patch
Size: 3452 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130110/6dbd4893/attachment.bin>


More information about the sssd-devel mailing list