Patchwork [1/1] services: ntpd: Make large clock adjustments if necessary.

login
register
mail settings
Submitter Ludovic Courtès
Date Oct. 19, 2016, 8:49 p.m.
Message ID <87h9883vz7.fsf@gnu.org>
Download mbox | patch
Permalink /patch/16676/
State New
Headers show

Comments

Ludovic Courtès - Oct. 19, 2016, 8:49 p.m.
Leo Famulari <leo@famulari.name> skribis:

> On Thu, Oct 13, 2016 at 07:06:50AM +0200, John Darrington wrote:
>> I think that this should be a configuration item in the service, so that the
>> user can decide whether to have it or not.  I don't think we should force it
>> on the user.   Some applications break if the clock makes large jumps.
>
> I agree. This is really something that the system administrator should
> evaluate and fix on a case-by-case basis.

I agree too!  And I think it’s good idea to have defaults that match
upstream’s defaults.

>> The ntpd authors decided that the default behaviour is not to make large jumps
>> so I think we should respect that unless there is a good reason to do otherwise.
>> So I think this should be configurable in /etc/config.scm and the default should
>> be not to use -g
>
> For me, I have to do this so rarely (and never on the machines I
> currently use) that I am fine with having to run `ntpd -g ...` by hand.
>
> So, if we want to make this configurable in the OS configuration,
> volunteers are welcome to work on it :)

Like this?
If that’s fine with you, I’ll commit it with a doc update.

Thanks,
Ludo’.
Leo Famulari - Oct. 19, 2016, 9:39 p.m.
On Wed, Oct 19, 2016 at 10:49:32PM +0200, Ludovic Courtès wrote:
> diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
> index 7495179..ce281c5 100644
> --- a/gnu/services/networking.scm
> +++ b/gnu/services/networking.scm
> @@ -265,11 +265,13 @@ Protocol (DHCP) client, on all the non-loopback network interfaces."
>    ntp-configuration?
>    (ntp      ntp-configuration-ntp
>              (default ntp))
> -  (servers  ntp-configuration-servers))
> +  (servers  ntp-configuration-servers)
> +  (allow-large-adjustment? ntp-allow-large-adjustment?
> +                           (default #f)))
>  
>  (define ntp-shepherd-service
>    (match-lambda
> -    (($ <ntp-configuration> ntp servers)
> +    (($ <ntp-configuration> ntp servers allow-large-adjustment?)
>       (let ()
>         ;; TODO: Add authentication support.
>         (define config
> @@ -296,7 +298,10 @@ restrict -6 ::1\n"))
>                (requirement '(user-processes networking))
>                (start #~(make-forkexec-constructor
>                          (list (string-append #$ntp "/bin/ntpd") "-n"
> -                              "-c" #$ntpd.conf "-u" "ntpd")))
> +                              "-c" #$ntpd.conf "-u" "ntpd"
> +                              #$@(if allow-large-adjustment?
> +                                     '("-g")
> +                                     '()))))
>                (stop #~(make-kill-destructor))))))))
>  
>  (define %ntp-accounts
> @@ -331,10 +336,13 @@ restrict -6 ::1\n"))
>                                            ntp-service-activation)))))
>  
>  (define* (ntp-service #:key (ntp ntp)
> -                      (servers %ntp-servers))
> +                      (servers %ntp-servers)
> +                      allow-large-adjustment?)
>    "Return a service that runs the daemon from @var{ntp}, the
>  @uref{http://www.ntp.org, Network Time Protocol package}.  The daemon will
> -keep the system clock synchronized with that of @var{servers}."
> +keep the system clock synchronized with that of @var{servers}.
> +@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
> +make an initial adjustment of more than 1,000 seconds."
>    (service ntp-service-type
>             (ntp-configuration (ntp ntp) (servers servers))))
>  

> 
> If that’s fine with you, I’ll commit it with a doc update.

It looks good to me! Once we can run GuixSD on armhf or aarch64, I bet
we will have lots of users with hardware lacking a battery-backed real
time clock, and they will appreciate this option.
Ludovic Courtès - Oct. 20, 2016, 7:44 p.m.
Leo Famulari <leo@famulari.name> skribis:

> On Wed, Oct 19, 2016 at 10:49:32PM +0200, Ludovic Courtès wrote:
>> diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
>> index 7495179..ce281c5 100644
>> --- a/gnu/services/networking.scm
>> +++ b/gnu/services/networking.scm
>> @@ -265,11 +265,13 @@ Protocol (DHCP) client, on all the non-loopback network interfaces."
>>    ntp-configuration?
>>    (ntp      ntp-configuration-ntp
>>              (default ntp))
>> -  (servers  ntp-configuration-servers))
>> +  (servers  ntp-configuration-servers)
>> +  (allow-large-adjustment? ntp-allow-large-adjustment?
>> +                           (default #f)))
>>  
>>  (define ntp-shepherd-service
>>    (match-lambda
>> -    (($ <ntp-configuration> ntp servers)
>> +    (($ <ntp-configuration> ntp servers allow-large-adjustment?)
>>       (let ()
>>         ;; TODO: Add authentication support.
>>         (define config
>> @@ -296,7 +298,10 @@ restrict -6 ::1\n"))
>>                (requirement '(user-processes networking))
>>                (start #~(make-forkexec-constructor
>>                          (list (string-append #$ntp "/bin/ntpd") "-n"
>> -                              "-c" #$ntpd.conf "-u" "ntpd")))
>> +                              "-c" #$ntpd.conf "-u" "ntpd"
>> +                              #$@(if allow-large-adjustment?
>> +                                     '("-g")
>> +                                     '()))))
>>                (stop #~(make-kill-destructor))))))))
>>  
>>  (define %ntp-accounts
>> @@ -331,10 +336,13 @@ restrict -6 ::1\n"))
>>                                            ntp-service-activation)))))
>>  
>>  (define* (ntp-service #:key (ntp ntp)
>> -                      (servers %ntp-servers))
>> +                      (servers %ntp-servers)
>> +                      allow-large-adjustment?)
>>    "Return a service that runs the daemon from @var{ntp}, the
>>  @uref{http://www.ntp.org, Network Time Protocol package}.  The daemon will
>> -keep the system clock synchronized with that of @var{servers}."
>> +keep the system clock synchronized with that of @var{servers}.
>> +@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
>> +make an initial adjustment of more than 1,000 seconds."
>>    (service ntp-service-type
>>             (ntp-configuration (ntp ntp) (servers servers))))
>>  
>
>> 
>> If that’s fine with you, I’ll commit it with a doc update.
>
> It looks good to me! Once we can run GuixSD on armhf or aarch64, I bet
> we will have lots of users with hardware lacking a battery-backed real
> time clock, and they will appreciate this option.

Good point.  Pushed as dc0322b51111d12e1d97e2cc456100c44dd31bb6.

Ludo’.

Patch

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 7495179..ce281c5 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -265,11 +265,13 @@  Protocol (DHCP) client, on all the non-loopback network interfaces."
   ntp-configuration?
   (ntp      ntp-configuration-ntp
             (default ntp))
-  (servers  ntp-configuration-servers))
+  (servers  ntp-configuration-servers)
+  (allow-large-adjustment? ntp-allow-large-adjustment?
+                           (default #f)))
 
 (define ntp-shepherd-service
   (match-lambda
-    (($ <ntp-configuration> ntp servers)
+    (($ <ntp-configuration> ntp servers allow-large-adjustment?)
      (let ()
        ;; TODO: Add authentication support.
        (define config
@@ -296,7 +298,10 @@  restrict -6 ::1\n"))
               (requirement '(user-processes networking))
               (start #~(make-forkexec-constructor
                         (list (string-append #$ntp "/bin/ntpd") "-n"
-                              "-c" #$ntpd.conf "-u" "ntpd")))
+                              "-c" #$ntpd.conf "-u" "ntpd"
+                              #$@(if allow-large-adjustment?
+                                     '("-g")
+                                     '()))))
               (stop #~(make-kill-destructor))))))))
 
 (define %ntp-accounts
@@ -331,10 +336,13 @@  restrict -6 ::1\n"))
                                           ntp-service-activation)))))
 
 (define* (ntp-service #:key (ntp ntp)
-                      (servers %ntp-servers))
+                      (servers %ntp-servers)
+                      allow-large-adjustment?)
   "Return a service that runs the daemon from @var{ntp}, the
 @uref{http://www.ntp.org, Network Time Protocol package}.  The daemon will
-keep the system clock synchronized with that of @var{servers}."
+keep the system clock synchronized with that of @var{servers}.
+@var{allow-large-adjustment?} determines whether @command{ntpd} is allowed to
+make an initial adjustment of more than 1,000 seconds."
   (service ntp-service-type
            (ntp-configuration (ntp ntp) (servers servers))))