On Thu, Oct 01, 2015 at 01:15:10PM +0200, Pavel Reichl wrote:
On 09/30/2015 03:43 PM, Jakub Hrozek wrote:
>On Tue, Sep 29, 2015 at 11:40:34AM +0200, Pavel Reichl wrote:
>>
>>
>>On 09/21/2015 05:47 PM, Jakub Hrozek wrote:
>>>On Mon, Sep 21, 2015 at 01:03:13PM +0200, Pavel Reichl wrote:
>>>>
>>>>
>>>>On 09/21/2015 10:52 AM, Pavel Březina wrote:
>>>>>On 08/28/2015 11:05 AM, Pavel Reichl wrote:
>>>>>>On 08/14/2015 04:12 PM, Jakub Hrozek wrote:
>>>>
>>>>>>Test failed because of missing patch [PATCH 6/9] DYNDNS: use
realm and
>>>>>>server commands only as fallback
>>>>>>
>>>>>>I must have skipped it while respinning the patch set, sorry.
>>>>>>
>>>>>>I attached this missing patch and test that was failing because
of it.
>>>>>
>>>>>Hi,
>>>>>
>>>>>> DYNDNS: use realm and server commands only as fallback
>>>>>>
>>>>>> Resolves:
>>>>>>
https://fedorahosted.org/sssd/ticket/2195
>>>>>
>>>>>You have a wrong ticket number here, I think. The patches look good
otherwise.
>>>>
>>>>Yes, thanks for noticing! Updated patchset attached.
>>>
>>>https://fedorahosted.org/sssd/ticket/2495 that the ticket reference is
>>>closed, did you mean another one?
>>
>>We cleared that off-list - patch was accidentally omitted from the original patch
set.
>
>Can you rebase these patches, please?
Sure, rebased patches attached.
The patches seem to work well for both IPA and AD, CI also passes:
http://sssd-ci.duckdns.org/logs/job/29/29/summary.html
See two questions inline:
From b59a854b5a57cfbedd79a5d3b42555c65d4a3c1f Mon Sep 17 00:00:00
2001
From: Pavel Reichl <preichl(a)redhat.com>
Date: Thu, 23 Jul 2015 10:51:50 -0400
Subject: [PATCH 1/2] DYNDNS: use realm and server commands only as fallback
Resolves:
https://fedorahosted.org/sssd/ticket/2495
---
[...]
@@ -375,6 +375,23 @@ static char *nsupdate_msg_add_ptr(char
*update_msg,
return update_msg;
}
+static char *
+nsupdate_msg_add_realm_cmd(TALLOC_CTX *mem_ctx, const char *realm)
+{
+ if (realm != NULL) {
+ return talloc_asprintf(mem_ctx, "realm %s\n", realm);
+ }
+ return talloc_asprintf(mem_ctx, "\n");
+}
+#else
+static char *
+nsupdate_msg_add_realm_cmd(TALLOC_CTX *mem_ctx, const char *realm)
+{
+ return talloc_asprintf(mem_ctx, "\n");
+}
Wouldn't this be more compact and readable:
static char *
nsupdate_msg_add_realm_cmd(TALLOC_CTX *mem_ctx, const char *realm)
{
#ifdef HAVE_NSUPDATE_REALM
if (realm != NULL) {
return talloc_asprintf(mem_ctx, "realm %s\n", realm);
}
#endif
return talloc_asprintf(mem_ctx, "\n");
}
Additionally, what versions of nsupdate do /not/ have the realm command?
If they are very old, can we just drop the support for them? (I haven't
checked, just asking..)
From 82c9372e5c0c38e83a87867cfc046ab1610791ae Mon Sep 17 00:00:00
2001
From: Pavel Reichl <preichl(a)redhat.com>
Date: Fri, 24 Jul 2015 13:25:56 -0400
Subject: [PATCH 2/2] DYNDNS: improve nsupdate_msg_add_fwd()
Update nsupdate_msg_add_fwd() to group commands by address family
processed IP address belongs to.
It's better to group removing old A addresses and adding new A
addresses in a single transaction. Same goes for AAAA addresses.
Separate transaction for A and AAAA addresses updates are important
because server might block updates for one of these families and thus
the update even for the non-blocked address family would unnecessarily
fail.
For more details please see:
https://fedorahosted.org/sssd/wiki/DesignDocs/DDNSMessagesUpdate
Resolves:
https://fedorahosted.org/sssd/ticket/2495
[...]
+ update_msg = talloc_asprintf_append(update_msg,
"send\n");
+
+ /* AAAA addresses first */
Should this say AAAA addresses next?
+ /* Remove existing entries as needed */
if (remove_af & DYNDNS_REMOVE_AAAA) {