On Mon, May 26, 2014 at 10:33:22PM +0200, Pavel Březina wrote: > On 05/26/2014 05:45 PM, Jakub Hrozek wrote: > >On Thu, May 22, 2014 at 07:57:28PM +0200, Pavel Březina wrote: > >>On 05/21/2014 11:03 PM, Jakub Hrozek wrote: > >>>Hi, > >>> > >>>the attaches two patches implement DBus methods to retrieve SSSD domains > >>>using the system bus as well as two example getters that leverage the > >>>infrastructure to Get and GetAll the object properties. > >>> > >>>Pavel, I know you have a large patchset in a private tree that > >>>implements the other getters and moves the domain methods into a > >>>separate file. I'm fine with merging your and mine patches, I just > >>>wanted to get the ball rolling on the list. Feel free to reply with your > >>>patchset. > >> > >>Hi, > >>see the attachment. > >> > >>I squash all my patches to these five - mainly discarded the moving > >>the code around. > >> > >>I kept you as the author for the methods as I made only very minor > >>changes there. However, I took the ownership for the properties > >>since there only very little of your original code remains there - I > >>hope you don't mind. > >> > >>The next three patches implements the SSSD components interface > >>required for the OpenLMI provider. I'm sending it to this thread > >>since it depends on the domain patches anyway - there is enough > >>dependency fuzz already. > >> > > > >> From 7e55b23741ed749e95e9d33184b0fd702b86bb46 Mon Sep 17 00:00:00 2001 > >>From: Jakub Hrozek <[email protected]> > >>Date: Tue, 22 Apr 2014 15:38:08 +0200 > >>Subject: [PATCH 1/5] IFP: Add ListDomains and FindDomainByName > > > >This one was acked by Pavel separately. > > > > Hi, > thank you for the review. I'm sending new patches. > > >>+ > >>+ /* TODO: replace hardcoded values with option names from the provider > >>*/ > > > >I don't like this at all, but I can't see another way with the current > > Me neither, thus todo. > > >code. Maybe we should add a new ID target for getting info about > >domains? I think the performance would be a non-issue since all that > >would be needed is an sbus round trip to the DP that would just reply > >right away. But any DP changes should be discussed on the list first -- > >for now, just please file a ticket. > > My first idea was to do it as a part of future failover refactoring > (to make the failover context per domain, not per backend). The idea > was to provide a dbus interface for a failover that would retrieve a > list of current servers, also with their status etc. > > >>+ if (strcasecmp(dom->provider, "ldap") == 0) { > >>+ option = backup == false ? "ldap_uri" : "ldap_backup_uri"; > >>+ } else if (strcasecmp(dom->provider, "ipa") == 0) { > >>+ option = backup == false ? "ipa_server" : "ipa_backup_server"; > >>+ } else if (strcasecmp(dom->provider, "ad") == 0) { > >>+ option = backup == false ? "ad_server" : "ad_backup_server"; > >>+ } else { > >>+ ret = EINVAL; > >>+ goto done; > >>+ } > > >
> From 10e4b069bec51dab06be674cec4e881adda7bb4c Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]> > Date: Thu, 22 May 2014 19:01:35 +0200 > Subject: [PATCH 1/4] IFP: Implement domain getters > > --- > src/responder/ifp/ifp_domains.c | 307 +++++++++++++++++++++++ > src/responder/ifp/ifp_domains.h | 60 +++++ > src/responder/ifp/ifp_iface.xml | 19 ++ > src/responder/ifp/ifp_iface_generated.c | 415 > ++++++++++++++++++++++++++++++++ > src/responder/ifp/ifp_iface_generated.h | 39 +++ > src/responder/ifp/ifpsrv.c | 19 ++ > 6 files changed, 859 insertions(+) > > diff --git a/src/responder/ifp/ifp_domains.c b/src/responder/ifp/ifp_domains.c > index > a06ba471c43ac7111693aa0d22a14cf33d168caa..5b34f96e9e6c4b45a1d775818fe15b328d335f18 > 100644 > --- a/src/responder/ifp/ifp_domains.c > +++ b/src/responder/ifp/ifp_domains.c > @@ -20,6 +20,8 @@ > */ > > #include <talloc.h> > +#include <tevent.h> > +#include <string.h> > > #include "db/sysdb.h" > #include "util/util.h" > @@ -27,6 +29,19 @@ > #include "responder/common/responder.h" > #include "responder/ifp/ifp_domains.h" > > +#define RETURN_DOM_PROP_AS_STRING(dbus_req, pvt_data, out, property) do { \ > + struct sss_domain_info *dom; \ When you declare a variable inside a macro, it's better to give it some strange name to minimize shadowing other variables. Something like _pvt_dom maybe. > + \ > + *(out) = NULL; \ > + \ > + dom = get_domain_info_from_req((dbus_req), (pvt_data)); \ > + if (dom == NULL) { \ > + return; \ > + } \ > + \ > + *(out) = dom->realm; \ dom->property :-) > +} while (0) > + The rest looks good to me. I'm still reviewing the latest patch, but I can't apply the one you sent previously to the thread, can you rebase (or rather regenerate the code, I think something had change in the last iteration of the GetAll patches that are luckily pushed now). _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
