On 10/05/2015 11:55 AM, Jakub Hrozek wrote:
>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:
Sure, I'll do that.
>
>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..)
It seems to me that support for REALM keyword was introduced in bind 9.9.0 in
'bug' 2929 (
https://kb.isc.org/article/AA-00496/0/). In my opinion this is old
version and we can expect newer version to be present on rhel and fedora.
Petr, do you agree? Thanks!
If RHEL-6 and later contain the nsupdate version with realm, then I
would prefer to remove this code..it wouldn't be tested anyway (we don't
have separately built unit tests for this condition..)
But that can be done in a follow-up patch.