Bug 9265 - Bind dlz fails to start if there is a trustedanchors zone
Summary: Bind dlz fails to start if there is a trustedanchors zone
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.0
Classification: Unclassified
Component: DNS server (show other bugs)
Version: 4.0.0rc2
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-08 06:54 UTC by Matthieu Patou
Modified: 2012-12-03 19:27 UTC (History)
3 users (show)

See Also:


Attachments
patch1 (964 bytes, patch)
2012-10-09 05:08 UTC, Amitay Isaacs
mat: review+
Details
patch2 (1.15 KB, patch)
2012-10-09 05:08 UTC, Amitay Isaacs
mat: review+
Details
replicate code change from dlz into internal dns server (1.22 KB, patch)
2012-10-13 08:03 UTC, Matthieu Patou
kai: review-
Details
Updated version of the patch with comments (1.49 KB, patch)
2012-10-13 08:25 UTC, Matthieu Patou
no flags Details
Updated version of the patch with comments (1.48 KB, patch)
2012-10-13 08:28 UTC, Matthieu Patou
no flags Details
Rechecked patch + strcmp format fix (1.48 KB, patch)
2012-10-13 22:18 UTC, Matthieu Patou
kai: review+
obnox: review+
Details
[PATCH] bug 9265 s4-dns: dlz_bind9: Ignore zones that are not used by BIND9 DLZ plugin (623 bytes, patch)
2012-10-17 05:57 UTC, Matthieu Patou
no flags Details
[PATCH] bug 9265 s4-rpc: dnsserver: Ignore DNS zones that are not used by RPC dnsserver (642 bytes, patch)
2012-10-17 06:06 UTC, Matthieu Patou
no flags Details
Patch proposed by metze (2.26 KB, patch)
2012-12-03 12:08 UTC, Karolin Seeger
obnox: review+
obnox: review+
kseeger: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matthieu Patou 2012-10-08 06:54:35 UTC

    
Comment 1 Matthieu Patou 2012-10-08 06:55:28 UTC
amitay, can you attach the patch you did for dlz ?
We need also my patch for the internal server.
Comment 2 Amitay Isaacs 2012-10-09 01:23:56 UTC
Matthieu, I have pushed the two patches upstream.

e65a24b59f1dc7d212a46014a1d7c2531263529f 
    s4-rpc: dnsserver: Ignore DNS zones that are not used by RPC dnsserver

d70f3644a485ef53e6173ef81326ba6f065f418a
    s4-dns: dlz_bind9: Ignore zones that are not used by BIND9 DLZ plugin
Comment 3 Amitay Isaacs 2012-10-09 05:08:20 UTC
Created attachment 8013 [details]
patch1
Comment 4 Amitay Isaacs 2012-10-09 05:08:47 UTC
Created attachment 8014 [details]
patch2
Comment 5 Amitay Isaacs 2012-10-09 05:10:48 UTC
Matthieu, please find the patches to fix the problem reported in this defect for BIND DLZ module and dnsserver RPC module. 

patch1 is for dnsserver RPC module
patch2 is for BIND DLZ module
Comment 6 Matthieu Patou 2012-10-13 08:03:19 UTC
Created attachment 8067 [details]
replicate code change from dlz into internal dns server
Comment 7 Kai Blin 2012-10-13 08:09:44 UTC
Comment on attachment 8067 [details]
replicate code change from dlz into internal dns server

I'd still prefer if we explicitly explain why we don't support zones we're skipping in the code.
Comment 8 Matthieu Patou 2012-10-13 08:25:21 UTC
Created attachment 8068 [details]
Updated version of the patch with comments
Comment 9 Matthieu Patou 2012-10-13 08:26:00 UTC
Comment on attachment 8068 [details]
Updated version of the patch with comments

Updated to take in accounts remarks of kai.
Comment 10 Matthieu Patou 2012-10-13 08:26:32 UTC
08:23 < ekacnet> as for the zones I think I explain why we don't do this 
08:26 < kblin> I want this in the code
08:26 < kblin> this is a magic constant, and I'd like the explanation with the magic constant
08:26 < ekacnet> ok wasn't clear for me 
08:27 < kblin> having to run git blame to figure out _why_ code does what it does sucks
Comment 11 Matthieu Patou 2012-10-13 08:28:47 UTC
Created attachment 8069 [details]
Updated version of the patch with comments
Comment 12 Kai Blin 2012-10-13 08:32:42 UTC
Comment on attachment 8069 [details]
Updated version of the patch with comments

That looks much better, thanks. Now if you could just align the two strcmps to
be below each other, it'd be perfect. :)
Comment 13 Kai Blin 2012-10-13 08:42:58 UTC
Also, the patch doesn't apply for me. :(
Comment 14 Matthieu Patou 2012-10-13 21:55:54 UTC
(In reply to comment #13)
> Also, the patch doesn't apply for me. :(

on master ? or about to samba-4.0 test ?
Comment 15 Matthieu Patou 2012-10-13 22:18:47 UTC
Created attachment 8071 [details]
Rechecked patch + strcmp format fix

Regenerated patch on top of d5c51e95ff8dbd4f5f50a7ea3f1d211194bb2809 with indentation fix
Applied with 
git am <patch_name>
Comment 16 Kai Blin 2012-10-13 22:34:21 UTC
Comment on attachment 8071 [details]
Rechecked patch + strcmp format fix

Great, thanks alot
Comment 17 Kai Blin 2012-10-13 22:37:11 UTC
Comment on attachment 8069 [details]
Updated version of the patch with comments

Marking old patch obsolete
Comment 18 Kai Blin 2012-10-13 22:37:45 UTC
Karolin, please pick for rc3
Comment 19 Karolin Seeger 2012-10-15 09:57:53 UTC
Pushed to autobuild-v4-0-test.
Closing out bug report.

Thanks!
Comment 20 Matthieu Patou 2012-10-17 05:54:21 UTC
Karolin the three patches needed to be picked up, for next RC can you pick-up the two patches from amitay ?
Comment 21 Matthieu Patou 2012-10-17 05:57:57 UTC
Created attachment 8080 [details]
[PATCH] bug 9265 s4-dns: dlz_bind9: Ignore zones that are not used by BIND9 DLZ plugin


Bug: 9265, part 2
Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Signed-off-by: Matthieu Patou <mat@matws.net>
---
 source4/dns_server/dlz_bind9.c |    5 +++++
 1 file changed, 5 insertions(+)
Comment 22 Matthieu Patou 2012-10-17 06:06:47 UTC
Created attachment 8081 [details]
[PATCH] bug 9265 s4-rpc: dnsserver: Ignore DNS zones that are not used by RPC dnsserver


..TrustAnchors zone is not interpreted by RPC dnsserver code.

Bug: 9265, part 1
Signed-off-by: Amitay Isaacs <amitay@gmail.com>

Autobuild-User(master): Amitay Isaacs <amitay@samba.org>
Autobuild-Date(master): Tue Oct  9 03:21:07 CEST 2012 on sn-devel-104

Signed-off-by: Matthieu Patou <mat@matws.net>
---
 source4/rpc_server/dnsserver/dnsdb.c |    4 ++++
 1 file changed, 4 insertions(+)
Comment 23 Karolin Seeger 2012-12-03 12:08:23 UTC
Created attachment 8266 [details]
Patch proposed by metze
Comment 24 Michael Adam 2012-12-03 12:15:54 UTC
Comment on attachment 8266 [details]
Patch proposed by metze

ACK
Comment 25 Michael Adam 2012-12-03 12:16:34 UTC
Comment on attachment 8014 [details]
patch2

obsoleted by patchset with cherry-pick-info
Comment 26 Michael Adam 2012-12-03 12:17:07 UTC
==> Karolin for 4.0
Comment 27 Karolin Seeger 2012-12-03 12:21:14 UTC
Pushed to autobuild-v4-0-test.
Comment 28 Michael Adam 2012-12-03 12:25:10 UTC
Comment on attachment 8071 [details]
Rechecked patch + strcmp format fix

This patch  is already in v4-0-test
Comment 29 Karolin Seeger 2012-12-03 19:27:08 UTC
Pushed to v4-0-test.
Closing out bug report.

Thanks!