On Fri, 2013-01-11 at 08:54 -0500, Simo Sorce wrote: > On Thu, 2013-01-10 at 20:34 +0100, Jakub Hrozek wrote: > > On Thu, Jan 10, 2013 at 08:29:36PM +0100, Jakub Hrozek wrote: > > > On Thu, Jan 10, 2013 at 12:39:53PM -0500, Simo Sorce wrote: > > > > On Tue, 2013-01-08 at 23:15 -0500, Simo Sorce wrote: > > > > > Attached find a somewhat big patchset composed of 59 patches. > > > > > These patches start to address #1747 by changing a fundamental issue > > > > > in > > > > > the current code where the sysdb_ctx structure includes a reference > > > > > to a > > > > > sss_domain_info one. > > > > > > > > > > The issue is that we now have multiple named domain in a single sysdb > > > > > database so a univocal sysdb->domain relationship doesn't represent > > > > > reality anymore. > > > > > > > > > > So far we have coped by somewhat faking up sysdb contexts for domains > > > > > but this is all backwards and makes it hard to predict if code is > > > > > behaving correctly. > > > > > > > > > > Although this patchset is huge the bulk is just a series of > > > > > painstaking > > > > > patches to change a ton of interfaces to take a sss_domain_info > > > > > structure in input so we can avoid ambiguity about which domain the > > > > > sysdb is asked to deal with. > > > > > > > > > > As mentioned this is just an 'initial' patchset, there are still other > > > > > aspects to handle as part o #1747 but this patchset is self-contained > > > > > and big enough it would only make it harder to get in if we waited for > > > > > more changes to pile on top. > > > > > > > > > > The patchset is attached in tar.gz format (sorry but unpacked is too > > > > > big, ~650K) > > > > > > > > > > I will keep rebasing this[0] branch on top of master with this same > > > > > patchset, and I recommend for your sanity to pull from there rather > > > > > than > > > > > try to apply 59 patches for patchfiles. The reason is that this > > > > > patchset > > > > > touches a lot of code and any new commit in master is almost certainly > > > > > going to require a rebase to this patchset. > > > > > > > > > > Keep in mind that the general principle in building this patchset was > > > > > to > > > > > try to have the least impact on the code, not necessarily to beautify > > > > > the code I was going to touch, this in order to make it very clear > > > > > what > > > > > is changed and ckeep churn to a bare minimum, so I haven't changed > > > > > things like DEBUG levels to avoid any noise and additional great pain > > > > > in > > > > > the inevitable rebases that are to come. > > > > > > > > > > I did test the code on my install with multiple trusts, and it seem to > > > > > be working correctly with normal and subdomain users. > > > > > > > > > > Happy reviewing :) > > > > > > > > > > Simo. > > > > > > > > > > [0] > > > > > http://fedorapeople.org/cgit/simo/public_git/sssd.git/log/?h=sysdb_refactor > > > > > > > > > > Note: The git tree has actually 63 patchse on top of master, the > > > > > first 4 > > > > > are pre-requisite patches I already sent to the list for review > > > > > separately. > > > > > > > > FYI: I rebased my public git tree against the current master as the > > > > latest patches that went in caused conflicts. > > > > > > That's fine, I started reviewing the patches from your git tree. I still > > > think it was a good idea to send the patch to the ML as the archive will > > > keep them forever :-) > > > > > > I'll make comments as replies to this thread. > > > > Probably needs another rebase after yesterday's AD patches: > > CC src/providers/ldap/sdap_async_services.lo > > src/providers/ldap/sdap_async_groups.c: In function > > 'sdap_get_members_with_primary_gid': > > src/providers/ldap/sdap_async_groups.c:117:30: warning: passing argument 3 > > of 'sysdb_search_users' from incompatible pointer type [enabled by default] > > In file included from src/providers/ldap/sdap_async_groups.c:25:0: > > ./src/db/sysdb.h:746:5: note: expected 'struct sss_domain_info *' but > > argument is of type 'char *' > > src/providers/ldap/sdap_async_groups.c:117:30: warning: passing argument 4 > > of 'sysdb_search_users' from incompatible pointer type [enabled by default] > > In file included from src/providers/ldap/sdap_async_groups.c:25:0: > > ./src/db/sysdb.h:746:5: note: expected 'const char *' but argument is of > > type 'const char **' > > src/providers/ldap/sdap_async_groups.c:117:30: warning: passing argument 5 > > of 'sysdb_search_users' from incompatible pointer type [enabled by default] > > In file included from src/providers/ldap/sdap_async_groups.c:25:0: > > ./src/db/sysdb.h:746:5: note: expected 'const char **' but argument is of > > type 'size_t *' > > src/providers/ldap/sdap_async_groups.c:117:30: warning: passing argument 6 > > of 'sysdb_search_users' from incompatible pointer type [enabled by default] > > In file included from src/providers/ldap/sdap_async_groups.c:25:0: > > ./src/db/sysdb.h:746:5: note: expected 'size_t *' but argument is of type > > 'struct ldb_message ***' > > src/providers/ldap/sdap_async_groups.c:117:30: error: too few arguments to > > function 'sysdb_search_users' > > In file included from src/providers/ldap/sdap_async_groups.c:25:0: > > ./src/db/sysdb.h:746:5: note: declared here > > > > To spare you from further headaches, I think we need to review this patchset > > before doing any more changes. > > Ah may bad, I had found this yesterday on the bus to the doctor, but > when I came back home forgot to push, I am going to make all rebases > requested today asap and then push all on branches.
New rebase pushed -- Simo Sorce * Red Hat, Inc * New York _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel