Re: [chrony-dev] Feature chrony + patch

[Miroslav Lichvar <mlichvar@xxxxxxxxxx>]

On Wed, Sep 02, 2015 at 02:26:54PM +0200, Gautier Philippon wrote:
> I use a VPN and the DNS resolution is not the same if the VPN is up or down. The public ip resolution is still reachable when the VPN is up and that is why I wanted a way to refresh the source IP. 
> 
> I've patched myself chrony to add the a command refresh to the client which redo the DNS resolution for all the sources. I have sent it to Miroslav Lichvar who reviewed it and recommended me to send here for wider review. 

It looks good to me. Maybe the command could be improved to accept an
address and netmask as the other NTP source commands, but I think that
can be done later if needed.

I have just few comments on some details. Can you please then send it
as a git commit so it has your name attached to it? (e.g. with git
format-patch and git send-email commands)

> --- a/ntp_sources.c
> +++ b/ntp_sources.c
> @@ -114,6 +114,7 @@ static ARR_Instance pools;
>  static void resolve_sources(void *arg);
>  static void rehash_records(void);
>  static void clean_source_record(SourceRecord *record);
> +void NSR_IPRefreshAdresses(void);
>  
This shouldn't be here as the function is already declared in
ntp_sources.h.

>  static void
>  slew_sources(struct timeval *raw,
> @@ -661,7 +662,6 @@ resolve_source_replacement(SourceRecord *record)
>    us->replace_source = *record->remote_addr;
>  
>    append_unresolved_source(us);
> -  NSR_ResolveSources();
>  }

I think the NSR_ResolveSources() call can stay here, no need to move
it to the resolve_source_replacement() callers. It shouldn't matter if
it will be called multiple times with just one unresolved source
added.

> +/* ================================================== */
> +
> +void
> +NSR_IPRefreshAdresses(void)

Can you please rename it to NSR_RefreshAddresses to follow the
imperative form of the other function names in the file?

> @@ -1079,6 +1099,5 @@ NSR_GetActivityReport(RPT_ActivityReport *report)
>    }
>  }
>  
> -
>  /* ================================================== */
>  

This looks like an unrelated change.

Thanks,

-- 
Miroslav Lichvar

-- 
To unsubscribe email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "unsubscribe" in the subject.
For help email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "help" in the subject.
Trouble?  Email listmaster@xxxxxxxxxxxxxxxxxxxx.