[PATCH] Fix Bug 5917 - Samba does not work on site with Read Only Domain Controller

[Andrew Bartlett]

On Tue, 2013-09-03 at 14:10 -0700, Jeremy Allison wrote:
> On Wed, Sep 04, 2013 at 08:57:51AM +1200, Andrew Bartlett wrote:
> > On Tue, 2013-09-03 at 12:50 -0700, Richard Sharpe wrote:
> > > On Tue, Sep 3, 2013 at 12:37 PM, Jeremy Allison <jra at samba.org> wrote:
> > > > Fix inspired by a patch created by hemanth.thummala at gmail.com
> > > > on the bug.
> > > >
> > > > From his comment on the bug:
> > > >
> > > > "I am able to figure out the root cause. Looks like we are not doing domain
> > > > level DC discovery if we find few DCs at site level.
> > > >
> > > > In the code , initially discover_dc_dns() will find DCs at site level first.
> > > > And if the number of DCs returned from site is zero then it will try to fetch
> > > > the DCs at domain level(by setting site_name to NULL).
> > > >
> > > > DC validation is actually done later in process_dc_dns(). There we realize that
> > > > the list of DCs are not valid for domain join as in this case they are not
> > > > writable."
> > > >
> > > > This patchset fixes the problem in a cleaner
> > > > way by creating a wrapper function for
> > > > dsgetdcname() that does the sitename
> > > > manipulation that callers expect when
> > > > the pass in NULL or "" as the sitename,
> > > > yet still allows callers who want an
> > > > explicit site to pass in a non-NULL
> > > > sitename to get returns only from
> > > > that site.
> > > >
> > > > Hemanth has confirmed this fixes
> > > > the problem for him. Please review
> > > > and push. Patchset cleanly applies
> > > > to master, 4.1.0, 4.0.next and 3.6.next.
> > > 
> > > Reviewed-by: Richard Sharpe <rsharpe at samba.org>
> > > 
> > > If someone else does not push this I will do so tonight, CA time.
> > 
> > I've also reviewed it, and pushed it to autouild.
> 
> Thanks a *lot* Andrew ! There is one additional patch
> I think is needed on top as an optimization, which I
> realized after I'd sent the original mail. Here it is:
> 
> In the case where the requested site_name parameter
> to dsgetdcname() was null, but sitename_fetch() returned NULL
> (which I checked in the code and it can do so) we already
> did the dsgetdcname() with a NULL site name, so we shouldn't
> do the retry logic in that case.

Thanks.  While we are looking at additional changes, any chance of
changing this (for master) to use talloc?

malloc()/free() is so 1990's ;-)

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Catalyst IT                   http://catalyst.net.nz