[snip]
> From 4ac6071a397b315957f0257bd0c9467ca48fcec0 Mon Sep 17
00:00:00 2001
> From: Jakub Hrozek <jhrozek(a)redhat.com>
> Date: Sun, 6 Jul 2014 22:53:27 +0200
> Subject: [PATCH 1/9] DYNDNS: Add a new option dyndns_server
Can you review this patch?
+++ b/src/man/sssd-ad.5.xml
@@ -812,6 +812,26 @@ ad_gpo_map_deny = +my_pam_service
</listitem>
</varlistentry>
+ <varlistentry>
+ <term>dyndns_server (string)</term>
+ <listitem>
+ <para>
+ The DNS server to use when performing a DNS
+ update. In most setups, it's recommended
to leave
+ this option unset.
+ </para>
+ <para>
+ Setting this option makes sense for
environments
+ where the DNS server is different from
the identity
+ server.
+ </para>
+ <para>
+ Default: None (let nsupdate choose the
server)
+ </para>
+ </listitem>
+ </varlistentry>
+
+
One empty line too many?
<xi:include
xmlns:xi="http://www.w3.org/2001/XInclude"
href="include/override_homedir.xml" />
<xi:include
xmlns:xi="http://www.w3.org/2001/XInclude"
href="include/homedir_substring.xml" />
ACK, I'm not running CI, I
would prefer to run in on whole patch set.
I was thinking whether we should amend the man page to state explicitly
that dyndns_server is not used in the first attempt to perform DNS
udpate, but only in the subsequent (failover) attempt that contains such
hints. However, I suppose that such change should be part of the patches
that actually change this behavior...or we could be silent about this...?
> From 618230028bac79daed2447d5ea428bf62ee20c26 Mon Sep 17 00:00:00 2001
> From: Pavel Reichl <preichl(a)redhat.com>
> Date: Thu, 23 Jul 2015 04:40:03 -0400
> Subject: [PATCH 2/9] DYNDNS: Don't use server cmd in nsupdate by default
>
>
https://fedorahosted.org/sssd/ticket/2495
ACK, but can you add some explanation to the commit message?
done
[snip]
> From 4807c5d5f641f561d02f3671da36227da181358c 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 6/9] DYNDNS: use realm and server commands only as fallback
>
> Resolves:
>
https://fedorahosted.org/sssd/ticket/2195
[...]
> diff --git a/src/providers/dp_dyndns.c b/src/providers/dp_dyndns.c
> index
ae3f913ee392a6513f75aab497e7f2d784784748..efa6d73dcf66cb685cf3f331b38e75b5435aead1 100644
> --- a/src/providers/dp_dyndns.c
> +++ b/src/providers/dp_dyndns.c
> @@ -394,11 +394,16 @@ nsupdate_msg_create_common(TALLOC_CTX *mem_ctx, const char
*realm,
> tmp_ctx = talloc_new(NULL);
> if (tmp_ctx == NULL) return NULL;
>
> + if (realm != NULL) {
> #ifdef HAVE_NSUPDATE_REALM
> realm_directive = talloc_asprintf(tmp_ctx, "realm %s\n", realm);
> #else
> realm_directive = talloc_asprintf(tmp_ctx, "\n");
> #endif
> + } else {
> + realm_directive = talloc_asprintf(tmp_ctx, "\n");
> + }
> +
Please move this block into a function..
done
[snip]
> From 02e090379b5ea50578b7f6451e89df0dd418762e 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 9/9] DYNDNS: fix nsupdate_msg_add_fwd()
>
> Update nsupdate_msg_add_fwd() to group commands by address family
> processed IP address belongs to.
Please reword the commit message, the "fix" part sounds scary. Please
also add the explanation why the separate sends are preferable.
Done.
[snip]
+ struct sockaddr_in sin;
+ memset (&sin, 0, sizeof (sin));
I don't like the spaces after calling a function, style-wise.
done
> + sin.sin_family = AF_INET;
> + sin.sin_addr.s_addr = inet_addr ("192.168.0.2");
> + ret = sss_get_dualstack_addresses(dyndns_test_ctx,
> + (struct sockaddr *) &sin,
> + &addrlist);
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel