Hi Andrew,
wow, how can this patch cause such a failure?
The dsgetdcname interface is only used in the s3 join code (including
offline joins where this patch became necessary) and in some few exotic
s3 netlogon server calls as well as in winbind. Are there any
logs/traces available to allow to compare success and failure without
and with the patch?
I guess the real cause for the failure could be more in the correctnes
fixes for the returned dcinfo (like those in
22d500ec5411c3e0e82711217b15e3a6e52e0224). Maybe these changes show up
visible in other places. I have started on a testsuite to test the
DsGetDCname interface output at least on the netapi layer a while back
but it is not finished yet. It could help to check if any of this output
mismatches expectations though.
Unfortunately I'm out of office this and next week (and travelling over
the weekend) so I cannot really research it myself right now. Sorry but
I can only take a closer look again on monday.
Thanks,
Guenther
On 14/10/2021 22:56, Andrew Bartlett wrote:> That's awesome work. Thanks.
>
> G?nther,
>
> Can you look into why this change looks to have caused this regression
> and take this from here?
>
> Thanks,
>
> Andrew Bartlett
>
> On Thu, 2021-10-14 at 22:11 +0200, Ingo Asche wrote:
>> Hi Andrew,
>>
>> by mistake I done the second one (git checkout
>> 22d500ec5411c3e0e82711217b15e3a6e52e0224) first. Does this matter?
>>
>> But the result was - as I think you hoped - that with the "git
>> revert
>> 997fbcbc902" the login worked.
>>
>> I will do the other one tomorrow.
>>
>> Regards
>> Ingo
>>
>> Andrew Bartlett schrieb am 14.10.2021 um 11:24:
>>> On Thu, 2021-10-14 at 11:18 +0200, Ingo Asche wrote:
>>>> Hi Andrew,
>>>>
>>>> first step done (git revert...) so it's now 4.15.0pre1-GIT-
>>>> 12ba3d9d8f2.
>>>>
>>>> Login is again working.
>>> Well that is awesome. Can you checkout 'master' or
'origin/v4-15-
>>> test'
>>> and see if 'git revert 997fbcbc902' also builds and works?
>>>
>>>> For the triple check that would be then
>>>> git checkout 997fbcbc902 and
>>>> git checkout 12ba3d9d8f2, right?
>>> it would be
>>>
>>> git checkout 997fbcbc902d945eb5261ddc6667f830fbcd5931
>>> and
>>> git checkout 22d500ec5411c3e0e82711217b15e3a6e52e0224
>>>
>>> But you have pretty much done that with the revert you tried
>>> already.
>>>
>>> The revert on master is the last key point, just so we know we can
>>> revert that as a possible fix going forward (we can't wind back
all
>>> the
>>> other changes since).
>>>
>>>> Can I do that at once or have I to build after both steps?
>>> Build install test etc after each step.
>>>
>>> Thanks!
>>>
>>> Andrew Bartlett
>>>
>>>> Regards
>>>> Ingo
>>>>
>>>> Andrew Bartlett via samba schrieb am 13.10.2021 um 22:11:
>>>>> That's a very strange commit to have broken it, but try
on
>>>>> master:
>>>>>
>>>>> git revert 997fbcbc902
>>>>>
>>>>> If that builds and works, then we have a simple workaround
>>>>> while we
>>>>> ask
>>>>> G?nther and Alexander for help as that is:
>>>>>
>>>>> commit 997fbcbc902d945eb5261ddc6667f830fbcd5931
>>>>> Author: G?nther Deschner <gd at samba.org>
>>>>> Date: Sat Feb 13 22:11:52 2021 +0100
>>>>>
>>>>> s3-dsgetdcname: return dcinfo also when delivering
from
>>>>> the
>>>>> cache.
>>>>>
>>>>> Guenther
>>>>>
>>>>> Signed-off-by: Guenther Deschner <gd at
samba.org>
>>>>> Reviewed-by: Alexander Bokovoy <ab at
samba.org>
>>>>>
>>>>> But do tripple-check, by manually confirming that commit
'git
>>>>> checkout 997fbcbc902' and the one before 'git
checkout
>>>>> 997fbcbc902^'.
>>>>>
>>>>> Thanks!
>>>>>
>>>>> On Wed, 2021-10-13 at 20:46 +0200, Ingo Asche wrote:
>>>>>> Hi Andrew, Hi Rowland,
>>>>>>
>>>>>> found a bad one: 4.15.0pre1-GIT-997fbcbc902
>>>>>>
>>>>>> After installing this one login failed. How should I
proceed?
>>>>>>
>>>>>> Regards
>>>>>> Ingo
>>>>>>
>>>>>> Andrew Bartlett schrieb am 13.10.2021 um 00:56:
>>>>>>> On Wed, 2021-10-13 at 10:47 +1300, Andrew Bartlett
via
>>>>>>> samba
>>>>>>> wrote:
>>>>>>>> On Tue, 2021-10-12 at 21:24 +0100, Rowland
Penny wrote:
>>>>>>>>> On Tue, 2021-10-12 at 22:13 +0200, Ingo
Asche wrote:
>>>>>>>>>> Hi Rowland,
>>>>>>>>>>
>>>>>>>>>> I asked Andrew the same, here's
what he answered:
>>>>>>>>>>
>>>>>>>>>> - Yes, you start from 4.14.0rc1 as this
is the branch
>>>>>>>>>> point
>>>>>>>>>> where
>>>>>>>>>> master
>>>>>>>>>> - split into 4.14 (working) and what
would eventually
>>>>>>>>>> be
>>>>>>>>>> 4.15.
>>>>>>>>>>
>>>>>>>>>> Regards
>>>>>>>>>> Ingo
>>>>>>>>>>
>>>>>>>>> Well, no one has complained about 4.14.x
version, the
>>>>>>>>> problem
>>>>>>>>> only
>>>>>>>>> seems to exist on 4.15.0 (and I cannot get
the problem
>>>>>>>>> to
>>>>>>>>> not
>>>>>>>>> work
>>>>>>>>> for
>>>>>>>>> myself), so surely it is something in
4.15.0. I await
>>>>>>>>> Andrew
>>>>>>>>> commenting
>>>>>>>>> on this. If you do have to start from
4.14.0rc1, then
>>>>>>>>> you
>>>>>>>>> are
>>>>>>>>> going
>>>>>>>>> to
>>>>>>>>> be at it for sometime.
>>>>>>>> G'Day Rowland,
>>>>>>>>
>>>>>>>> A bisect does a binary search, so even across
large
>>>>>>>> version
>>>>>>>> gaps
>>>>>>>> it
>>>>>>>> the
>>>>>>>> increased workload in testing is minimal. That
is why we
>>>>>>>> use
>>>>>>>> that
>>>>>>>> approach, rather than (say) linearly selecting
all
>>>>>>>> commits.
>>>>>>>>
>>>>>>>> It is exceedingly unlikely the issue was
introduced after
>>>>>>>> 4.15.0rc1,
>>>>>>>> so
>>>>>>>> we must start before that. The correct spot to
start is
>>>>>>>> 4.14.0rc1 as
>>>>>>>> discussed, master development that became 4.15
started
>>>>>>>> from
>>>>>>>> that
>>>>>>>> point.
>>>>>>>>
>>>>>>>> Somewhere between
>>>>>>> ... that point and ...
>>>>>>>
>>>>>>>> 4.15/master (they are so alike it
doesn't change
>>>>>>>> much) this regressed, and we will find it.
I've got an
>>>>>>>> idea
>>>>>>>> for
>>>>>>>> one
>>>>>>>> thing it might be but only testing changes
speculation
>>>>>>>> into
>>>>>>>> verification.
>>>>>>>>
>>>>>>>> Also manual bisect testing is something our
users can do
>>>>>>>> that
>>>>>>>> I
>>>>>>>> don't
>>>>>>>> have the time to handle right now, so it is
incredibly
>>>>>>>> valuable.
>>>>>>>>
>>>>>>>> Confirming that 4.14.0rc1 works *in the test
being done
>>>>>>>> as
>>>>>>>> the
>>>>>>>> validation* is important, as otherwise we could
be
>>>>>>>> chasing
>>>>>>>> the
>>>>>>>> wrong
>>>>>>>> thing as the decision basis.
>>>>>>>>
>>>>>>>> I trust this clarifies,
>>>>>>>>
>>>>>>>> Andrew Bartlett
>>>>>>>> --
>>>>>>>> Andrew Bartlett (he/him)
>>>>>>>> https://samba.org/~abartlet/
>>>>>>>> Samba Team Member (since 2001)
https://samba.org
>>>>>>>> Samba Team Lead, Catalyst IT
>>>>>>>> https://catalyst.net.nz/services/samba
>>>>>>>>
>>>>>>>> Samba Development and Support, Catalyst IT -
Expert Open
>>>>>>>> Source
>>>>>>>> Solutions
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
--
G?nther Deschner GPG-ID: 8EE11688
Red Hat gdeschner at redhat.com
Samba Team gd at samba.org