On Tue, Apr 30, 2013 at 08:16:05PM +0200, Jakub Hrozek wrote:
On Mon, Apr 22, 2013 at 02:49:11PM +0200, Jakub Hrozek wrote:
> On Mon, Apr 22, 2013 at 02:20:59PM +0200, Pavel Březina wrote:
> > On 04/17/2013 09:04 PM, Jakub Hrozek wrote:
> > >https://fedorahosted.org/sssd/ticket/1504
> > >
> > >Implements dynamic DNS updates for the AD provider. By default, the
> > >updates also update the reverse zone and run periodically every 24
> > >hours.
> >
> > Hi,
> > I will do more thorough review in next days, but there are two thing
> > that struck in my eyes.
> >
> > >0001-Active-Directory-dynamic-DNS-updates.patch
> > >
> > >
> > >From 5306abceba7e6eed97763a0cd48bb886ff161835 Mon Sep 17 00:00:00
> > >2001 From: Jakub Hrozek<jhrozek(a)redhat.com> Date: Tue, 16 Apr 2013
> > >17:04:43 +0200 Subject: [PATCH] Active Directory dynamic DNS updates
> > >
> > >https://fedorahosted.org/sssd/ticket/1504
> > >
> > >Implements dynamic DNS updates for the AD provider. By default, the
> > >updates also update the reverse zone and run periodically every 24
> > >hours.
> >
> > >--- a/src/man/sssd-ad.5.xml +++ b/src/man/sssd-ad.5.xml
> >
> > Please, move dyndns options into a separate xml and include it into
> > sssd-ad and sssd-ipa.
> >
>
> yes, I wanted to do so, but there are two differences I didn't know how
> to solve without spending too much time on a man page:
> 1) defaults. We could solve this by saying "In IPA the default is foo
> but in AD default is bar". It's ugly.
> 2) the IPA manpage includes the old options.
>
> Maybe it could be solved by using a stringparam of xsltproc, I'll take a
> look. Ideally I wanted to get the feature accepted first so it's
> testable, then worry about duplication in man pages :)
>
> > ad_dyndns.c and ipa_dyndns.c are *very* similar. It must be possible to
> > write this code only ones. Do you have any specific reasons why haven't
> > you done so?
>
> They access private data and call static functions directly. As above, I
> will take a look, but reducing code duplication is something we can do
> post-beta.
I split the common code into the sdap_dyndns.c module and now both IPA
and AD timers are just thin wrappers around this request.
I didn't squash the new request into the periodic update task patch,
sorry. I don't think it's necessary to spend time resolving conflicts
and actually I think the patches are easier to review this way.
New patches attached.
Attached patches are rebased on top of Pavel's AD site patches.