[Freeipa-devel] [PATCH] bind-dyndb-ldap: hashsize() return type changed in libdns v164
Hello.
Since bind version 9.10.4b1 (libdns version 164), the return type of
hashsize() has changed from unsigned int to size_t. Without this change
the plugin does not compile against bind 9.10.4b1 or newer on 64bit
architecture.
I tested building of the package on Fedora 24 and 25 and also RHEL-7.3.
Regards,
--
Tomas Hozza
Senior Software Engineer - EMEA ENG Developer Experience
PGP: 1D9F3C2D
UTC+1 (CET)
Red Hat Inc. http://cz.redhat.com
>From 91b4fdefc5836c259b783e56d77ff3e27ad62236 Mon Sep 17 00:00:00 2001
From: Tomas Hozza
Date: Fri, 27 May 2016 10:21:15 +0200
Subject: [PATCH] hashsize() return type changed in libdns v164
Since bind version 9.10.4b1 (libdns version 164), the return type of
hashsize() has changed from unsigned int to size_t. Without this change
the plugin does not compile against bind 9.10.4b1 or newer on 64bit
architecture.
Signed-off-by: Tomas Hozza
---
src/ldap_driver.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index 5727641..83ec00a 100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -871,7 +871,11 @@ setcachestats(dns_db_t *db, isc_stats_t *stats)
return dns_db_setcachestats(ldapdb->rbtdb, stats);
}
+#if LIBDNS_VERSION_MAJOR >= 164
+size_t
+#else
unsigned int
+#endif /* LIBDNS_VERSION_MAJOR >= 164 */
hashsize(dns_db_t *db)
{
ldapdb_t *ldapdb = (ldapdb_t *) db;
--
2.5.5
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0399-0402] Do not log warning about empty zones which are already disabled or unloaded & prepare 9.0 release
On 05/09/2016 04:30 PM, Petr Spacek wrote: > On 9.5.2016 16:25, Petr Spacek wrote: > > Hello, > > > > following patch should cover most misleading warnings produced by new code > > handling empty zones. > > > > If it is okay I will release version 9.0 with it. > > > > Please review it ASAP. Thank you very much! > > ... and here are patches :-) > ACK. I tested the changes and warning is now logged only if the empty zone is still loaded. In case the configuration changes after the empty zone is already unloaded, no message is logged. Other than that, the changes look good to me. Regards, -- Tomas Hozza Senior Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D UTC+1 (CET) Red Hat Inc. http://cz.redhat.com -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0393-0398] Unload automatic empty zones only if conflicting forward zone has policy 'only'Add ability to log warningsUnload automatic empty zones which are super/sub/equal d
On 04/06/2016 01:42 PM, Petr Spacek wrote:
> Hello,
>
> attached patch set implements
> https://fedorahosted.org/bind-dyndb-ldap/ticket/160
> described in
> https://fedorahosted.org/bind-dyndb-ldap/wiki/BIND9/Design/AutomaticEmptyZones
>
> It will be accompanied with upgrade code in FreeIPA which will automatically
> convert forward zones as necessary.
>
ACK.
I reviewed the patches on GitHub
(https://github.com/pspacek/bind-dyndb-ldap/commits/empty_zones_unload_only)
I have these comments:
- commit 396
(https://github.com/pspacek/bind-dyndb-ldap/commit/a1bb7c0f802107d1fc67de8a58d0f33095accd06)
- it uses "forwarders" in wrong context. In many places and also in the
commit message you should use "forward zone" instead of "forwarder". "forwarder
is the server to which the queries are forwarded, not the zone.
- It wold be nice to explicitly note in the commit message, that conflicting
empty zones are unloaded regardless of the forward policy ("only" or "first").
- It would be good to document, that once you add forward zone, which causes
removal of empty zone and then you remove the forward zone, the empty zone is
not added back. The empty zone will appear again only after restart of BIND,
when it loads all empty zones and only existing ones are removed. The situation
is the same when you configure global forwarders and them remove them = all
empty zones are unloaded and these are not added back until the restart of
BIND. This is a limitation and IMHO the users should be informed about this
behavior, as it may not be completely expected.
- Configuring global forwarders as forward first creates too many log messages
(for each empty zone). This may not be a problem, but it is IMHO not necessary.
Also when I configure global forwarders as forward "only" and then change it to
forward "first", the log message is printed even though the empty zones are not
there any more, which is a bit misleading.
- Warning log message about the configuration of global forward zone are logged
every time on start-up regardless of the forward policy set in LDAP.
Other than that the changes look good to me. The functionality works as
documented with the shortcomings mentioned above.
PS: There seems to be some kind of race condition when you configure forward
zone and you have global forwarding turned off. In case you configure a forward
zone with forward "only" and it conflicts with an empty zone, you can get
SERVFAIL from the server for hostname from such zone. It is reproducible only
if you are quick enough. The next query is forwarded correctly and the response
from forwarder is returned.
Regards,
--
Tomas Hozza
Senior Software Engineer - EMEA ENG Developer Experience
PGP: 1D9F3C2D
UTC+1 (CET)
Red Hat Inc. http://cz.redhat.com
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0391-0392] Add missing return value checks to pthread operations & replace strcmp(var, "") with strlen(var) to workaround Clang bug 20144
On 03/01/2016 02:36 PM, Petr Spacek wrote: > Hello, > > Add missing return value checks to pthread operations. > Detected by clang 3.8 -O2 -Wunused-value. > > Replace strcmp(var, "") with strlen(var) to workaround Clang bug 20144. > https://llvm.org/bugs/show_bug.cgi?id=20144 > ACK. I was not able to reproduce the issues. However the changes look good to me. I tested the plugin on Fedora 24 with basic tasks (query, zone transfer, DNS update) without DNSSEC signing. Regards, -- Tomas Hozza Senior Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D UTC+1 (CET) Red Hat Inc. http://cz.redhat.com -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0384-0385] Replace isc_atomic_* in with reference counter
On 23.06.2015 11:32, Petr Spacek wrote: > On 10.6.2015 19:07, Petr Spacek wrote: > > Hello, > > > > Replace isc_atomic_* in MetaLDAP with reference counter abstraction. > > + > > Replace isc_atomic_* in instance tainting with reference counter > > abstraction. > > > > Reference counters are used as abstraction which hides missing > > isc_atomic_*() > > functions on some architectures. > > > > > > This change is necessary for architectures like s390x and ppc64le where BIND > > does not provide isc_atomic_* abstractions. > > Fixed version of the patch is attached. > > The same code is also on Github: > https://github.com/pspacek/bind-dyndb-ldap/commits/atomic_to_refcnt > > Thank you for review! > I did formal review of patches 384 and 385. The fixed version looks good. ACK. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0383] Fix metadb_iterator_destroy() to accept NULL iterators
On 08.06.2015 14:08, Petr Spacek wrote: > Hello, > > Fix metadb_iterator_destroy() to accept NULL iterators. > > This prevents potential crash in error handling, e.g. if memory > allocation failed. > Hi. I did formal review. The patch looks good. ACK. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0377-0382] Synchronize changes from LDAP after reconnect
On 05/28/2015 05:58 PM, Matus Honek wrote: > Hi, > > functionality seems to work fine. I have not checked the code thoroughly. > Kind of a test is attached (requires setting named's ldap connection > appropriately). > > ACK > > Matúš Honěk > > > - Original Message - > From: "Petr Spacek" > To: tho...@redhat.com, "Matus Honek" > Cc: freeipa-devel@redhat.com > Sent: Wednesday, May 27, 2015 2:50:52 PM > Subject: [PATCH 0377-0382] Synchronize changes from LDAP after reconnect > > Hello, > > https://fedorahosted.org/bind-dyndb-ldap/ticket/128 > > Previously records deleted when connection to LDAP server was down were not > synchronized properly. It should work now. > > I use this command to simulate broken connections and connection > re-establishment: > $ socat tcp-listen:3899,reuseaddr,fork tcp-connect:localhost:389 > > It should be enough to add "ldap://$(hostname):3899" as LDAP URI to > /etc/named.conf and then simulate changes by killing and restarting socat. > > Let me know if you need any assistance! > Hi. I did a formal review of the code. Everything looks good. ACK. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0367] Support unknown record types (RFC 3597)
On 05/22/2015 10:03 AM, Petr Spacek wrote: > On 18.5.2015 17:31, Petr Spacek wrote: > > Hello, > > > > This patch is unrelated to metaDB but it should be merged before alpha, too. > > > > Thank you for review! > > > > Support unknown record types (RFC 3597). > > > > Fallback to generic LDAP attribute "UnknownRecord;TYP256" if attempt to > > add specific attribute like "URIRecord" failed with > > LDAP_OBJECT_CLASS_VIOLATION > > and always delete both attributes like "URIRecord" and > > "UnknownRecord;TYPE256". > > > > https://fedorahosted.org/bind-dyndb-ldap/ticket/157 > > Fixed version is attached. Version 1 could dereference NULL pointers in second > iteration of while loops. > I did only formal review. Didn't find any issues. ACK. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0376] Add schema for unknown record types
On 05/21/2015 12:42 PM, Petr Spacek wrote: > Hello, > > Add schema for unknown record types. > > This patch complements my previous patch 367. > > The change was pushed to > https://github.com/pspacek/bind-dyndb-ldap/tree/unknown_record_types , too. > ACK Tomas -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0368-0371] Support LDAP MODRDN for ordinary DNS records
On 05/20/2015 09:06 AM, Petr Spacek wrote: > Hello, > > this patchset implements support for MODRDN for ordinary records. As noted in > ticket https://fedorahosted.org/bind-dyndb-ldap/ticket/123, we agreed > yesterday that renaming zones is out of scope and seems unnecessarily complex. > > This patch set depends on 'metadb' branch. It is also available from: > https://github.com/pspacek/bind-dyndb-ldap/tree/modrdn > > Thank you for your time! > I did formal review. Everything looks OK. ACK Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0339-0363] Implement meta-database
On 05/15/2015 11:37 AM, Petr Spacek wrote: > Hello, > > this patch set adds meta-database which is one of prerequisites for other > work. > > These changes should not be user-visible. You might compile the plugin with > CFLAGS="-DMETADB_DEBUG" and check contect of /tmp/metadb.db after BIND > shutdown. > > Please see > https://fedorahosted.org/bind-dyndb-ldap/ticket/151 > https://fedorahosted.org/bind-dyndb-ldap/wiki/Design/MetaDB > for further information and let me know if you can help you somehow. > In Patch 351 Rename ldap_entry_create() to ldap_entry_parse(), you should rename the functions also in documentation: https://github.com/pspacek/bind-dyndb-ldap/blob/4fb7bd42609c2b6a4ffbdf6f3a1e58e00d84fa1e/src/ldap_entry.c#L111 https://github.com/pspacek/bind-dyndb-ldap/blob/4fb7bd42609c2b6a4ffbdf6f3a1e58e00d84fa1e/src/ldap_entry.h#L101 Other than that, it looks good. I did no functional testing... It compiled, functional testing done by others. ACK Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0338] Add includes to zone.c to improve compatibility with BIND 9.9.4
On 05/07/2015 02:55 PM, Petr Spacek wrote: > Hello, > > This is minor improvement for patch set related to ticket #155. > > Add includes to zone.c to improve compatibility with BIND 9.9.4. > Hi. I tested and reviewed the patch from https://github.com/pspacek/bind-dyndb-ldap/commits/t155.syncptr ACK. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0322-0337] Fix mysterious failures in PTR record synchronization
On 05/05/2015 05:24 PM, Petr Spacek wrote: > Hello, > > Attached patch set is the best fix for > https://fedorahosted.org/bind-dyndb-ldap/ticket/155 > I was able to write. > > This patch set should fix vast majority of race conditions. Unfortunately it > cannot be 100 % reliable without support for LDAP transactions. > > For convenience you can download the whole tree from > https://github.com/pspacek/bind-dyndb-ldap/commits/t155.syncptr > HEAD = da2552632f6ce67f1bb9d9b3cdd3e0a8e06ce9ea > > Enjoy. > Hi. There is one unused variable after patch 325 Move SOA serial update functions to zone.c. - it looks like you forgot to remove: https://github.com/pspacek/bind-dyndb-ldap/blob/d616021d6665ebab97035efb687a88a4a139f636/src/ldap_helper.c#L3892 https://github.com/pspacek/bind-dyndb-ldap/blob/d616021d6665ebab97035efb687a88a4a139f636/src/ldap_helper.c#L4037 https://github.com/pspacek/bind-dyndb-ldap/blob/d616021d6665ebab97035efb687a88a4a139f636/src/ldap_helper.c#L4038 Other than that, patches look good. I tested them and reviewed from https://github.com/pspacek/bind-dyndb-ldap/commits/t155.syncptr ACK with the fix for unused variable. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH 0316] Fix crash triggered by zone objects with unexpected DN
On 02/24/2015 03:01 PM, Petr Spacek wrote: > Hello, > > On 18.2.2015 10:36, Tomas Hozza wrote: > > On 12/16/2014 04:32 PM, Petr Spacek wrote: > >> Hello, > >> > >> Fix crash triggered by zone objects with unexpected DN. > >> > >> https://fedorahosted.org/bind-dyndb-ldap/ticket/148 > >> > > NACK. > > > > The patch seems to make no difference when using the reproducer from ticket > > 148 > > > > 18-Feb-2015 10:34:09.067 running > > 18-Feb-2015 10:34:09.139 ldap_helper.c:4876: INSIST(task == inst->task) > > failed, back trace > > 18-Feb-2015 10:34:09.139 #0 0x55587a80 in ?? > > 18-Feb-2015 10:34:09.139 #1 0x7620781a in ?? > > 18-Feb-2015 10:34:09.139 #2 0x720b00b2 in ?? > > 18-Feb-2015 10:34:09.140 #3 0x71e7ccf9 in ?? > > 18-Feb-2015 10:34:09.140 #4 0x71e7d992 in ?? > > 18-Feb-2015 10:34:09.140 #5 0x720a7f3b in ?? > > 18-Feb-2015 10:34:09.140 #6 0x75dda52a in ?? > > 18-Feb-2015 10:34:09.140 #7 0x7508d79d in ?? > > 18-Feb-2015 10:34:09.140 exiting (due to assertion failure) > > > > Program received signal SIGABRT, Aborted. > > [Switching to Thread 0x7fffea7cd700 (LWP 1719)] > > 0x74fc18c7 in __GI_raise (sig=sig@entry=6) at > > ../sysdeps/unix/sysv/linux/raise.c:55 > > 55 return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig); > > Missing separate debuginfos, use: debuginfo-install > > cyrus-sasl-gssapi-2.1.26-19.fc21.x86_64 > > cyrus-sasl-lib-2.1.26-19.fc21.x86_64 cyrus-sasl-md5-2.1.26-19.fc21.x86_64 > > cyrus-sasl-plain-2.1.26-19.fc21.x86_64 gssproxy-0.3.1-4.fc21.x86_64 > > keyutils-libs-1.5.9-4.fc21.x86_64 libattr-2.4.47-9.fc21.x86_64 > > libdb-5.3.28-9.fc21.x86_64 libgcc-4.9.2-6.fc21.x86_64 > > libselinux-2.3-5.fc21.x86_64 nspr-4.10.8-1.fc21.x86_64 > > nss-3.17.4-1.fc21.x86_64 nss-softokn-freebl-3.17.4-1.fc21.x86_64 > > nss-util-3.17.4-1.fc21.x86_64 pcre-8.35-8.fc21.x86_64 > > sssd-client-1.12.3-4.fc21.x86_64 xz-libs-5.1.2-14alpha.fc21.x86_64 > > (gdb) bt > > #0 0x74fc18c7 in __GI_raise (sig=sig@entry=6) at > > ../sysdeps/unix/sysv/linux/raise.c:55 > > #1 0x74fc352a in __GI_abort () at abort.c:89 > > #2 0x55587c29 in assertion_failed (file=, > > line=, type=, cond=) at > > ./main.c:220 > > #3 0x7620781a in isc_assertion_failed > > (file=file@entry=0x720bad2a "ldap_helper.c", line=line@entry=4876, > > type=type@entry=isc_assertiontype_insist, > > cond=cond@entry=0x720baf04 "task == inst->task") at assertions.c:57 > > #4 0x720b00b2 in syncrepl_update (chgtype=1, entry=0x70125590, > > inst=0x77fa3160) at ldap_helper.c:4876 > > #5 ldap_sync_search_entry (ls=, msg=, > > entryUUID=, phase=LDAP_SYNC_CAPI_ADD) at ldap_helper.c:5031 > > #6 0x71e7ccf9 in ldap_sync_search_entry > > (ls=ls@entry=0x7fffe40008c0, res=0x7fffe4003870) at ldap_sync.c:228 > > #7 0x71e7d992 in ldap_sync_init (ls=0x7fffe40008c0, > > mode=mode@entry=3) at ldap_sync.c:792 > > #8 0x720a7f3b in ldap_syncrepl_watcher (arg=0x77fa3160) at > > ldap_helper.c:5247 > > #9 0x75dda52a in start_thread (arg=0x7fffea7cd700) at > > pthread_create.c:310 > > #10 0x7508d79d in clone () at > > ../sysdeps/unix/sysv/linux/x86_64/clone.S:109 > > Thank you for catching this! I was using slightly different test which > triggered the new code but by using different code path. > > This new version should be more robust. Please re-test it, thank you! > ACK for version 2. No crash during testing ;) Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0316] Fix crash triggered by zone objects with unexpected DN
On 12/16/2014 04:32 PM, Petr Spacek wrote: > Hello, > > Fix crash triggered by zone objects with unexpected DN. > > https://fedorahosted.org/bind-dyndb-ldap/ticket/148 > NACK. The patch seems to make no difference when using the reproducer from ticket 148 18-Feb-2015 10:34:09.067 running 18-Feb-2015 10:34:09.139 ldap_helper.c:4876: INSIST(task == inst->task) failed, back trace 18-Feb-2015 10:34:09.139 #0 0x55587a80 in ?? 18-Feb-2015 10:34:09.139 #1 0x7620781a in ?? 18-Feb-2015 10:34:09.139 #2 0x720b00b2 in ?? 18-Feb-2015 10:34:09.140 #3 0x71e7ccf9 in ?? 18-Feb-2015 10:34:09.140 #4 0x71e7d992 in ?? 18-Feb-2015 10:34:09.140 #5 0x720a7f3b in ?? 18-Feb-2015 10:34:09.140 #6 0x75dda52a in ?? 18-Feb-2015 10:34:09.140 #7 0x7508d79d in ?? 18-Feb-2015 10:34:09.140 exiting (due to assertion failure) Program received signal SIGABRT, Aborted. [Switching to Thread 0x7fffea7cd700 (LWP 1719)] 0x74fc18c7 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:55 55 return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig); Missing separate debuginfos, use: debuginfo-install cyrus-sasl-gssapi-2.1.26-19.fc21.x86_64 cyrus-sasl-lib-2.1.26-19.fc21.x86_64 cyrus-sasl-md5-2.1.26-19.fc21.x86_64 cyrus-sasl-plain-2.1.26-19.fc21.x86_64 gssproxy-0.3.1-4.fc21.x86_64 keyutils-libs-1.5.9-4.fc21.x86_64 libattr-2.4.47-9.fc21.x86_64 libdb-5.3.28-9.fc21.x86_64 libgcc-4.9.2-6.fc21.x86_64 libselinux-2.3-5.fc21.x86_64 nspr-4.10.8-1.fc21.x86_64 nss-3.17.4-1.fc21.x86_64 nss-softokn-freebl-3.17.4-1.fc21.x86_64 nss-util-3.17.4-1.fc21.x86_64 pcre-8.35-8.fc21.x86_64 sssd-client-1.12.3-4.fc21.x86_64 xz-libs-5.1.2-14alpha.fc21.x86_64 (gdb) bt #0 0x74fc18c7 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:55 #1 0x74fc352a in __GI_abort () at abort.c:89 #2 0x55587c29 in assertion_failed (file=, line=, type=, cond=) at ./main.c:220 #3 0x7620781a in isc_assertion_failed (file=file@entry=0x720bad2a "ldap_helper.c", line=line@entry=4876, type=type@entry=isc_assertiontype_insist, cond=cond@entry=0x720baf04 "task == inst->task") at assertions.c:57 #4 0x720b00b2 in syncrepl_update (chgtype=1, entry=0x70125590, inst=0x77fa3160) at ldap_helper.c:4876 #5 ldap_sync_search_entry (ls=, msg=, entryUUID=, phase=LDAP_SYNC_CAPI_ADD) at ldap_helper.c:5031 #6 0x71e7ccf9 in ldap_sync_search_entry (ls=ls@entry=0x7fffe40008c0, res=0x7fffe4003870) at ldap_sync.c:228 #7 0x71e7d992 in ldap_sync_init (ls=0x7fffe40008c0, mode=mode@entry=3) at ldap_sync.c:792 #8 0x720a7f3b in ldap_syncrepl_watcher (arg=0x77fa3160) at ldap_helper.c:5247 #9 0x75dda52a in start_thread (arg=0x7fffea7cd700) at pthread_create.c:310 #10 0x7508d79d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109 -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0319] Fix crash caused by race condition during resolver cache flushing
On 01/13/2015 02:16 PM, Petr Spacek wrote: > Hello, > > This patch should be applied to v2 branch. > > Fix crash caused by race condition during resolver cache flushing. > > dns_view_flushcache() call has to be always done in single-thread mode. > Locking around the call was missing in forwarder reconfiguration and > zone deletion which could cause crash. > > https://fedorahosted.org/bind-dyndb-ldap/ticket/142 ACK. I was not able to reproduce the issue with this patch included in bind-dyndb-ldap package using reproducer from https://bugzilla.redhat.com/show_bug.cgi?id=1126841#c14 Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0315] Support BIND 9.10
On 12/04/2014 05:04 PM, Petr Spacek wrote: > Hello, > > Support BIND 9.10. > > https://fedorahosted.org/bind-dyndb-ldap/ticket/139 > > > This patch definitely needs more testing but ...: > - It compiles with BIND 9.9 and BIND 9.10. > - It seems that it is able to load master and forward zones. > - Basic dynamic updates work. > > I did not test other features yet. > ACK. The driver compiles with BIND 9.10 and works with IPA. I tested basic usage. BTW available in COPR: http://copr-fe.cloud.fedoraproject.org/coprs/thozza/bind-9.10/ Tomas -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0307] Send DNS NOTIFY message after any modification to the zone
On 11/26/2014 01:46 PM, Martin Basti wrote: > On 07/11/14 15:34, Petr Spacek wrote: > > Hello, > > > > Send DNS NOTIFY message after any modification to the zone. > > > > https://fedorahosted.org/bind-dyndb-ldap/ticket/144 > > > Works for me. But don't push it before Tomas check the code please. > Martin^2 > ACK. Works for me... Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0310] Fix misleading error message about forward zones on reconnect
On 11/25/2014 07:25 PM, Martin Basti wrote: > On 25/11/14 18:27, Petr Spacek wrote: > > Hello, > > > > Fix misleading error message about forward zones on reconnect. > > > > Previously the plugin could log 'already exist' error after > > successful reconnection to LDAP for each active forward zone. > > > > Now it prints message: > > forward zone 'fw.example.com': loaded > > > > > Log looks better now, ACK if Tomas has no objections. > ACK. Looks good. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0308] Improve detection of BIND 9 isc__errno2result header file
On 11/25/2014 07:48 PM, Martin Basti wrote: > On 12/11/14 16:11, Petr Spacek wrote: > > Hello, > > > > Improve detection of BIND 9 isc__errno2result header file. > > > > This header file is not in standard distribution so normal isc-config.sh > > detection is not enough. > > > > With this patch, ./configure should work even without explicit CFLAGS and it > > should also detect that bind-devel or bind-lite-devel packages are missing. > > > > > Works for me > ACK. Works for me, too. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0309] Fix crash caused by interaction between forward and master zones
On 11/25/2014 07:07 PM, Martin Basti wrote: > On 25/11/14 18:11, Petr Spacek wrote: > > Hello, > > > > Fix crash caused by interaction between forward and master zones. > > > > LDAP modifications made to idnsName=sub, idnsName=example.com, cn=dns object > > were incorrectly processed using update_zone() in cases where forward zone > > sub.example.com. existed in LDAP as object idnsName=sub.example.com, cn=dns. > > > > https://fedorahosted.org/bind-dyndb-ldap/ticket/145 > > > > > > Tomas and Martin^2, please review it ASAP. Thank you! > > > Works for me, IPA tests passed, can you Tomas check the code before push? > ACK. The patch looks good. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0306] Improve info messages about number of defined/loaded zones
On 11/07/2014 01:33 PM, Petr Spacek wrote: > Hello, > > Improve info messages about number of defined/loaded zones. > ACK. The new message looks good. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0228] Drop unnecessary #define _BSD_SOURCE
On 11/25/2014 07:53 PM, Martin Basti wrote: > On 12/11/14 16:34, Petr Spacek wrote: > > On 25.2.2014 15:05, Lukas Slebodnik wrote: > >> On (25/02/14 09:54), Petr Spacek wrote: > >>> On 24.2.2014 18:56, Lukas Slebodnik wrote: > >>>> On (24/02/14 16:48), Petr Spacek wrote: > >>>>> Hello, > >>>>> > >>>>> Drop unnecessary #define _BSD_SOURCE. > >>>>> > >>>>> -- > >>>>> Petr^2 Spacek > >>>> >From 1b5105e3ab92f2a898313da5f7e20e6f3e9d1d2a Mon Sep 17 00:00:00 2001 > >>>>> From: Petr Spacek > >>>>> Date: Mon, 24 Feb 2014 16:48:09 +0100 > >>>>> Subject: [PATCH] Drop unnecessary #define _BSD_SOURCE. > >>>>> > >>>>> Signed-off-by: Petr Spacek > >>>>> --- > >>>>> src/krb5_helper.c | 2 -- > >>>>> 1 file changed, 2 deletions(-) > >>>>> > >>>>> diff --git a/src/krb5_helper.c b/src/krb5_helper.c > >>>>> index > >>>>> d1787209483f2ae49b480492290ff5d4bafc677c..71f4fff9fec551abbd81e25c59de80d2ded0dfc6 > >>>>> 100644 > >>>>> --- a/src/krb5_helper.c > >>>>> +++ b/src/krb5_helper.c > >>>>> @@ -15,8 +15,6 @@ > >>>>> * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 > >>>>> USA > >>>>> */ > >>>>> > >>>>> -#define _BSD_SOURCE > >>>>> - > >>>>> #include > >>>>> #include > >>>>> #include > >>>>> -- > >>>>> 1.8.5.3 > >>>>> > >>>> Simo is an author (according to git blame) > >>>> He defined this macro due to function setenv > >>>> > >>> >from man setenv: > >>>> NAME > >>>> setenv - change or add an environment variable > >>>> > >>>> SYNOPSIS > >>>> #include > >>>> > >>>> int setenv(const char *name, const char *value, int overwrite); > >>>> > >>>> int unsetenv(const char *name); > >>>> > >>>> Feature Test Macro Requirements for glibc (see > >>>> feature_test_macros(7)): > >>>> > >>>> setenv(), unsetenv(): > >>>> _BSD_SOURCE || _POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE > >>>> >= 600 > >>>> > >>>> > >>>> Macros _BSD_SOURCE _POSIX_C_SOURCE were defined when I included > >>>> header file . I tested only on fedora 20. It can be used > >>>> on the other distributions. > >>>> > >>>> I would rather let this macro as is. > >>> Wow, I didn't expect that somebody will spend time on this :-) > >>> > >>> See build logs from Fedora 21 > >>> http://koji.fedoraproject.org/koji/getfile?taskID=6565007&name=build.log > >> You should have noticed this in the 1st mail. Because it is difference > >> between > >> removing unnecessary macro and depprecated usage of macro. > >> > >> /usr/include/features.h:145:3: error: #warning "_BSD_SOURCE and > >> _SVID_SOURCE > >> are deprecated, use _DEFAULT_SOURCE" [-Werror=cpp] > >> # warning "_BSD_SOURCE and _SVID_SOURCE are deprecated, use > >> _DEFAULT_SOURCE" > >> > >>> Patches with 'the right' solution are welcome. I'm not going to spend > >>> more time on this. > > Attached patch should fix the warning in the 'proper' way, I hope. Without > > this patch the warning constantly pops up on Fedora 21. > > > Works for me, I haven't had warning there. > ACK. Works for me, too. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][bind-dyndb-ldap] AUTOCONF: Improve detection of bind9 header files
On 11/12/2014 03:30 PM, Petr Spacek wrote: > On 24.7.2014 11:00, Petr Spacek wrote: > > On 27.2.2014 15:19, Lukas Slebodnik wrote: > >> ehlo, > >> > >> I did some reviews of bind-dyndb-ldap last week and it was little bit > >> annoying > >> to export special CFLAGS for bind9 header files. It can be automatically > >> detected in configure script using utility isc-config. > >> > >> Attached patch should improve this and CFLAGS needn't be exported. > > > > Kind NACK. It would be valuable to test if isc/errno2result.h header is > > present and throw appropriate error. > > > > Current check with isc-config.sh only will pass if you have bind-devel > > package > > installed but you are missing bind-lite-devel package. > > > > > > I have a question: How > > +ldap_la_CFLAGS = $(BIND9_CFLAGS) -Wall -Wextra @WERROR@ -std=gnu99 > > works? > > > > Will it take user-defined CFLAGS into account? I would like to place > > user-defined flags at the end of the list so you can easily override > > settings > > given by autotools. > > > > Thank you for clarification :-) > > > > > > I will be really happy to commit complete fix. Thank you for cleaning this > > autotools mess! > > This version actually works. Previous version did not take CFLAGS from > isc-config.sh into account during libdns version check so it actually did not > work at all :-) > > Please review it (and send me a modified patch if you see a problem). > > Thank you for your time! > ACK. No need to export CPPFLAGS any more! Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [WIP] DNSSEC check for DNS forwarders
On 10/09/2014 10:50 AM, Petr Spacek wrote: > Hello, > > bad things will happen (i.e. external DNS resolution will not work) if > configured DNS forwarders are not standard compliant, i.e. EDNS or DNSSEC > support is not enabled. > > For this reason I'm proposing to add explicit check to IPA installer and > possibly even to dnsconfig-mod/dnszone-mod commands so forwarders are be > tested > before putting them in effect. > > This check should detect failures soon and prevent surprises where IPA > installs > itself but DNS resolution doesn't work for some domains etc. > > > Instructions for attached patch/script: > # ./dnssec_test.py 127.127.127.127 > -> Will (likely) time-out, print a warning and return None > - This should be a reason to abort installation because forwarder doesn't work > at all. > > # ./dnssec_test.py 10.1.2.3 > - Result depends on your local resolver. > - In RH's network it will print a scary warning message and return False > because > internal forwarder doesn't support DNSSEC. > - Should be a reason to abort installation. (This could be overridden by > --force > switch but then "dnssec-validation" option in /etc/named.conf has to be set to > "no" otherwise IPA DNS will not work properly.) > (I would rather force people to flip the switch in named.conf on forwarder so > this could be a hidden option.) > > # ./dnssec_test.py 199.7.83.42 > -> Should return True - forwarder works and DNSSEC is supported > - Installation should continue. > > Please voice your concerns ASAP. > I must confirm that if using DNSSEC, it is essential to probe the forwarder for proper DNSSEC support before using it. If the forwarder is not able to provide all the necessary information, the validation will not work. This is basically the same we are already doing on the client side where dnssec-trigger tries to determine if network-provided DNS forwarders are DNSSEC enabled before configuring unbound server. Therefore I agree with the idea, however it is up to IPA developers how they end up implementing the probing. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0298-0302] Implement handling of inactive master zones
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/19/2014 03:46 PM, Petr Spacek wrote: > Hello, > > This patch set fixes > https://fedorahosted.org/bind-dyndb-ldap/ticket/127 > https://bugzilla.redhat.com/show_bug.cgi?id=1138317 > > Please review it ASAP, it targets IPA 4.1/Fedora 21. > > Tomas and Martin, please communicate who is going to review what :-) > > Thank you for your time! > The code seems to be fine. ACK. Regards, - -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQEcBAEBAgAGBQJUIB/eAAoJEMWIetUdnzwtTdUH/iJX0CY5c5inZVXqOv+5Tt+V pcAwe/vlh6/3qJbZaA2sUc+i3M9dNhM2v2TPAugzfF1ZDGKwCjn8T+7XROsit/17 67XfZDhw/3Q4wsmsxR24YUXG5Q7TlX9NdlvFOUtsbeGfKdQKxsZB+cResv5dz6O8 p/gyNvvKJOE8nbJ33yE5A2tUocdgJHDcgsgCKWiYP3pOleJuKHYK0uyQZmAxxOJ+ q09KTBoEUg7fOI+ekReCzUysdDzOJSc+6zpJQ8LbK/g8Fa5Pg/+q9zBKVN5xng9l 4TCvVzz8kkVp8ArNAl4nE5eLVaYksT4xUWmf2RMzbtxOzJ+gmrYmce4mHRR6zcM= =ViAQ -END PGP SIGNATURE- ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0297] Add log message about initial LDAP synchronization
On 09/17/2014 01:33 PM, Petr Spacek wrote: > Hello, > > Add log message about initial LDAP synchronization. > ACK. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] [dyndb] Fix error handling in configure_view() to prevent deadlocks
On Thu 18 Sep 2014 08:49:05 AM CEST, Petr Spacek wrote:
> On 17.9.2014 20:04, Tomas Hozza wrote:
>> On Tue 16 Sep 2014 07:32:39 PM CEST, Petr Spacek wrote:
>>> Hello,
>>>
>>> attached patches fix
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1142150
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1142152
>>>
>>> ... and improve related error messages.
>>>
>>> I will push it to https://github.com/spacekpe/bind-dynamic_db if you are
>>> okay
>>> with it.
>>>
>>
>> I think there is a mistake in the first patch:
>> 0001-Fix-error-handling-in-configure_view-to-prevent-dead.patch
>>
>> diff --git a/lib/dns/dynamic_db.c b/lib/dns/dynamic_db.c
>> index
>> bf831617b391778ec540b2a5ca0df341937f2427..30c56a65c7227497c3e772c3e1b58ff49eacbd35
>>
>> 100644
>> --- a/lib/dns/dynamic_db.c
>> +++ b/lib/dns/dynamic_db.c
>> @@ -280,16 +280,24 @@ dns_dyndb_arguments_create(isc_mem_t *mctx)
>> }
>>
>> void
>> -dns_dyndb_arguments_destroy(isc_mem_t *mctx, dns_dyndb_arguments_t
>> *args)
>> +dns_dyndb_arguments_destroy(isc_mem_t *mctx, dns_dyndb_arguments_t
>> **argsp)
>> {
>> +dns_dyndb_arguments_t *args;
>> +
>> REQUIRE(args != NULL);
>> args is not initialized here. I think it should be "argsp"
>
> Good point! I will fix that. Do I have your ACK then? :-)
>
Yes, It worked with the change I proposed.
So ACK with the fix ;)
Regards,
--
Tomas Hozza
Software Engineer - EMEA ENG Developer Experience
PGP: 1D9F3C2D
Red Hat Inc. http://cz.redhat.com
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] [dyndb] Fix error handling in configure_view() to prevent deadlocks
On Tue 16 Sep 2014 07:32:39 PM CEST, Petr Spacek wrote:
> Hello,
>
> attached patches fix
> https://bugzilla.redhat.com/show_bug.cgi?id=1142150
> https://bugzilla.redhat.com/show_bug.cgi?id=1142152
>
> ... and improve related error messages.
>
> I will push it to https://github.com/spacekpe/bind-dynamic_db if you are okay
> with it.
>
I think there is a mistake in the first patch:
0001-Fix-error-handling-in-configure_view-to-prevent-dead.patch
diff --git a/lib/dns/dynamic_db.c b/lib/dns/dynamic_db.c
index
bf831617b391778ec540b2a5ca0df341937f2427..30c56a65c7227497c3e772c3e1b58ff49eacbd35
100644
--- a/lib/dns/dynamic_db.c
+++ b/lib/dns/dynamic_db.c
@@ -280,16 +280,24 @@ dns_dyndb_arguments_create(isc_mem_t *mctx)
}
void
-dns_dyndb_arguments_destroy(isc_mem_t *mctx, dns_dyndb_arguments_t
*args)
+dns_dyndb_arguments_destroy(isc_mem_t *mctx, dns_dyndb_arguments_t
**argsp)
{
+ dns_dyndb_arguments_t *args;
+
REQUIRE(args != NULL);
args is not initialized here. I think it should be "argsp"
+ args = *argsp;
+ if (args == NULL)
+ return;
+
dns_dyndb_set_view(args, NULL);
dns_dyndb_set_zonemgr(args, NULL);
dns_dyndb_set_task(args, NULL);
dns_dyndb_set_timermgr(args, NULL);
isc_mem_put(mctx, args, sizeof(*args));
+
+ *argsp = NULL;
}
Regards,
--
Tomas Hozza
Software Engineer - EMEA ENG Developer Experience
PGP: 1D9F3C2D
Red Hat Inc. http://cz.redhat.com
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0277] Bump NVR to 5.1
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 07/23/2014 02:27 PM, Petr Spacek wrote: > Hello, > > Bump NVR to 5.1. > ACK. Tomas -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJT0MHvAAoJEMWIetUdnzwtwG4H/3yoylMft5vskUK5thelM+TH 19dLJ5LmDNuEhLQx2nsz7L4BzI9XLfESj1KYZiqFzR9ENqvwb9uKoEV6rZztwnNe jT3A14AIRoE6S/hhaaXbDvOzTheyfQBbHh0WZxTY7Ge+QVv/1WfD1Ko8bLiPbNY3 m+CdXGGB+lnF6zBP+yNyyHwnyhI9Vda7JwVi5VfIVKPJCtafXloVTiHGKVBKyBj/ sojXcVOF2FSyhykBFbM24HnirWbHVUuv5NLiO1ciwKzJx3hL3feP8bTySCwr+FzH E4vvaa3jVT9yBLzz7bnzsL4r2jLn+IYaT84nG8SGlMyqNioHqxrQPKXRnSGBuds= =X41u -END PGP SIGNATURE- ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0276] Fix crash during reconnection to LDAP
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 07/23/2014 02:26 PM, Petr Spacek wrote: > Hello, > > Fix crash during reconnection to LDAP. > The fix works. ACK. Tomas -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJT0MHhAAoJEMWIetUdnzwtwBYH/1GyydKyhakuGp42PWLUHs55 cpl/VvHjQtDqlQPe5osxkMRhz32oJ/pn5B965lHZnRHsOySp9Cqtk/3dRiyxxvUA rllLgunTrC4oVM4SsLateREOZPl+hvIy599e2ZiXyHnPqvmi1rXN/SX9BZEhLGoh z6DAK6unkkcEG+8NUkt4SnPXwnQ6zY4yYicmAp73IjO9p09Xy8uV98xk6Od/UfSB 6VPfvre3OhEEndiCISnzti18LTKrmgCeH371bXb8gKcX8y3zNcnxAFE95Yp5N2W3 /qViRpHTt+iUABN4Bt5rk4fVMN95Mn9AndE2O7CKx42xdh9TMzYYPM5amZKZUz8= =Ka0U -END PGP SIGNATURE- ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0275] Add TLSARecord to idnsRecord object class
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 07/23/2014 02:25 PM, Petr Spacek wrote: > Hello, > > Add TLSARecord to idnsRecord object class. > ACK. Tomas -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJT0MG0AAoJEMWIetUdnzwtdCcH/RqNYSs4x/rRh0RfrFnl01UG ub7B7Er3BOgYwB1KeOcjFMBySI8DXZHPE40YshBw1fWfhatrc9DMPGZGJSebW2ti 4rvr6olfRHPwGJuzDxG7FLpd7CDt16m2Vc62a6191LlZeXYbeABimfUuz6Ffst+8 JejLkdKR2dpKOOYHWEZ1TMKG7WVd1qA3eOWdzWWVbiCxt7oAGZpJL+ftDh9UnJMS PwwWlYKKiZI8d+bqX7fmZfsGkMGIliQxEnKoRV/j+G/dJjIIGeGJuHRMas/NGlSL CUD8NuUTuJmLwYzZRkPrRoeOKiLhZhjlEuOhUMNqHNGaA91zi1bQYF9XeMPt16s= =m1A3 -END PGP SIGNATURE- ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0263-0265] Support root master zone in LDAP & Follow BIND semantics for forwarders
- Original Message - > Hello, > > This patch set contains necessary changes for supporting root master zone in > LDAP. I had to remove one hack so now we follow BIND semantics for > forwarders. > > Please see commit messages. > > https://fedorahosted.org/bind-dyndb-ldap/ticket/122 > > -- > Petr^2 Spacek > Looks good. ACK. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0261-0262] Support run-time changes in idnsSecInlineSigning attribute
- Original Message - > Hello, > > This patch set allows you to change DNSSEC zone configuration at run-time. > > -- > Petr^2 Spacek > Looks good. ACK. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0259] Fix run-time zone addition for invalid secure zones
- Original Message - > Hello, > > Fix run-time zone addition for invalid secure zones. > > It is important *not* to delete invalid zones to prevent > ldap_parse_master_zoneentry() from entering infinite cycle. > > Zone addition in ldap_parse_master_zoneentry() enforces serial > write-back to LDAP. This write generates LDAP modify event which > again triggers ldap_parse_master_zoneentry() and so on. > > https://fedorahosted.org/bind-dyndb-ldap/ticket/56 > > -- > Petr^2 Spacek > Looks good. ACK. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0260] Add wrappers for isc_task_*exclusive()
- Original Message - > Hello, > > Add wrappers for isc_task_*exclusive(). > > This patch replaces scattered isc_task_* calls and associated locking to one > place. It helps with debugging sometimes. > > -- > Petr^2 Spacek > Looks good. ACK. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0258] Fix run-time zone addition for secure zones
- Original Message - > Subject: Re: [Freeipa-devel] [PATCH 0258] Fix run-time zone addition for > secure zones > Date: Wed, 04 Jun 2014 17:34:29 +0200 > From: Petr Spacek > Organization: Red Hat > To: freeipa-devel@redhat.com > > On 3.6.2014 10:53, Petr Spacek wrote: > > Hello, > > > > Fix run-time zone addition for secure zones. > > Here comes fix for the fix ... > > We really need a test-suite for bind-dyndb-ldap. > > > https://fedorahosted.org/bind-dyndb-ldap/ticket/56 > > -- > Petr^2 Spacek > > > > ACK. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0266] (aka 257.5) Fix zone reloading for in-line signed zones
- Original Message - > Hello, > > I forgot to send one patch between no. 257 and 258, so here it is :-) > > Fix zone reloading for in-line signed zones. > > A invalid secure zone (e.g. without NS records) is now automatically > reloaded when data inside the zone are changed. > > https://fedorahosted.org/bind-dyndb-ldap/ticket/56 > > -- > Petr^2 Spacek > ACK. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0257] Fix race condition during zone loading
- Original Message - > On 28.5.2014 13:26, Tomas Hozza wrote: > > On 05/27/2014 03:59 PM, Petr Spacek wrote: > >> On 27.5.2014 15:54, Petr Spacek wrote: > >>> Fix race condition during zone loading. > >>> > >>> DNS zone has to be added to DNS view before dns_zone_load() is called. > >>> It is necessary to prevent dns_zone_load() from racing with > >>> dns_zone_setview(). > >>> > >>> This race condition sometimes prevents zone from being signed. > >>> Now the unsigned zone is visible until signing process is complete. This > >>> mimics BIND behavior for in-line signed zones. > >>> > >>> https://fedorahosted.org/bind-dyndb-ldap/ticket/56 > >> > >> And here is the patch... > >> > > > > Hi. > > > > When I use bind-dyndb-ldap plugin with this patch, named > > does not start due to: > > > > rbt.c:1379: REQUIRE(name->buffer != ((void *)0)) failed, back trace > > > > (gdb) bt > > #0 0x7f3963924c39 in raise () from /lib64/libc.so.6 > > #1 0x7f3963926348 in abort () from /lib64/libc.so.6 > > #2 0x7f3966979aee in assertion_failed () > > #3 0x7f3964b6917a in isc_assertion_failed () from /lib64/libisc.so.95 > > #4 0x7f39661ca9da in dns_rbt_fullnamefromnode () from > > /lib64/libdns.so.100 > > #5 0x7f396011824b in rbt_iter_getnodename (iter=, > > nodename=nodename@entry=0x7f39625f8bf0) at rbt_helper.c:46 > > #6 0x7f396011839b in rbt_iter_next > > (iterp=iterp@entry=0x7f39625f8b90, > > nodename=nodename@entry=0x7f39625f8bf0) at rbt_helper.c:144 > > #7 0x7f3960112d32 in activate_zones > > (task=task@entry=0x7f39668f5790, inst=0x7f39668e4160) at ldap_helper.c:1164 > > #8 0x7f396011a20d in barrier_decrement (task=0x7f39668f5790, > > event=0x7f396005b010) at syncrepl.c:138 > > #9 0x7f3964b8b836 in run () from /lib64/libisc.so.95 > > #10 0x7f396473ff33 in start_thread () from /lib64/libpthread.so.0 > > #11 0x7f39639e3ded in clone () from /lib64/libc.so.6 > > > > > > It looks like you should use INIT_BUFFERED_NAME(name); used in the > > original code instead of dns_name_init(&name, NULL). The macro > > initializes the buffer in "name", which is missing in the new code. > > Oh yes, it didn't happened on my machine because I have had only single zone > defined in LDAP at the time of testing. Thank you for catching this! > > I'm attaching fixed patch. dns_name_reset() is good enough in this case. > > -- > Petr^2 Spacek > Now it works. ACK Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0257] Fix race condition during zone loading
On 05/27/2014 03:59 PM, Petr Spacek wrote: > On 27.5.2014 15:54, Petr Spacek wrote: >> Fix race condition during zone loading. >> >> DNS zone has to be added to DNS view before dns_zone_load() is called. >> It is necessary to prevent dns_zone_load() from racing with >> dns_zone_setview(). >> >> This race condition sometimes prevents zone from being signed. >> Now the unsigned zone is visible until signing process is complete. This >> mimics BIND behavior for in-line signed zones. >> >> https://fedorahosted.org/bind-dyndb-ldap/ticket/56 > > And here is the patch... > Hi. When I use bind-dyndb-ldap plugin with this patch, named does not start due to: rbt.c:1379: REQUIRE(name->buffer != ((void *)0)) failed, back trace (gdb) bt #0 0x7f3963924c39 in raise () from /lib64/libc.so.6 #1 0x7f3963926348 in abort () from /lib64/libc.so.6 #2 0x7f3966979aee in assertion_failed () #3 0x7f3964b6917a in isc_assertion_failed () from /lib64/libisc.so.95 #4 0x7f39661ca9da in dns_rbt_fullnamefromnode () from /lib64/libdns.so.100 #5 0x7f396011824b in rbt_iter_getnodename (iter=, nodename=nodename@entry=0x7f39625f8bf0) at rbt_helper.c:46 #6 0x7f396011839b in rbt_iter_next (iterp=iterp@entry=0x7f39625f8b90, nodename=nodename@entry=0x7f39625f8bf0) at rbt_helper.c:144 #7 0x7f3960112d32 in activate_zones (task=task@entry=0x7f39668f5790, inst=0x7f39668e4160) at ldap_helper.c:1164 #8 0x7f396011a20d in barrier_decrement (task=0x7f39668f5790, event=0x7f396005b010) at syncrepl.c:138 #9 0x7f3964b8b836 in run () from /lib64/libisc.so.95 #10 0x7f396473ff33 in start_thread () from /lib64/libpthread.so.0 #11 0x7f39639e3ded in clone () from /lib64/libc.so.6 It looks like you should use INIT_BUFFERED_NAME(name); used in the original code instead of dns_name_init(&name, NULL). The macro initializes the buffer in "name", which is missing in the new code. Regards, Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][bind-dyndb-ldap] AUTOCONF: Improve detection of bind9 header files
- Original Message - > ehlo, > > I did some reviews of bind-dyndb-ldap last week and it was little bit > annoying > to export special CFLAGS for bind9 header files. It can be automatically > detected in configure script using utility isc-config. > > Attached patch should improve this and CFLAGS needn't be exported. > > LS > ACK. Works like a charm... Thanks! Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0251-0256] Add support for NSEC3
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 05/21/2014 11:33 AM, Petr Spacek wrote: > On 7.5.2014 15:27, Petr Spacek wrote: >> On 29.4.2014 23:34, Petr Spacek wrote: >>> This patch set adds support for NSEC3. See commit messages for details. >> >> Patch 253 was obsoleted by patches 244v2 and 246v2. >> >> You can download latest & greatest version from dnssec branch on github: >> >> https://github.com/spacekpe/bind-dyndb-ldap/tree/dnssec > > Patch 256v2 removes dead code from zone_master_reconfigure_nsec3param() > function. > > You can download latest & greatest version from dnssec branch on github. > > This doesn't solve a race condition somewhere in start-up sequence, I'm > looking into it. > Hi. I tested and reviewed patches 244-256 (all latest versions) and tested the https://github.com/spacekpe/bind-dyndb-ldap/tree/dnssec HEAD. Everything works as expected in constraints described in commit messages. There is still a race condition with signing that Petr is aware of and is working on it. (The zone is sometimes not signed if started using systemd). So I'm ACKing the patch-set 244-256 Regards, Tomas -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTfJRdAAoJEMWIetUdnzwtzW0H/ifMHoW8p3gapxBxt3nmWtT4 rlGZAU0V9dwO8DAsEM2J73ZIehzoEPOX2c8CGqa3uZwuph9fH4gwqDOfw452ho5B YqfI84hU18ncOHq5TXtu2SiwFqHWZveFATihx4Ds/Cg01KNSWeZ7bHzaaHQOlFOg FFl7CAX5raNgIY97H1nJxs1AfmTWGFDC3oRDpbA1NXIYvWFprri/WNnREFNLTwsW knxdxuS4pVpL9keQJUnQwbFbY12XqdGEhFgT8mwd0B9LEHsk1fTeat/P9rtOPPFF ot81VoJ3bPs5eUZ9TdiyP4Ur6Y0fGfoIMUXTyDJ5/OarkOi+tAZoPLn1Gz60N3E= =AWBQ -END PGP SIGNATURE- ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0239-0243] Refactor ldap_parse_master_zoneentry()
- Original Message - > On 17.4.2014 20:00, Petr Spacek wrote: > > Hello, > > > > This patch set attempts to move ldap_parse_master_zoneentry() a little bit > > closer to sane code. > > > > It is preparation for > > https://fedorahosted.org/bind-dyndb-ldap/ticket/56 > > bind-dyndb-ldap-pspacek-0242-2-Refactor-master-zone-configuration.patch fixes > zone loading for zones without idnsAllowTransfer attribute in LDAP. > > Previously, the plugin refused to load such zones with error ISC_R_NOTFOUND - > missing attribute was treated as fatal error. > > -- > Petr^2 Spacek > ACK. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0239-0243] Refactor ldap_parse_master_zoneentry()
- Original Message - > Hello, > > This patch set attempts to move ldap_parse_master_zoneentry() a little bit > closer to sane code. > > It is preparation for > https://fedorahosted.org/bind-dyndb-ldap/ticket/56 > > -- > Petr^2 Spacek > Patches look good. ACK. ACKing of version 2 of the patch 242 will follow. The patch 243 introduced new compilation warning that Peter is aware of. Unfortunately we are unable to find the root cause of it, so leaving it as is for now... Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0238] Update .gitignore to skip Eclipse and Autotools file
- Original Message - > Hello, > > Update .gitignore to skip Eclipse and Autotools files. > > -- > Petr^2 Spacek > ACK -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0237] Handle paths without trailing / in fs_dirs_create()
- Original Message - > Hello, > > Handle paths without trailing / in fs_dirs_create(). > > This patch should go to all branches with fs_dirs_create() function. > > -- > Petr^2 Spacek > Looks good. ACK Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0236] Fix crash in create_zone()
- Original Message - > Hello, > > Fix crash in create_zone(). > > dns_zone_getmgr(zone) call in cleanup section was called even if zone > was NULL. > > This patch should go to master, v4 and v3 branches where applicable. > > You probably need to use debugger to reproduce this crash. I have encountered > it during work on new DNSSEC code ... > > -- > Petr^2 Spacek > Looks good. ACK. Regards, -- Tomas Hozza Software Engineer - EMEA ENG Developer Experience PGP: 1D9F3C2D Red Hat Inc. http://cz.redhat.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0234] Prevent NULL dereference before sync_concurr_limit_signal() calls
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 04/09/2014 02:07 PM, Petr Spacek wrote: > Hello, > > Prevent NULL dereference before sync_concurr_limit_signal() calls. > > Missing check was causing NULL dereference in case where > manager_get_ldap_instance() failed. This typically happens when BIND > is processing LDAP updates during shutdown. > > I noticed this crash during sanity testing 4.2 release... > > Please review it ASAP so I can release 4.3. > > How to reproduce the problem: > Run BIND manually from console: > $ named -4 -g -u named -m record -n 10 > and press Ctrl+C "almost immediately". > > Sometimes it shutdowns cleanly and sometimes you can see a crash: > > Thank you for your time! > ACK. I'm not able to reproduce the issue, but the patch looks reasonable and should not break anything. Regards, Tomas Hozza -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTRUklAAoJEMWIetUdnzwtSSoH/0ZIz+MZfZ1O9JH5kGvJQKQo 3s1Fqeh601ReRKGnFu+nBt5hXzGzDOjsXM0x8bJH5r1cABn9XzYjupVQg0FjziM1 XIoJaDRB8L09sjLefjPoY87x0K6OsEQ/EmipDZPQB62MPpWJdGkJi6u2ug2gguRN fEdMeFdvm6T8faU/POd1D/1mEky2DqzNXViLf+6Pdi03b6lvUzoOvqsLwh4+Npvw NVUw7+SzwylCJQBcmZShCL6XRnaN4Qe9LCUG997HNwF/n3K1NjUgbwVqqmdLPGXh XAtcQxic5TQkKl2A1+52YFLMLSLY5gfAg/qBYmT1BQM+IuUIUYOe7fXL0fxr+A0= =TNAe -END PGP SIGNATURE- ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0231] Fix record parsing to prevent child zone corruption
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 04/01/2014 08:29 PM, Petr Spacek wrote: > Hello, > > Fix record parsing to prevent child zone corruption. > > Child zone hosted on the same server as parent zone was > corrupted by bug in update_record(). > Child zone's apex was modified by update_records() > instead of delegation records in the parent zone. > > https://fedorahosted.org/bind-dyndb-ldap/ticket/134 > ACK Solves the problem described in the ticket. Regards, Tomas Hozza -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTRP7XAAoJEMWIetUdnzwt6sMH/1OABEPs2+4nat6gKn849dTR ZrfrBiAu6nKMyzAGNfEGMpoqUxm94OrvdGXdqVhu0zUAT7NLTlvnlDziCqTggVnI vG7BENtMR4XMCDp+6o0cS8YsQoipUw4RGGA9eMvqD73tTUPGJf1XEMrTd0QQ5HJy XpsbGNMeMH+tzrTn65rM2zTCYZDDyO/adbejWyn3/JUfTpSLWicAldyUW1FECYtN vH2miG4vjwr+4ivZxvGNds0GskM3GZBxPHz6dWXkrrI2mKtR3kFgaOTKNtlNwfBC tCXYnlDuBDwEYe1kRekiG4gYhjjLq9dq8DgTTqO5W/J1Bz3eNjlJ3i3fh6o6s7g= =nrmr -END PGP SIGNATURE- ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0223] Update Fedora SPEC file for v4.0 (RPM expert needed)
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 02/21/2014 01:37 PM, Petr Spacek wrote: > On 21.2.2014 13:02, Tomas Hozza wrote: >> On 02/21/2014 12:54 PM, Tomas Hozza wrote: >>> On 02/21/2014 12:10 PM, Petr Spacek wrote: >>>> On 21.2.2014 11:05, Tomas Hozza wrote: >>>>> On 02/21/2014 10:46 AM, Petr Spacek wrote: >>>>>> I want to release bind-dyndb-ldap 4.0 to Fedora 20+ but I have found >>>>>> that we >>>>>> need to enable SELinux boolean named_write_master_zones otherwise the >>>>>> plugin >>>>>> will not be able to write journal files to /var/named. >>>>>> >>>>>> I have asked Miroslav Grepl for advice and his >>>>>> recommendation is to use another context for our dyndb-ldap >>>>>> sub-directory or >>>>>> to enable named_write_master_zones. >>>>>> >>>>>> (See https://bugzilla.redhat.com/show_bug.cgi?id=1066333) >>>>>> >>>>>> I have decided to use more generic named_write_master_zones because >>>>>> it will be >>>>>> need for DNSSEC key management anyway. >>>>>> >>>>>> Miroslav told me that it is allowed to change SELinux booleans in RPM >>>>>> scriptlets - it is normal operation - but that we have to disable the >>>>>> boolean >>>>>> during package un-installation. >>>>>> >>>>>> Please review %post and %postun sections in SPEC file. >>>>>> >>>>>> Thank you! >>>>>> >>>>>> -- Petr^2 Spacek > > >>>>>> +%post >>>>>> +if [ "0$1" -eq "1" ] && [ -x "/usr/sbin/setsebool" ] ; then >> >> I just noticed that you are setting the SELinux option ONLY when >> installing the package. I think you want to set it also if updating >> the package from older version... >> >> So you should use "-ge" instead of "-eq". > > Good catch! Fixes patch is attached. > > According to > https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Syntax > the condition is redundant so I replaced it with a comment about > intended effect. > >>>>>> + echo "Enabling SELinux boolean named_write_master_zones" >>>>>> + /usr/sbin/setsebool -P named_write_master_zones=1 || true >>>>> >>>>> I think you should redirect all output from the setsebool to /dev/null >>>>> so it does not produce any output during the "yum install". The same >>>>> for the "echo" I'm not sure if it should be there, but I didn't >>>>> find any >>>>> rule in packaging guidelines that is prohibiting you from doing so. >>> >>>> I don't understand what is the point. I guess that it is an anachronism >>>> from old times when RPM have problems with that. >>> >>>> If you don't insist (or find any rule about this) I will let the output >>>> as is. >>> >>>> IMHO it is much much better to show to user what went wrong instead of >>>> telling just "post scriptlet failed". >>> >>> I don't insist on this. However from my point of view at least the >>> STDOUT should be discarded. You may leave the STDERR as is. > > setsebool prints nothing anyway (unless there is an problem). I think > that SELinux policy is sensitive enough so any error/warning should be > visible to a user. > >>> Keep in mind that user using graphical installation tool will not >>> see those outputs anyway. > > I would call it a bug in the GUI tool. As far as I remember from > Synaptic utility (on Debian) have had a button like "Show me log". It > seems perfectly reasonable to me. However, I have never seen any > graphical package manager for Fedora :-) > Changes to the SPEC look good now. ACK from my side. Regards, Tomas -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTB0nPAAoJEMWIetUdnzwtONwIAJpc7mB1ptP7k6Ma6B8vv/55 IW9+YI4o9VydxhsW/2BHNsunX52/VT/bG1XKGhDtk5obK0QUudFj6nVFcwvm3wfM oImt0+4W/ALPJho28wil4IdRopJL72k0nssbCc6CudtafvCU/bAPYRrY6GtT8Aol yQh3dn2jsmqM7Vd0TUvU+zSm6Uo2ir3Lv7evubo9bGKUzWODy95XTjFy9QOBi26x 0UpKRrO4147bO19LLTM5gPyUUmZvTRxQAGcwhnpZwPY8+zr86lT4BoeKwAOC Bl96gAuwzhmQPxJXZZvYtUYeuDiaVhnQW3qC0QbYFQB1rAt7a3SKpyj/hEHec/c= =9hLp -END PGP SIGNATURE- ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0223] Update Fedora SPEC file for v4.0 (RPM expert needed)
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 02/21/2014 12:54 PM, Tomas Hozza wrote:
> On 02/21/2014 12:10 PM, Petr Spacek wrote:
>> On 21.2.2014 11:05, Tomas Hozza wrote:
>>> On 02/21/2014 10:46 AM, Petr Spacek wrote:
>>>> I want to release bind-dyndb-ldap 4.0 to Fedora 20+ but I have found
>>>> that we
>>>> need to enable SELinux boolean named_write_master_zones otherwise the
>>>> plugin
>>>> will not be able to write journal files to /var/named.
>>>>
>>>> I have asked Miroslav Grepl for advice and his
>>>> recommendation is to use another context for our dyndb-ldap
>>>> sub-directory or
>>>> to enable named_write_master_zones.
>>>>
>>>> (See https://bugzilla.redhat.com/show_bug.cgi?id=1066333)
>>>>
>>>> I have decided to use more generic named_write_master_zones because
>>>> it will be
>>>> need for DNSSEC key management anyway.
>>>>
>>>> Miroslav told me that it is allowed to change SELinux booleans in RPM
>>>> scriptlets - it is normal operation - but that we have to disable the
>>>> boolean
>>>> during package un-installation.
>>>>
>>>> Please review %post and %postun sections in SPEC file.
>>>>
>>>> Thank you!
>>>>
>>>> -- Petr^2 Spacek
>>>>
>>>>
>>>>
>>>> From a7329ae3459a135eff2897d3de9da607280b4615 Mon Sep 17 00:00:00 2001
>>>> From: Petr Spacek
>>>> Date: Fri, 21 Feb 2014 10:35:35 +0100
>>>> Subject: [PATCH] Update to 4.0.
>>>>
>>>> Signed-off-by: Petr Spacek
>>>> ---
>>>> bind-dyndb-ldap.spec | 31 ---
>>>> 1 file changed, 24 insertions(+), 7 deletions(-)
>>>>
>>>> ===
>>>>
>>>> diff --git a/bind-dyndb-ldap.spec b/bind-dyndb-ldap.spec
>>>> index
>>>> 85b59e40035a35276ee0997764cdd976a8716df5..cbe6b7c76327a9df8e49d4acf925be8f9c1da29b
>>>> 100644
>>>>
>>>> --- a/bind-dyndb-ldap.spec
>>>>
>>>> +++ b/bind-dyndb-ldap.spec
>>>>
>>>> @@ -1,26 +1,22 @@
>>>>
>>>> -#%define PATCHVER P4
>>>> -#%define PREVER 20121009git6a86b1
>>>> -#%define VERSION %{version}-%{PATCHVER}
>>>> -#%define VERSION %{version}-%{PREVER}
>>>> %define VERSION %{version}
>>>> Name: bind-dyndb-ldap
>>>> -Version: 3.5
>>>> +Version: 4.0
>>>> Release: 1%{?dist}
>>>> Summary: LDAP back-end plug-in for BIND
>>>> Group: System Environment/Libraries
>>>> License: GPLv2+
>>>> URL: https://fedorahosted.org/bind-dyndb-ldap
>>>> Source0:
>>>> https://fedorahosted.org/released/%{name}/%{name}-%{VERSION}.tar.bz2
>>>> BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u}
>>>> -n)
>>>> -BuildRequires: bind-devel >= 32:9.6.1-0.3.b1
>>>> +BuildRequires: bind-devel >= 32:9.9.0-1, bind-lite-devel >= 32:9.9.0-1
>>>> BuildRequires: krb5-devel
>>>> BuildRequires: openldap-devel
>>>> BuildRequires: automake, autoconf, libtool
>>>> -Requires: bind >= 32:9.6.1-0.3.b1
>>>> +Requires: bind >= 32:9.9.0-1
>>>> %description
>>>> This package provides an LDAP back-end plug-in for BIND. It features
>>>>
>>>> @@ -41,25 +37,45 @@
>>>>
>>>> make %{?_smp_mflags}
>>>> %install
>>>> rm -rf %{buildroot}
>>>> make install DESTDIR=%{buildroot}
>>>> +mkdir -m 770 -p %{buildroot}/%{_localstatedir}/named/dyndb-ldap
>>>> # Remove unwanted files
>>>> rm %{buildroot}%{_libdir}/bind/ldap.la
>>>> rm -r %{buildroot}%{_datadir}/doc/%{name}
>>>> +# SELinux boolean named_write_master_zones has to be enabled
>>>> +# otherwise plugin will not be able to write to /var/named
>>>> +%post
>>>> +if [ "0$1" -eq "1" ] && [ -x "/usr/sbin/setsebool" ] ; then
I just noticed that you are setting the SELinux option ONLY when
installing the package. I think you want to set it also if updating
the package from older version...
So you should use "-ge" instead of "-eq".
>>>> + echo "Enabling SELinux boolean named_write_master_zones"
>>>> + /usr/sbin/setsebool -P named_w
Re: [Freeipa-devel] [PATCH 0223] Update Fedora SPEC file for v4.0 (RPM expert needed)
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 02/21/2014 12:10 PM, Petr Spacek wrote:
> On 21.2.2014 11:05, Tomas Hozza wrote:
>> On 02/21/2014 10:46 AM, Petr Spacek wrote:
>>> I want to release bind-dyndb-ldap 4.0 to Fedora 20+ but I have found
>>> that we
>>> need to enable SELinux boolean named_write_master_zones otherwise the
>>> plugin
>>> will not be able to write journal files to /var/named.
>>>
>>> I have asked Miroslav Grepl for advice and his
>>> recommendation is to use another context for our dyndb-ldap
>>> sub-directory or
>>> to enable named_write_master_zones.
>>>
>>> (See https://bugzilla.redhat.com/show_bug.cgi?id=1066333)
>>>
>>> I have decided to use more generic named_write_master_zones because
>>> it will be
>>> need for DNSSEC key management anyway.
>>>
>>> Miroslav told me that it is allowed to change SELinux booleans in RPM
>>> scriptlets - it is normal operation - but that we have to disable the
>>> boolean
>>> during package un-installation.
>>>
>>> Please review %post and %postun sections in SPEC file.
>>>
>>> Thank you!
>>>
>>> -- Petr^2 Spacek
>>>
>>>
>>>
>>> From a7329ae3459a135eff2897d3de9da607280b4615 Mon Sep 17 00:00:00 2001
>>> From: Petr Spacek
>>> Date: Fri, 21 Feb 2014 10:35:35 +0100
>>> Subject: [PATCH] Update to 4.0.
>>>
>>> Signed-off-by: Petr Spacek
>>> ---
>>> bind-dyndb-ldap.spec | 31 ---
>>> 1 file changed, 24 insertions(+), 7 deletions(-)
>>>
>>> ===
>>>
>>> diff --git a/bind-dyndb-ldap.spec b/bind-dyndb-ldap.spec
>>> index
>>> 85b59e40035a35276ee0997764cdd976a8716df5..cbe6b7c76327a9df8e49d4acf925be8f9c1da29b
>>> 100644
>>>
>>> --- a/bind-dyndb-ldap.spec
>>>
>>> +++ b/bind-dyndb-ldap.spec
>>>
>>> @@ -1,26 +1,22 @@
>>>
>>> -#%define PATCHVER P4
>>> -#%define PREVER 20121009git6a86b1
>>> -#%define VERSION %{version}-%{PATCHVER}
>>> -#%define VERSION %{version}-%{PREVER}
>>> %define VERSION %{version}
>>> Name: bind-dyndb-ldap
>>> -Version: 3.5
>>> +Version: 4.0
>>> Release: 1%{?dist}
>>> Summary: LDAP back-end plug-in for BIND
>>> Group: System Environment/Libraries
>>> License: GPLv2+
>>> URL: https://fedorahosted.org/bind-dyndb-ldap
>>> Source0:
>>> https://fedorahosted.org/released/%{name}/%{name}-%{VERSION}.tar.bz2
>>> BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u}
>>> -n)
>>> -BuildRequires: bind-devel >= 32:9.6.1-0.3.b1
>>> +BuildRequires: bind-devel >= 32:9.9.0-1, bind-lite-devel >= 32:9.9.0-1
>>> BuildRequires: krb5-devel
>>> BuildRequires: openldap-devel
>>> BuildRequires: automake, autoconf, libtool
>>> -Requires: bind >= 32:9.6.1-0.3.b1
>>> +Requires: bind >= 32:9.9.0-1
>>> %description
>>> This package provides an LDAP back-end plug-in for BIND. It features
>>>
>>> @@ -41,25 +37,45 @@
>>>
>>> make %{?_smp_mflags}
>>> %install
>>> rm -rf %{buildroot}
>>> make install DESTDIR=%{buildroot}
>>> +mkdir -m 770 -p %{buildroot}/%{_localstatedir}/named/dyndb-ldap
>>> # Remove unwanted files
>>> rm %{buildroot}%{_libdir}/bind/ldap.la
>>> rm -r %{buildroot}%{_datadir}/doc/%{name}
>>> +# SELinux boolean named_write_master_zones has to be enabled
>>> +# otherwise plugin will not be able to write to /var/named
>>> +%post
>>> +if [ "0$1" -eq "1" ] && [ -x "/usr/sbin/setsebool" ] ; then
>>> + echo "Enabling SELinux boolean named_write_master_zones"
>>> + /usr/sbin/setsebool -P named_write_master_zones=1 || true
>>
>> I think you should redirect all output from the setsebool to /dev/null
>> so it does not produce any output during the "yum install". The same
>> for the "echo" I'm not sure if it should be there, but I didn't find any
>> rule in packaging guidelines that is prohibiting you from doing so.
>
> I don't understand what is the point. I guess that it is an anachronism
> from old times when RPM have problems with that.
>
> If you don't insist (or find any rule about this) I will let the output
> as is.
>
> IMHO it is much much better to show to
Re: [Freeipa-devel] [PATCH 0223] Update Fedora SPEC file for v4.0 (RPM expert needed)
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Hi Peter.
See comments below...
On 02/21/2014 10:46 AM, Petr Spacek wrote:
> Hello list,
>
> I want to release bind-dyndb-ldap 4.0 to Fedora 20+ but I have found that we
> need to enable SELinux boolean named_write_master_zones otherwise the plugin
> will not be able to write journal files to /var/named.
>
> I have asked Miroslav Grepl for advice and his
> recommendation is to use another context for our dyndb-ldap sub-directory or
> to enable named_write_master_zones.
>
> (See https://bugzilla.redhat.com/show_bug.cgi?id=1066333)
>
> I have decided to use more generic named_write_master_zones because it will
> be
> need for DNSSEC key management anyway.
>
> Miroslav told me that it is allowed to change SELinux booleans in RPM
> scriptlets - it is normal operation - but that we have to disable the boolean
> during package un-installation.
>
> Please review %post and %postun sections in SPEC file.
>
> Thank you!
>
> -- Petr^2 Spacek
>
>
>
> From a7329ae3459a135eff2897d3de9da607280b4615 Mon Sep 17 00:00:00 2001
> From: Petr Spacek
> Date: Fri, 21 Feb 2014 10:35:35 +0100
> Subject: [PATCH] Update to 4.0.
>
> Signed-off-by: Petr Spacek
> ---
> bind-dyndb-ldap.spec | 31 ---
> 1 file changed, 24 insertions(+), 7 deletions(-)
>
> ===
>
> diff --git a/bind-dyndb-ldap.spec b/bind-dyndb-ldap.spec
> index
> 85b59e40035a35276ee0997764cdd976a8716df5..cbe6b7c76327a9df8e49d4acf925be8f9c1da29b
> 100644
>
> --- a/bind-dyndb-ldap.spec
>
> +++ b/bind-dyndb-ldap.spec
>
> @@ -1,26 +1,22 @@
>
> -#%define PATCHVER P4
> -#%define PREVER 20121009git6a86b1
> -#%define VERSION %{version}-%{PATCHVER}
> -#%define VERSION %{version}-%{PREVER}
> %define VERSION %{version}
> Name: bind-dyndb-ldap
> -Version: 3.5
> +Version: 4.0
> Release: 1%{?dist}
> Summary: LDAP back-end plug-in for BIND
> Group: System Environment/Libraries
> License: GPLv2+
> URL: https://fedorahosted.org/bind-dyndb-ldap
> Source0:
> https://fedorahosted.org/released/%{name}/%{name}-%{VERSION}.tar.bz2
> BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
> -BuildRequires: bind-devel >= 32:9.6.1-0.3.b1
> +BuildRequires: bind-devel >= 32:9.9.0-1, bind-lite-devel >= 32:9.9.0-1
> BuildRequires: krb5-devel
> BuildRequires: openldap-devel
> BuildRequires: automake, autoconf, libtool
> -Requires: bind >= 32:9.6.1-0.3.b1
> +Requires: bind >= 32:9.9.0-1
> %description
> This package provides an LDAP back-end plug-in for BIND. It features
>
> @@ -41,25 +37,45 @@
>
> make %{?_smp_mflags}
> %install
> rm -rf %{buildroot}
> make install DESTDIR=%{buildroot}
> +mkdir -m 770 -p %{buildroot}/%{_localstatedir}/named/dyndb-ldap
> # Remove unwanted files
> rm %{buildroot}%{_libdir}/bind/ldap.la
> rm -r %{buildroot}%{_datadir}/doc/%{name}
> +# SELinux boolean named_write_master_zones has to be enabled
> +# otherwise plugin will not be able to write to /var/named
> +%post
> +if [ "0$1" -eq "1" ] && [ -x "/usr/sbin/setsebool" ] ; then
> + echo "Enabling SELinux boolean named_write_master_zones"
> + /usr/sbin/setsebool -P named_write_master_zones=1 || true
I think you should redirect all output from the setsebool to /dev/null
so it does not produce any output during the "yum install". The same
for the "echo" I'm not sure if it should be there, but I didn't find any
rule in packaging guidelines that is prohibiting you from doing so.
It is also "common" to use ":" instead of "true" after OR, but this is
a cosmetic thing.
You can find more information (if you didn't already) here:
https://fedoraproject.org/wiki/Packaging:ScriptletSnippets
> +fi
> +
> +
> +%postun
> +if [ "0$1" -eq "0" ] && [ -x "/usr/sbin/setsebool" ] ; then
> + echo "Disabling SELinux boolean named_write_master_zones"
> + /usr/sbin/setsebool -P named_write_master_zones=0 || true
The same as above...
> +fi
> +
> +
> %clean
> rm -rf %{buildroot}
> %files
> %defattr(-,root,root,-)
> %doc NEWS README COPYING doc/{example.ldif,schema}
> +%dir %attr(770, root, named) %{_localstatedir}/named/dyndb-ldap
> %{_libdir}/bind/ldap.so
> %changelog
> +* Wed Feb 19 2014 Petr Spacek 4.0-1
> +- update to 4.0
> +
> * Thu Jul 18 2013 Petr Spacek 3.5-1
> - update to 3.5
> --
>
> 1.8.5.3
Regards,
Tomas
-BEGIN PGP SIGNATURE-
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQEcBAEBAgAGBQJTByT9AAoJEMWIetUdnzwtbW0H/38n6O3KKuwbwZgV+SVToZLE
CIu7RvzLcLejhVyi8ncVrFUs4jS6xl4Uf2t9OmGjQlkuHECjXu/0Nz1Rkher2fZh
c4qyvKrpBaKXpcWtOHEdOKBCKEjq2Qjque1c4zeklSIqtJL5qqrLjcJGrtET5p8C
hFy3+FrnvY2va+vK1NJMFfvQ0qhU2OGOJG6SKrsOJcVy1GIVX3dRAMYL1mPyKlb3
LazBqa7vgWkw9ZwSzMH/5CMrih6te7DeEzCsTsXQY4oMGEro+2VoTMaVhNMu19jb
DuxUUG8AbPwh1p8yhhppf0s8gXZnKPGzBBnezkC6KBXmw3ppnUm8DLeclcNlrPU=
=6o0G
-END PGP SIGNATURE-
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo
Re: [Freeipa-devel] [PATCH 0221] Make getcwd() calls safer
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 02/18/2014 10:34 AM, Petr Spacek wrote: > ewer GCC complains that I didn't check return value from getcwd() ... Hi. I reviewed all patches from "PATCH 0181" to the latest one "PATCH 0221" and tested the bind-dyndb-ldap on Fedora 20 (adding/removing records/zones using FreeIPA WebUI, resolving using dig, AXFR, IXFR, adding/removing Dynamic UPDATES). I didn't encounter any issues other than documented in "Known problems and limitations" section of NEWS file. So I finally ACK all of these patches... :) Regards, Tomas -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTA16KAAoJEMWIetUdnzwteYcH/iKx8bWOD9AbwSGIezoxZfFO 0k2/XexmFPAkYJ44N7fYm9oYbLkZ7PoKdEdnXW0Etvsna7vS/GKxQvwJnOq3aYSL o8SqyD3KmvXgGWtBDZEkWcoVxyjAFpaDxGSnwjRXrL6AOjN6T7ZqetUJkEMlu+fD mpRoP8l0otk1me6VKMr1UTZuF8NUe7N+onMrHTCD8TQmteK1Kmi7vPasEoGdX30L I0SzEzpmfaDWR3WJXDasiiJo/iqZTuiLL5q7HjNqZVtGPRS4Ey5zYgR+9DEBmQ30 MaYlVbQX8seQvphlBMyeOFlvlV+GBm4zQpzXqeM78uWJvHnZQH/Aijz0OGd534Y= =rt23 -END PGP SIGNATURE- ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0204] Remove obsolete zr_get_rbt() function from zone register
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 11/11/2013 12:53 PM, Petr Spacek wrote: > Hello, > > Remove obsolete zr_get_rbt() function from zone register. > ACK. Patch looks good. Regards, Tomas Hozza -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJS2UiGAAoJEMWIetUdnzwtACUH/jygrRD1QKit5atNb416vWUM qTE/ozdZ6bFfRB9ndFSj3n8Qcq9wqOV493Dbe+Hhh8fdKmCSzqJ3MN6UpFhmv4M6 O0jAkYnMDqd+k5zb9+bVtqdj0SLvtzfqLGVL7ydxzg4zMp/H2Su1YdRARt/KkYUA z3nosofXgU418v0gG/+wegQKCzJPqQ7F/+ZuF6QbC9BAwYjpQA4FoH/gNZk7QuoU LafA/OveHEGgfmVq+5bcxMFYty2tLgWifRBCGruECwOc4qu8mhwVlZKb4FpsX5nR R5qh7W93d372QL/1I+QSHA4Z2rOYUhc04OBL90xPjf48jlzu8MnqRujvYddgy1U= =kk7O -END PGP SIGNATURE- ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH][bind-dyndb-ldap] Fix warning duplicate 'const' declaration specifier
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 10/30/2013 09:21 AM, Lukas Slebodnik wrote: > ehlo, > > There were few warnings in bind-dyndb-ldap "duplicate 'const' declaration > specifier". > > It does not make sense to have const twice in declaration > like a "const settings_set_t const settings_default_set" > This one was false positive. > > The 2nd warning revealed potential problem. > const char const * dns_str > With previous declaration, you cannot modify data, but you can modify > pointer itself. > *dns_str = "asdasd" //compilation error > dns_str++ //works fine > > If you want to disable modification data and disable modification of pointer > you will need to have 2nd const modifier after star "*" > > You can find some examples of "const" usage in attached file test.c or > you can read article with explanation of next declaration > "char *(*(**foo [][8])())[];" > http://eli.thegreenplace.net/2008/07/18/reading-c-type-declarations/ > > Simple patch is attached. > > LS > ACK. Looks good. Regards, Tomas Hozza -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJS2UbdAAoJEMWIetUdnzwtqKIIAKEnhrYiT85yvGYkMVUjGZ5Y s42WXAcJOswo8rAiZwbMPmyGU7Imr+tEYf92Uu8S9kRipI6RnQYO0WFjt/HP/qQJ DblisCEgrWiPwYRTrEVuk2K7HZXUIvcEhB6KXgGPLsBw0bNFxb8FYs2GND4NjByU c/OCTGLaRsRxqX7sLn4UYZl32xic/QKJUeUWkfSgCbB7hzAOQJh65I5pW8e8LJre DBihpudiWVs2c13rIxyAyvbGcJ9X3HUuiRt/j2kWIhK4ESzB7Rf2cE3R1Frz7Do9 uDz8/q9WXIXmmQKCnK3zc8IM1LukPBYQUFN2j9ThiqzDFb/lMhGpXO3EeNRtiMM= =t8pR -END PGP SIGNATURE- ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0202-0203] Improve performance of initial LDAP synchronizationDetect end of initial LDAP synchronization phase
- Original Message - > Hello, > > Improve performance of initial LDAP synchronization. > > Changes are not journaled and SOA serial is not incremented during initial > LDAP synchronization. > > This eliminates unnecessary synchronous writes to journal and also > unnecessary SOA serial writes to LDAP. > > See commit messages and comments in syncrepl.c for all the gory details. ACK. Patches look good. AXFR and IXFR works as expected. Also BIND starts up much faster with these patches. Good job... :) Regards, Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0201] Report error if RFC 4533 initialization failed
On 10/23/2013 05:14 PM, Petr Spacek wrote: > Hello, > > this patch belongs to 4.0 release. It allows the user to catch some > mis-configurations. > > It produces error messages like this: > LDAP error: Critical extension is unavailable: unable to start SyncRepl > session: is RFC 4533 supported on LDAP server? > ACK. Patch works and looks good. Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0197-0200] Preparation for bind-dyndb-ldap release 4.0
On 10/11/2013 03:35 PM, Petr Spacek wrote: > Hello, > > update documentation and schema files for upcoming version 4.0. > > This fixes typo in schema file: > https://fedorahosted.org/bind-dyndb-ldap/ticket/121 > > Have a nice weekend! > ACK. Looks good. Regards, Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0192-0196] Write all changes to journal
On 10/10/2013 07:05 PM, Petr Spacek wrote:
> Hello,
>
> this patch set adds journaling to bind-dyndb-ldap.
>
> Journaling requires proper SOA serial maintenance, so from now
> SOA serial auto-incrementation is mandatory.
>
> Journal file is deleted on each start, so IXFR is limited to time frame
> from last BIND start.
>
> https://fedorahosted.org/bind-dyndb-ldap/ticket/64
>
>
> You can use my personal branch for testing:
> https://github.com/spacekpe/bind-dyndb-ldap/tree/rbtdb.v7
>
> It contains all unpushed patches. Basically, next master should be identical
> to this branch.
>
> Thank you for your time!
>
> -- Petr^2 Spacek
ACK.
IXFR works as should. Also tested that journal is cleared after BIND
restart, so server responds with AXFR.
Patches look good, I have only one small thing to patch 0193:
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index
> 0786979a1970f4b61ac5b92dd5554bf87032d1ff..89acbe610519bbe0610a07d5fa5d4ffceddc71cd
> 100644
>
> --- a/src/ldap_helper.c
>
> +++ b/src/ldap_helper.c
>
> ...
>
> @@ -1401,7 +1405,155 @@
>
> cleanup:
> return result;
> }
> +/**
> + * Process strictly minimal diff and detect if data were changed
> + * and return latest SOA RR.
> + *
> + * @pre Input diff has to be minimal, i.e. it can't contain DEL & ADD
> operation
> + * for the same data under the same name and TTL.
> + *
> + * @pre If the tuple list contains SOA RR, then exactly one SOA RR
> deletion
> + * has to precede exactly one SOA RR addition.
> + * (Each SOA RR deletion has to have matching addition.)
> + *
> + * @param[in] diff Input diff. List of tuples can be empty.
> + * @param[out] soa_latest Pointer to last added SOA RR from tuple list.
> + * Result can be NULL if there is no added SOA RR
> + * in the tuple list.
> + * @param[out] data_changed ISC_TRUE if any data other than SOA serial
> were
> + * changed. ISC_FALSE if nothing (except SOA
> + * serial) was changed.
> + *
> + */
> +static isc_result_t ATTR_NONNULLS
> +diff_analyze_serial(dns_diff_t *diff, dns_difftuple_t **soa_latest,
> + isc_boolean_t *data_changed) {
> + dns_difftuple_t *t = NULL;
> + dns_rdata_t *del_soa = NULL; /* last seen SOA with op == DEL */
> + dns_difftuple_t *tmp_tuple = NULL; /* tuple used for SOA comparison */
> + isc_result_t result = ISC_R_SUCCESS;
> + int ret;
> +
> + REQUIRE(DNS_DIFF_VALID(diff));
> + REQUIRE(soa_latest != NULL && *soa_latest == NULL);
> + REQUIRE(data_changed != NULL);
> +
> + *data_changed = ISC_FALSE;
> + for (t = HEAD(diff->tuples);
> + t != NULL;
> + t = NEXT(t, link)) {
> + INSIST(tmp_tuple == NULL);
after this ^^^ line tmp_tuple is NULL in the current for loop cycle.
> + if (t->rdata.type != dns_rdatatype_soa)
> + *data_changed = ISC_TRUE;
> + else { /* SOA is always special case */
> + if (t->op == DNS_DIFFOP_DEL ||
> + t->op == DNS_DIFFOP_DELRESIGN) {
> + /* delete operation has to precede add */
> + INSIST(del_soa == NULL);
> + INSIST(tmp_tuple == NULL);
so this ^^^ check is duplicit
> + del_soa = &t->rdata;
> + } else if (t->op == DNS_DIFFOP_ADD ||
> + t->op == DNS_DIFFOP_ADDRESIGN) {
> + /* add operation has to follow a delete */
> + INSIST(del_soa != NULL);
> + INSIST(tmp_tuple == NULL);
and also this ^^^ check is duplicit
> + *soa_latest = t;
> +
> + /* detect if fields other than serial
> + * were changed (compute only if necessary) */
> + if (*data_changed == ISC_FALSE) {
> + CHECK(dns_difftuple_copy(t, &tmp_tuple));
> + dns_soa_setserial(dns_soa_getserial(del_soa),
> + &tmp_tuple->rdata);
> + ret = dns_rdata_compare(del_soa,
> + &tmp_tuple->rdata);
> + *data_changed = ISC_TF(ret != 0);
> + }
> + if (tmp_tuple != NULL)
> + dns_difftuple_free(&tmp_tuple);
> + /* re-start the SOA delete-add search cycle */
> + del_soa = NULL;
> + } else {
> + INSIST("unexpected diff: op != ADD || DEL"
> + == NULL);
> + }
> + }
> + }
> + /* SOA deletions & additions has to create self-contained couples */
> + INSIST(del_soa == NULL && tmp_tuple == NULL);
> +
> +cleanup:
> + if (tmp_tuple != NULL)
> + dns_difftuple_free(&tmp_tuple);
> + return result;
> +}
> +
> ...
Sorry for the formatting, obviously my email client is not perfect.
Hope you can understand it...
Regards,
Tomas
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0186-0191] Replace LDAP cache with RBTDB
On 10/10/2013 06:58 PM, Petr Spacek wrote: > On 8.10.2013 12:00, Tomas Hozza wrote: >> On 10/02/2013 12:57 PM, Petr Spacek wrote: >>> On 13.9.2013 15:31, Petr Spacek wrote: >>>> On 14.8.2013 16:42, Petr Spacek wrote: >>>>> On 14.8.2013 16:25, Petr Spacek wrote: >>>>>> On 1.8.2013 15:57, Petr Spacek wrote: >>>>>>> Hello, >>>>>>> >>>>>>> attached monster patches replace our internal cache/database with >>>>>>> RBTDB >>>>>>> implementation. See commit messages and comments inside. >>>>>>> >>>>>>> This patch set provides very basic functionality (including DNS >>>>>>> support for >>>>>>> updates). Error handling definitely needs more love, but it should >>>>>>> be enough >>>>>>> for rapid DNSSEC prototyping. >>>>>> >>>>>> Patch 186 v2: The code now applies incremental changes in LDAP to the >>>>>> in-memory database. Commit message was modified to mention that >>>>>> wildcards are >>>>>> now supported. >>>>>> >>>>>> Patch 187 v2: The code was re-worked and now it respects >>>>>> serial_autoincrement >>>>>> option. >>>>>> >>>>>> Patch 188 v2: Minor comment clean-up and rebase on top of patch >>>>>> 187 v2. >>>>>> >>>>>> Patch 189 v2: Call to deleterdataset() nested in substractrdataset() >>>>>> was >>>>>> deleted. This code was meant only for testing purposes. >>>>>> >>>>>> These patch set is now ready for review. Please see commit messages! >>>>>> Some >>>>>> functionality is missing intentionally, but it will be fixed by >>>>>> separate >>>>>> patches. >>>>> >>>>> It would be too easy! >>>>> >>>>> Patch 186 v3: Commit message was extended with information that LDAP >>>>> MODRDN >>>>> operation is not supported at the moment. >>>>> >>>>> Patch 187 v3: Missing file ldap_driver.h was added. >>>> >>>> This extended patch set handles correctly object deletion from LDAP. >>>> >>>> Patches 186-189 contain very minor changes, only moving code from one >>>> place to >>>> the other. >>>> >>>> See commit messages for patches 190 and 191. >>>> >>>> This should be testable. I would recommend to test the whole patch >>>> set at >>>> once, most probably it doesn't make much sense to test patches >>>> separately. >>> >>> bind-dyndb-ldap-pspacek-0186-5-Use-RBTDB-instead-of-internal-LDAP-cache.patch >>> >>> adds missing missing include (db.h) to zone_register.c. >>> >> >> ACK. >> >> Patches 186-191 tested. Adding/removing/modifying records works fine. >> Also PTR synchronization works. Zone transfer to slave and NOTIFY >> tested when changes occurred on master. > > Patch 191-2 fixed problem with zone removal and race condition during > zone load. I would recommend you to test it with other patch I plan to > send today :-) > ACK. Patch looks good. Regards, Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0186-0191] Replace LDAP cache with RBTDB
On 10/02/2013 12:57 PM, Petr Spacek wrote: > On 13.9.2013 15:31, Petr Spacek wrote: >> On 14.8.2013 16:42, Petr Spacek wrote: >>> On 14.8.2013 16:25, Petr Spacek wrote: On 1.8.2013 15:57, Petr Spacek wrote: > Hello, > > attached monster patches replace our internal cache/database with > RBTDB > implementation. See commit messages and comments inside. > > This patch set provides very basic functionality (including DNS > support for > updates). Error handling definitely needs more love, but it should > be enough > for rapid DNSSEC prototyping. Patch 186 v2: The code now applies incremental changes in LDAP to the in-memory database. Commit message was modified to mention that wildcards are now supported. Patch 187 v2: The code was re-worked and now it respects serial_autoincrement option. Patch 188 v2: Minor comment clean-up and rebase on top of patch 187 v2. Patch 189 v2: Call to deleterdataset() nested in substractrdataset() was deleted. This code was meant only for testing purposes. These patch set is now ready for review. Please see commit messages! Some functionality is missing intentionally, but it will be fixed by separate patches. >>> >>> It would be too easy! >>> >>> Patch 186 v3: Commit message was extended with information that LDAP >>> MODRDN >>> operation is not supported at the moment. >>> >>> Patch 187 v3: Missing file ldap_driver.h was added. >> >> This extended patch set handles correctly object deletion from LDAP. >> >> Patches 186-189 contain very minor changes, only moving code from one >> place to >> the other. >> >> See commit messages for patches 190 and 191. >> >> This should be testable. I would recommend to test the whole patch set at >> once, most probably it doesn't make much sense to test patches >> separately. > > bind-dyndb-ldap-pspacek-0186-5-Use-RBTDB-instead-of-internal-LDAP-cache.patch > adds missing missing include (db.h) to zone_register.c. > ACK. Patches 186-191 tested. Adding/removing/modifying records works fine. Also PTR synchronization works. Zone transfer to slave and NOTIFY tested when changes occurred on master. Regards, Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0185] Do not execute new LDAP search for each updated object
On 08/01/2013 03:52 PM, Petr Spacek wrote: > Hello, > > Do not execute new LDAP search for each updated object. > > Syncrepl delivers notification about change in particular object > along with all data from the object. Resource Records are parsed out > from this data instead of data obtained via separate LDAP search. > > Current code doesn't have any rate limitation. As a result, > ldap_sync_init/poll() can enqueue update events faster than rest of BIND > is able to dequeue and process them. Unprocessed events can consume > significant amount of memory. This area definitely needs improvement. > > > This is next in complete transition to RBTDB. It should go to master. > ACK. Tested Patch bundle 181 - 185. Common tasks like adding/deleting/updating records work fine. Also PTR sync, zone serial number incrementation is OK. Regards, Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0184] Use DNS_RDATA_MAXLENGTH from rdata.h instead of own definition
On 08/01/2013 03:51 PM, Petr Spacek wrote: > Hello, > > Use DNS_RDATA_MAXLENGTH from rdata.h instead of own definition. > > > This minor fix could go to v3 and master. > ACK. Tested Patch bundle 181 - 185. Common tasks like adding/deleting/updating records work fine. Also PTR sync, zone serial number incrementation is OK. Regards, Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0183] Move data structures for parser from ldap_qresult_t to ldap_entry_t
On 08/01/2013 03:49 PM, Petr Spacek wrote: > Hello, > > Move data structures for parser from ldap_qresult_t to ldap_entry_t. > > > The target branch is master. > ACK. Tested Patch bundle 181 - 185. Common tasks like adding/deleting/updating records work fine. Also PTR sync, zone serial number incrementation is OK. Regards, Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0181] Replace LDAP persistent search with syncrepl (RFC 4533)
On 07/22/2013 03:16 PM, Petr Spacek wrote: > On 22.7.2013 13:23, Petr Spacek wrote: >> Hello, >> >> Replace LDAP persistent search with syncrepl (RFC 4533). >> >> All direct operations with LDAP Persistent Search control are replaced >> by ldap_sync_* calls. >> >> Syncrepl code works in exactly same way as old psearch code: >> Only the DN of the modified object is re-used from the message, >> data from the object are fetched via separate LDAP search. >> >> Current code is not able to detect object renaming because we don't have >> UUID->DN mapping yet. >> >> Another limitation is that current code can't detect unchanged entries, >> so serial is incremented after each parsed LDAP object. > > Clang noticed potential NULL dereference in cleanup section of > ldap_syncrepl_watcher(). Fixed patch is attached. > ACK. Tested Patch bundle 181 - 185. Common tasks like adding/deleting/updating records work fine. Also PTR sync, zone serial number incrementation is OK. Regards, Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0182] Fix false error messages when nonexistent object/attribute is deleted
On 08/01/2013 03:48 PM, Petr Spacek wrote: > Hello, > > Fix false error messages when nonexistent object/attribute is deleted. > > > This patch should go to branches v3 and master. > ACK. Tested Patch bundle 181 - 185. Common tasks like adding/deleting/updating records work fine. Also PTR sync, zone serial number incrementation is OK. No unexpected errors are printed. The described scenario was fixed by this patch. Regards, Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0192] Prevent deadlock in PTR record synchronization (versions <= 2.x)
On 09/26/2013 03:11 PM, Petr Spacek wrote: > Hello, > > attached patch prevents/hides deadlock in plugin versions versions <= 2.x. > > I plan to push it to v2 branch. Branches v3 and newer shouldn't be > affected. > > https://fedorahosted.org/bind-dyndb-ldap/ticket/113 ACK. I tested the patch with: freeipa-server-3.1.5-1.fc18.x86_64 freeipa-client-3.1.5-1.fc18.x86_64 With the patch, records are added successfully for the client and no warnings/errors are printed in the server log. However the ipa-client-install command still prints "Failed to update DNS records." even though everything passed fine on the server. Maybe there is some issue with the ipa-client. Regards, Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0180] Remove support for zone_refresh mode and options cache_ttl and psearch
On 07/22/2013 01:23 PM, Petr Spacek wrote: > Hello, > > Remove support for zone_refresh mode and options cache_ttl and psearch. > > All three options are ignored and persistent search is always enabled. ACK. Looks good and plugin works well with the change. Anyway I think it would be good to change the README to reflect this change in configuration. Regards, Tomas ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0178-0179] Preparation for 3.5 release
On 07/16/2013 10:13 AM, Petr Spacek wrote: > Hello, > > I plan to release 3.5 as soon as all previous patches are ACKed. > ACK. Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0175-0177] Prepare transition from persistent search to RFC 4533
On 07/16/2013 10:04 AM, Petr Spacek wrote: > Hello, > > this patch set changes default configuration to 'psearch yes' and > changes README and informational messages accordingly. > ACK. Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0174] Fix crash during zone_refresh triggered by connection failure
On 07/15/2013 03:13 PM, Petr Spacek wrote: > Hello, > > Fix crash during zone_refresh triggered by connection failure. > > Variable 'iter' was initialized too late. Code in cleanup section of > refresh_zones_from_ldap() dereferenced the uninitialized variable. > ACK. The crash is fixed. Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0173] Improve logging for cases where SOA serial autoincrementation failed
On 07/11/2013 12:24 PM, Petr Spacek wrote: > Hello, > > Improve logging for cases where SOA serial autoincrementation failed. > ACK Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0171-0172] Fix potential problems found by Clang static analyzer
On 07/08/2013 10:51 AM, Petr Spacek wrote: > Hello, > > several warnings from Clang static analyzer popped up after upgrade to > Fedora 19. Attached patches should fix all problems found by > clang-analyzer-3.3-0.6.rc3.fc19.x86_64. > ACK Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0169-0170] Modernize autotools configuration
On 07/10/2013 12:27 PM, Petr Spacek wrote: > On 10.7.2013 10:29, Lukas Slebodnik wrote: >> On (04/07/13 15:23), Petr Spacek wrote: >>> >Hello, >>> > >>> >several warnings from autotools popped up after upgrade to Fedora 19. >>> >Attached patches should make autotools configuration more modern. >>> > >>> >-- >>> >Petr^2 Spacek >>> From 07caec808e394bfcbd898905e917731cb3778e68 Mon Sep 17 00:00:00 2001 >>> >From: Petr Spacek >>> >Date: Thu, 4 Jul 2013 14:04:57 +0200 >>> >Subject: [PATCH] Add missing ar check to configure.ac. >>> > >>> >Signed-off-by: Petr Spacek >>> >--- >>> >configure.ac | 1 + >>> >1 file changed, 1 insertion(+) >>> > >>> >diff --git a/configure.ac b/configure.ac >>> >index >>> 222e520e0aa61bca58cff81bea672fcab04355b3..b6d66de1b862a6860226a5f9ae4a3f33842e6100 >>> 100644 >>> >--- a/configure.ac >>> >+++ b/configure.ac >>> >@@ -10,6 +10,7 @@ AC_CONFIG_HEADERS([config.h]) >>> >AC_DISABLE_STATIC >>> > >>> ># Checks for programs. >>> >+AM_PROG_AR >>> >AC_PROG_CC >>> >AC_PROG_LIBTOOL >>> > >>> >-- >>> >1.8.3.1 >>> > >> This solution is not very portable. Automake <= 1.11 does not contain >> this >> macro. >> >> configure.ac:14: warning: macro `AM_PROG_AR' not found in library >> configure.ac:14: error: possibly undefined macro: AM_PROG_AR >>If this token and others are legitimate, please use >> m4_pattern_allow. >>See the Autoconf documentation. >> autoreconf: /usr/bin/autoconf failed with exit status: 1 >> >> Better solution will be: >> m4_ifdef([AM_PROG_AR], [AM_PROG_AR]) >> >> You can find more information about this in mail thread >> http://lists.gnu.org/archive/html/automake/2012-05/msg00014.html > > Thank you for catching this. Fixed patch is attached. > ACK Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0165] Fix crash caused by race-condition between shutdown and update processing
ACK. Works as expected. Regards, Tomas Hozza - Original Message - > Hello, > > Fix crash caused by race-condition between shutdown and update processing. > > Variable 'name' was uninitialized when manager_get_ldap_instance() returned > ISC_R_NOTFOUND. The successive call to dns_name_dynamic() caused the crash. > > -- > Petr^2 Spacek > ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0166] Fix minor coding style issue in update_config()
ACK The patch looks good. Regards, Tomas Hozza - Original Message - > Hello, > > Fix minor coding style issue in update_config(). > > -- > Petr^2 Spacek > ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0162] Mark function arguments with __attribute__((nonnull)) when appropriate
ACK. The change looks reasonable and I'm not able to come up with a better macro name... :) Regards, Tomas Hozza - Original Message - > Hello, > > Mark function arguments with __attribute__((nonnull)) when appropriate. > > This patch prevents bugs like > https://git.fedorahosted.org/cgit/bind-dyndb-ldap.git/commit/?id=65de3f4d5718edf27899cf90389cb7cb15f5d725 > > The patch seems big, but it is really trivial. > > > I'm open to suggestions for better macro names etc. > > -- > Petr^2 Spacek > ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0159] Deprecate configuration without persistent search
ACK. Looks good. Regards, Tomas Hozza - Original Message - > On 28.5.2013 15:55, Petr Spacek wrote: > > Hello, > > > > Deprecate configuration without persistent search. > > > > https://fedorahosted.org/bind-dyndb-ldap/ticket/120 > > This version of the patch adds notice to the README. > > -- > Petr^2 Spacek > ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0161] Validate authentication settings strictly
ACK. Works OK. Regards, Tomas Hozza - Original Message - > Hello, > > Validate authentication settings strictly. > > - auth_method 'SASL' do not accept bind_dn and password options > - auth_method 'simple' do not accept sasl_* and krb5_* options > - auth_method 'none' do not accept any of options above > > -- > Petr^2 Spacek > ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0160] Fix crash triggered by missing sasl_user parameter
ACK Works as expected. Regards, Tomas Hozza - Original Message - > Hello, > > Fix crash triggered by missing sasl_user parameter. > > -- > Petr^2 Spacek > ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0156-0158] Automatically disable empty zones when necessary
ACK. Patches look good and work as expected! Regards, Tomas Hozza - Original Message - > Hello, > > this patch set enables bind-dyndb-ldap to automatically unload empty zone > (see > RFC 6303) if an explicit configuration for this zone is present in LDAP. > > Please test it with idnsZone and also idnsForwardZone objectClasses. > > -- > Petr^2 Spacek > ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0155] Fix IPv6 handling in PTR record synchronization
ACK The patch looks good and works as expected. Regards, Tomas Hozza - Original Message - > Hello, > > Fix IPv6 handling in PTR record synchronization. > > https://fedorahosted.org/bind-dyndb-ldap/ticket/118 > > -- > Petr^2 Spacek > ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0151] Disallow all zone transfers/queries if transfer/query policy configuration failed
On 04/19/2013 12:44 PM, Petr Spacek wrote: > Hello, > > Disallow all zone transfers/queries if transfer/query policy > configuration failed. > > Without this patch the old policy stays in effect > if re-configuration with the new policy failed. > > https://fedorahosted.org/bind-dyndb-ldap/ticket/116 > ACK. Patch looks OK! Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0150] Do not delete whole node during PTR record synchronization.
On 04/18/2013 04:58 PM, Petr Spacek wrote: > Hello, > > Do not delete whole node during PTR record synchronization. > > https://fedorahosted.org/bind-dyndb-ldap/ticket/115 > ACK. The patch looks good. Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0149] Clean up PTR record synchronization code and make it more robust
On 04/18/2013 11:04 AM, Petr Spacek wrote: > Hello, > > Clean up PTR record synchronization code and make it more robust. > > PTR record synchronization was split to smaller functions. > Input validation, error handling and logging was improved > significantly. > ACK. The patch looks OK! Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0148] Explicitly return SERVFAIL if PTR synchronization is misconfigured.
On 04/16/2013 12:45 PM, Petr Spacek wrote:
> Hello,
>
> Explicitly return SERVFAIL if PTR synchronization is misconfigured.
>
> SERVFAIL will be returned if PTR synchronization is enabled
> in forward zone but reverse zone has dynamic updates disabled.
>
What the patch does little bit differs from what the commit
message says. Explanation follows:
Snip from ldap_helper.c (starting line 2959):
/* Get attribute "idnsAllowDynUpdate" for reverse zone or use default. */
dns_name_free(&zone_name, mctx);
dns_name_init(&zone_name, NULL);
CHECK(dn_to_dnsname(mctx, owner_zone_dn_ptr, &zone_name, NULL));
zone_settings = NULL;
result = zr_get_zone_settings(ldap_inst->zone_register, &zone_name,
&zone_settings);
if (result != ISC_R_SUCCESS) {
if (result == ISC_R_NOTFOUND)
log_debug(3, "active zone '%s' not found", zone_dn);
goto cleanup;
^
You replaced this goto with "CLEANUP_WITH(DNS_R_SERVFAIL)" but
the check if dynamic updates in reverse zone are enabled
is done in the following IF statement
}
CHECK(setting_get_bool("dyn_update", zone_settings, &zone_dyn_update));
if (!zone_dyn_update) {
log_debug(3, "dynamic update is not allowed in zone "
"'%s'", zone_dn);
CLEANUP_WITH(ISC_R_NOPERM);
}
The patch modifies the plugin to explicitly return SERVFAIL if there was
some error while getting settings of PTR zone (the zone does not exist,
etc).
Maybe it would be good to explicitly return SERVFAIL also if dynamic
updates in PTR zone are disabled and modify the commit message to
better express what this patch does.
Regards,
Tomas Hozza
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0147] Improve error logging for zones with idnsAllowDynUpdate == FALSE.
On 04/16/2013 12:44 PM, Petr Spacek wrote: > Hello, > > Improve error logging for zones with idnsAllowDynUpdate == FALSE. > > Zones with dynamic updates disabled are re-configured with empty > update policy string, so the update is refused by BIND and > an error is logged. > ACK. The patch looks reasonable. (I didn't do functional test) Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0146] Disallow all dynamic updates if update policy configuration failed
On 04/16/2013 10:40 AM, Petr Spacek wrote: > Hello, > > Disallow all dynamic updates if update policy configuration failed. > > Without this patch the old update policy stays in effect > when re-configuration failed. > ACK. The patch looks good. (I didn't do functional test) Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0143] Treat syntax errors in LDAP filters as fatal
On 04/09/2013 03:39 PM, Petr Spacek wrote: > Hello, > > Treat syntax errors in LDAP filters as fatal. > > Filters are hardcoded at the moment, this is preventive action. > ACK. The patch looks good. (I didn't do functional test) Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0142] Improve LDAP error logging
On 04/09/2013 03:27 PM, Petr Spacek wrote:
> Hello,
>
> Improve LDAP error logging.
>
> Diagnostic error message is logged when it is available.
>
>
> Plugin with this patch produces messages like:
>
> LDAP error: Server is unwilling to perform: Minimum SSF not met.: bind
> to LDAP server failed
>
> intead of
>
> bind to LDAP server failed: Server is unwilling to perform
>
>
> Second example is:
>
> LDAP error: Object class violation: attribute "mgrecord" not allowed
> : while modifying(add) entry 'idnsName=pspacek,
> idnsname=example.com,cn=dns,dc=e,dc=test'
>
> instead of
>
> ""
>
> :-D
>
> diff --git a/src/log.h b/src/log.h
> index
> 312f24322fd0c6f9943c6beb810ac0bcd8f3896c..cbf1a3faaaccea7391d65d018e80d8ec688fc111
> 100644
>
> --- a/src/log.h
>
> +++ b/src/log.h
>
> @@ -55,16 +55,30 @@
>
> log_write(GET_LOG_LEVEL(level), format, ##__VA_ARGS__)
> /* LDAP logging functions */
> -#define log_ldap_error(ld) \
> - do {\
> - int err;\
> - char *errmsg = ""; \
> - if (ldap_get_option(ld, LDAP_OPT_RESULT_CODE, &err) \
> - == LDAP_OPT_SUCCESS)\
> - errmsg = ldap_err2string(err); \
> - log_error_position("LDAP error: %s", errmsg); \
> - } while (0);\
> +#define LOG_LDAP_ERR_PREFIX "LDAP error: "
> +#define log_ldap_error(ld, desc, ...)
> \
> + do {
> \
> + int err;
> \
> + char *errmsg = NULL;
> \
> + char *diagmsg = NULL;
> \
> + if (ldap_get_option(ld, LDAP_OPT_RESULT_CODE, &err)
> \
> + == LDAP_OPT_SUCCESS) {
> \
> + errmsg = ldap_err2string(err);
> \
Getting error msg for the first time here.
> + if (ldap_get_option(ld,
> LDAP_OPT_DIAGNOSTIC_MESSAGE, &diagmsg) \
> + == LDAP_OPT_SUCCESS && diagmsg != NULL)
> { \
> + errmsg = ldap_err2string(err);
> \
Again getting error msg with the same "err". Maybe a copy-paste error?
> + log_error(LOG_LDAP_ERR_PREFIX
> "%s: %s: " desc, \
> + errmsg, diagmsg,
> ##__VA_ARGS__);\
> + ldap_memfree(diagmsg);
> \
> + } else
> \
> + log_error(LOG_LDAP_ERR_PREFIX
> "%s: " desc, \
> + errmsg, ##__VA_ARGS__);
> \
> + } else {
> \
> + log_error(LOG_LDAP_ERR_PREFIX
> \
> + ": "
> \
> + desc, ##__VA_ARGS__);
> \
> + }
> \
> + } while (0);
> void
> log_write(int level, const char *format, ...) ISC_FORMAT_PRINTF(2, 3);
Regards,
Tomas Hozza
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0141] Generalize attribute_name<->rdata_type conversions.
On 04/08/2013 07:45 PM, Petr Spacek wrote: > Hello, > > Generalize attribute_name<->rdata_type conversions. > > Attribute names are generated on-the-fly: String "Record" is appended > to textual representation of DNS RDATA type. > > String "Record" is cut down from the attribute name during > attribute name to rdata type conversion. > > From now, the plugin doesn't add artificial limitation to supported > record types. ACK. The patch looks good. (I didn't do functional test) Cosmetic issue: I think it would be good to dynamically allocate "mod_type" in LDAPMod in every case and include the "mod_type" memory freeing in free_ldapmod() function. Now one has to be be careful when it is statically or dynamically allocated. Before it was static in every case. Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0152] Replace TTL values > 2^31-1 with 0.
- Original Message - > On 3.5.2013 14:35, Tomas Babej wrote: > > On 04/30/2013 03:45 PM, Petr Spacek wrote: > >> Hello, > >> > >> Replace TTL values > 2^31-1 with 0. > >> > >> The rule comes from RFC 2181 section 8. > >> > >> https://fedorahosted.org/bind-dyndb-ldap/ticket/117 > >> > >> > >> > >> ___ > >> Freeipa-devel mailing list > >> Freeipa-devel@redhat.com > >> https://www.redhat.com/mailman/listinfo/freeipa-devel > > > > ACK, works fine. > > > > Just one question though, the patch as it is leaves the invalid TTL value > > in > > the tree, > > even though it is never interpreted as one (thanks to this patch). > > > > $ ipa dnsrecord-show ipa.example.com skuska --all > >dn: > >idnsname=skuska,idnsname=ipa.example.com,cn=dns,dc=ipa,dc=example,dc=com > >Record name: skuska > >Time to live: 2147483648 > >A record: 192.168.0.1 > >objectclass: top, idnsrecord > > > > from /var/log/messages: > > named[18275]: entry > > 'idnsname=skuska,idnsname=ipa.example.com,cn=dns,dc=ipa,dc=example,dc=com': > > entry TTL 2147483648 > MAXTTL, setting TTL to 0 > > > > Wouldn't that be confusing to the user? Shouldn't we fix the TTL value set > > in > > the entry as well? > > It is exactly what "original" BIND does. I would like to imitate the same > behaviour if you are not against it strongly. > > I think that: > 1) Somebody could use bind-dyndb-ldap with read-only access to LDAP. > 2) It will unnecessarily complicate the code. > > -- > Petr^2 Spacek Review ACK. The patch looks good. I also agree with Peter's reasoning. There is also an error logged when the TTL has MSB set, so one can notice there is a bad TTL value set in LDAP. Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
