[PATCH] Support for getting NTP servers from DHCP

Chris Lumens clumens at redhat.com
Thu May 9 15:15:32 UTC 2013


> The DHCP server returns various options back to the client,
>  which may contain a list of NTP servers.
> This commit adds support for checking the response if it contains
> some NTP servers. If there are some, they will be stored in
> ksdata.timezone.ntpservers (only if no NTP servers were
> specified by kickstart).

Oh, great idea.  I've just got a couple points to me.

> @@ -888,8 +891,29 @@ def networkInitialize(ksdata):
>          hostname = getHostname()
>          update_hostname_data(ksdata, hostname)
>  
> +def _get_ntp_servers_from_dhcp(ksdata):
> +    """Check if some NTP servers were returned from DHCP and set them
> +    to ksdata (if not NTP servers were specified in the kickstart)"""
> +    ntp_servers = nm.nm_ntp_servers_from_dhcp()
> +    log.info("got %d NTP servers from DHCP" % len(ntp_servers))
> +    hostnames = []
> +    for server_address in ntp_servers:
> +        try:
> +            hostname = socket.gethostbyaddr(server_address)[0]
> +        except socket.error:
> +            # getting hostname failed, just use the address returned from DHCP
> +            log.debug("getting NTP server hostname failed for address: %s"
> +                      % server_address)
> +            hostname = server_address
> +        hostnames.append(hostname)
> +    # check if some NTP servers were specified from kickstart
> +    if not ksdata.timezone.ntpservers:
> +        # no NTP servers were specified, add those from DHCP
> +        ksdata.timezone.ntpservers = hostnames
> +

I think you should put this test at the beginning of the function, and
invert it like so:

   if ksdata.timezone.ntpservers:
      return

That way, you're not going through all the work of resolving those
addresses just to potentially throw it away.

> @@ -263,6 +263,38 @@ def nm_device_ip_addresses(name, version=4):
>  
>      return retval
>  
> +def nm_ntp_servers_from_dhcp():
> +    """Return a list of NTP servers that were specified the reply of the
                                                          ^

I think you're missing a word there.

> +       DHCP server or empty list if no NTP servers were returned.
> +       The return_hostnames  parameter specifies if the NTP server IP
> +       addresses from the DHCP reply should be converted to hostnames.
> +    """

There's no return_hostnames parameter in this function.

- Chris


More information about the anaconda-patches mailing list