[SSSD] [PATCH] Check DNS records before updating

Stephen Gallagher sgallagh at redhat.com
Mon Jul 11 20:51:43 UTC 2011


On Mon, 2011-07-11 at 14:57 +0200, Jakub Hrozek wrote:
> On 07/07/2011 09:01 PM, Stephen Gallagher wrote:
> > On Mon, 2011-07-04 at 19:24 -0400, Jakub Hrozek wrote:
> >> https://fedorahosted.org/sssd/ticket/802
> >>
> >> [PATCH 1/4] Do not hardcode default resolver timeout
> >> This will allow us to reuse the timeout during the dyndns check
> > 
> > Ack.
> > 
> >>
> >> [PATCH 2/4] Split reading resolver family order into a separate function
> >> The new function will be reused as well
> >>
> > 
> > Ack.
> > 
> >> [PATCH 3/4] Allow returning arbitrary address from resolv_hostent as string
> >> While we always connect to the first address in the list returned from 
> >> DNS, the dyndns check code needs the complete list
> >>
> > 
> > Ack
> > 
> >> [PATCH 4/4] Check DNS records before updating
> >> The check itself.
> >>
> > 
> > Nack.
> > nitpick: ipa_dyndns_gss_tsig_update_step() has a line:
> > DEBUG (7, ("Checking if the update is needed\n"));
> > The space between "DEBUG" and "(7," is wrong. (I also note that there's
> > a similar incorrect space in my original code in
> > ipa_dyndns_update_send() )
> > 
> > in ipa_dyndns_update_get_addrs_done() if the first address family
> > returns ENOENT, you're not checking the second address family.
> > 
> > Also, I'd prefer for readability that you should do:
> > if (ret == ENOENT) {
> >     tevent_req_done(req);
> >     return;
> > } else if (ret != EOK) {
> >     tevent_req_error(req, ret);
> >     return;
> > }
> > 
> > /* EOK */
> >    ...
> > 
> > It reads a lot nicer, leaves most of the function one indent level
> > further left and doesn't require the dangling tevent_req_error() at the
> > end.
> > 
> >> The patch also includes one change that I was wondering about splitting 
> >> out as a separate patch. The previous code would always delete both 
> >> address families even in case the updated address was detected from the 
> >> LDAP socket, so the other address family would disappear if set. This 
> >> patch changes the behaviour such that only the address family of the 
> >> ldap socket is deleted and readded (and also checked for in DNS).
> > 
> > This was actually intentional behavior. If the interface isn't
> > specified, we ONLY want to have the address that's connected to the LDAP
> > server stored. It's intentional that the other address family was
> > removed in this situation (since it may be a leftover from a connection
> > while on a different network that supported the other family).
> > 
> > 
> 
> Thans for the review, new patches are attached.
> 
> Only patch 4 has changed, though.

Ack.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20110711/87f2d61f/attachment.sig>


More information about the sssd-devel mailing list