[PATCH 0/1 v2][meta-networking] ntp: fix path to drift file

[PATCH 0/1 v2][meta-networking] ntp: fix path to drift file

[wenzong.fan]

From: Wenzong Fan <wenzong.fan@windriver.com>

v2 changes:

Update the commit logs:
-----------------------
* fix the path to drift file:

The default path of ntp drift file is /etc/ntp.drift, ntp daemon
maybe fails to create this file since the user ntp is not always
permitted to write /etc.

Refer to other distributions such as RedHat, Debian, just moving
the file to /var/lib/ntp which the home dir of user ntp.

* add tmpfile support:

Make sure the /var/lib/ntp is always created by sysvinit/systemd.
------------------------

The following changes since commit a4bdcbdbca05bbb4a452e06982038f0a38bcb91f:

  openldap: use PN for PACKAGES_DYNAMIC (2014-10-10 12:47:35 +0200)

are available in the git repository at:

  git://git.pokylinux.org/poky-contrib wenzong/ntp-fix
  http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=wenzong/ntp-fix

Wenzong Fan (1):
  ntp: fix path to drift file

 meta-networking/recipes-support/ntp/ntp.inc |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

-- 
1.7.9.5

[PATCH 1/1 v2][meta-networking] ntp: fix path to drift file

[wenzong.fan]

From: Wenzong Fan <wenzong.fan@windriver.com>

* fix the path to drift file:

The default path of ntp drift file is /etc/ntp.drift, ntp daemon
maybe fails to create this file since the user ntp is not always
permitted to write /etc.

Refer to other distributions such as RedHat, Debian, just moving
the file to /var/lib/ntp which the home dir of user ntp.

* add tmpfile support:

Make sure the /var/lib/ntp is always created by sysvinit/systemd.

Signed-off-by: Wenzong Fan <wenzong.fan@windriver.com>
---
 meta-networking/recipes-support/ntp/ntp.inc |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/meta-networking/recipes-support/ntp/ntp.inc b/meta-networking/recipes-support/ntp/ntp.inc
index f55a39a..f685274 100644
--- a/meta-networking/recipes-support/ntp/ntp.inc
+++ b/meta-networking/recipes-support/ntp/ntp.inc
@@ -53,6 +53,7 @@ PACKAGECONFIG[debug] = "--enable-debugging,--disable-debugging"
 do_install_append() {
     install -d ${D}${sysconfdir}/init.d
     install -m 644 ${WORKDIR}/ntp.conf ${D}${sysconfdir}
+    sed -i 's!/etc/ntp.drift!/var/lib/ntp/drift!g' ${D}${sysconfdir}/ntp.conf
     install -m 755 ${WORKDIR}/ntpd ${D}${sysconfdir}/init.d
     install -d ${D}${bindir}
     install -m 755 ${WORKDIR}/ntpdate ${D}${bindir}/ntpdate-sync
@@ -72,6 +73,18 @@ do_install_append() {
     install -m 644 ${WORKDIR}/ntpdate.default ${D}${sysconfdir}/default/ntpdate
     install -m 0644 ${WORKDIR}/sntp ${D}${sysconfdir}/default/
 
+    # Create tmpfiles
+    if ${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'true', 'false', d)}; then
+        install -d ${D}/${sysconfdir}/default/volatiles
+        echo "d ntp ntp 0755 /var/lib/ntp none" \
+            > ${D}/${sysconfdir}/default/volatiles/99_ntpd
+    fi
+    if ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
+        install -d ${D}${sysconfdir}/tmpfiles.d
+        echo "d /var/lib/ntp 0755 ntp ntp -" \
+            > ${D}${sysconfdir}/tmpfiles.d/99-ntpd.conf
+    fi
+
     install -d ${D}/${sysconfdir}/network/if-up.d
     ln -s ${bindir}/ntpdate-sync ${D}/${sysconfdir}/network/if-up.d
 
@@ -112,6 +125,8 @@ RSUGGESTS_${PN} = "iana-etc"
 
 FILES_${PN} = "${sbindir}/ntpd ${sysconfdir}/ntp.conf ${sysconfdir}/init.d/ntpd ${libdir} \
     ${systemd_unitdir}/ntp-units.d/60-ntpd.list \
+    ${sysconfdir}/default/volatiles \
+    ${sysconfdir}/tmpfiles.d \
 "
 FILES_${PN}-tickadj = "${sbindir}/tickadj"
 FILES_${PN}-utils = "${sbindir}"
-- 
1.7.9.5

Re: [PATCH 1/1 v2][meta-networking] ntp: fix path to drift file

[Peter A. Bigot]

Thanks for the clarifications.  A couple more comments now that I 
understand the goal:

On 10/11/2014 05:12 AM, wenzong.fan@windriver.com wrote:
> From: Wenzong Fan <wenzong.fan@windriver.com>
>
> * fix the path to drift file:
>
> The default path of ntp drift file is /etc/ntp.drift, ntp daemon
> maybe fails to create this file since the user ntp is not always
> permitted to write /etc.
>
> Refer to other distributions such as RedHat, Debian, just moving
> the file to /var/lib/ntp which the home dir of user ntp.
>
> * add tmpfile support:
>
> Make sure the /var/lib/ntp is always created by sysvinit/systemd.
>
> Signed-off-by: Wenzong Fan <wenzong.fan@windriver.com>
> ---
>   meta-networking/recipes-support/ntp/ntp.inc |   15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/meta-networking/recipes-support/ntp/ntp.inc b/meta-networking/recipes-support/ntp/ntp.inc
> index f55a39a..f685274 100644
> --- a/meta-networking/recipes-support/ntp/ntp.inc
> +++ b/meta-networking/recipes-support/ntp/ntp.inc
> @@ -53,6 +53,7 @@ PACKAGECONFIG[debug] = "--enable-debugging,--disable-debugging"
>   do_install_append() {
>       install -d ${D}${sysconfdir}/init.d
>       install -m 644 ${WORKDIR}/ntp.conf ${D}${sysconfdir}
> +    sed -i 's!/etc/ntp.drift!/var/lib/ntp/drift!g' ${D}${sysconfdir}/ntp.conf

Since ntp.conf is provided by OE in files/ntp.conf, this change should 
be made there.  Other users may provide their own ntp.conf through a 
bbappend and will not want it modified during installation.

>       install -m 755 ${WORKDIR}/ntpd ${D}${sysconfdir}/init.d
>       install -d ${D}${bindir}
>       install -m 755 ${WORKDIR}/ntpdate ${D}${bindir}/ntpdate-sync
> @@ -72,6 +73,18 @@ do_install_append() {
>       install -m 644 ${WORKDIR}/ntpdate.default ${D}${sysconfdir}/default/ntpdate
>       install -m 0644 ${WORKDIR}/sntp ${D}${sysconfdir}/default/
>   
> +    # Create tmpfiles
> +    if ${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'true', 'false', d)}; then
> +        install -d ${D}/${sysconfdir}/default/volatiles
> +        echo "d ntp ntp 0755 /var/lib/ntp none" \
> +            > ${D}/${sysconfdir}/default/volatiles/99_ntpd
> +    fi
> +    if ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
> +        install -d ${D}${sysconfdir}/tmpfiles.d
> +        echo "d /var/lib/ntp 0755 ntp ntp -" \
> +            > ${D}${sysconfdir}/tmpfiles.d/99-ntpd.conf
> +    fi
> +

ntp.drift is not a tmpfile and should not be considered volatile (its 
absence will cause NTP to spend the first 15 minutes estimating local 
oscillator characteristics, which is generally not desirable in a time 
server).  /var/lib is intended for persistent state so putting the drift 
file there is perfectly fine, but adding /var/lib/ntp in 
tmpfiles/volatiles is incorrect.

Since /var/lib/ntp is the ntp user's home directory, surely it's created 
automatically anyway and nothing need be done.

Peter

>       install -d ${D}/${sysconfdir}/network/if-up.d
>       ln -s ${bindir}/ntpdate-sync ${D}/${sysconfdir}/network/if-up.d
>   
> @@ -112,6 +125,8 @@ RSUGGESTS_${PN} = "iana-etc"
>   
>   FILES_${PN} = "${sbindir}/ntpd ${sysconfdir}/ntp.conf ${sysconfdir}/init.d/ntpd ${libdir} \
>       ${systemd_unitdir}/ntp-units.d/60-ntpd.list \
> +    ${sysconfdir}/default/volatiles \
> +    ${sysconfdir}/tmpfiles.d \
>   "
>   FILES_${PN}-tickadj = "${sbindir}/tickadj"
>   FILES_${PN}-utils = "${sbindir}"

Re: [PATCH 1/1 v2][meta-networking] ntp: fix path to drift file

[Peter A. Bigot]

On 10/11/2014 06:42 AM, Peter A. Bigot wrote:
> Thanks for the clarifications.  A couple more comments now that I 
> understand the goal:
>
> On 10/11/2014 05:12 AM, wenzong.fan@windriver.com wrote:
>> From: Wenzong Fan <wenzong.fan@windriver.com>
>>
>> * fix the path to drift file:
>>
>> The default path of ntp drift file is /etc/ntp.drift, ntp daemon
>> maybe fails to create this file since the user ntp is not always
>> permitted to write /etc.
>>
>> Refer to other distributions such as RedHat, Debian, just moving
>> the file to /var/lib/ntp which the home dir of user ntp.
>>
>> * add tmpfile support:
>>
>> Make sure the /var/lib/ntp is always created by sysvinit/systemd.
>>
>> Signed-off-by: Wenzong Fan <wenzong.fan@windriver.com>
>> ---
>>   meta-networking/recipes-support/ntp/ntp.inc |   15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/meta-networking/recipes-support/ntp/ntp.inc 
>> b/meta-networking/recipes-support/ntp/ntp.inc
>> index f55a39a..f685274 100644
>> --- a/meta-networking/recipes-support/ntp/ntp.inc
>> +++ b/meta-networking/recipes-support/ntp/ntp.inc
>> @@ -53,6 +53,7 @@ PACKAGECONFIG[debug] = 
>> "--enable-debugging,--disable-debugging"
>>   do_install_append() {
>>       install -d ${D}${sysconfdir}/init.d
>>       install -m 644 ${WORKDIR}/ntp.conf ${D}${sysconfdir}
>> +    sed -i 's!/etc/ntp.drift!/var/lib/ntp/drift!g' 
>> ${D}${sysconfdir}/ntp.conf
>
> Since ntp.conf is provided by OE in files/ntp.conf, this change should 
> be made there.  Other users may provide their own ntp.conf through a 
> bbappend and will not want it modified during installation.
>
>>       install -m 755 ${WORKDIR}/ntpd ${D}${sysconfdir}/init.d
>>       install -d ${D}${bindir}
>>       install -m 755 ${WORKDIR}/ntpdate ${D}${bindir}/ntpdate-sync
>> @@ -72,6 +73,18 @@ do_install_append() {
>>       install -m 644 ${WORKDIR}/ntpdate.default 
>> ${D}${sysconfdir}/default/ntpdate
>>       install -m 0644 ${WORKDIR}/sntp ${D}${sysconfdir}/default/
>>   +    # Create tmpfiles
>> +    if ${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'true', 
>> 'false', d)}; then
>> +        install -d ${D}/${sysconfdir}/default/volatiles
>> +        echo "d ntp ntp 0755 /var/lib/ntp none" \
>> +            > ${D}/${sysconfdir}/default/volatiles/99_ntpd
>> +    fi
>> +    if ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'true', 
>> 'false', d)}; then
>> +        install -d ${D}${sysconfdir}/tmpfiles.d
>> +        echo "d /var/lib/ntp 0755 ntp ntp -" \
>> +            > ${D}${sysconfdir}/tmpfiles.d/99-ntpd.conf
>> +    fi
>> +
>
> ntp.drift is not a tmpfile and should not be considered volatile (its 
> absence will cause NTP to spend the first 15 minutes estimating local 
> oscillator characteristics, which is generally not desirable in a time 
> server).  /var/lib is intended for persistent state so putting the 
> drift file there is perfectly fine, but adding /var/lib/ntp in 
> tmpfiles/volatiles is incorrect.
>
> Since /var/lib/ntp is the ntp user's home directory, surely it's 
> created automatically anyway and nothing need be done.

I see that in fact it is not created automatically, even though the user 
is created.  Ignoring the more fundamental and non-ntp question "why 
not?", adding:

    install -d ${D}/var/lib/ntp

to do_install_append() should cover it.

Peter

>
> Peter
>
>>       install -d ${D}/${sysconfdir}/network/if-up.d
>>       ln -s ${bindir}/ntpdate-sync ${D}/${sysconfdir}/network/if-up.d
>>   @@ -112,6 +125,8 @@ RSUGGESTS_${PN} = "iana-etc"
>>     FILES_${PN} = "${sbindir}/ntpd ${sysconfdir}/ntp.conf 
>> ${sysconfdir}/init.d/ntpd ${libdir} \
>>       ${systemd_unitdir}/ntp-units.d/60-ntpd.list \
>> +    ${sysconfdir}/default/volatiles \
>> +    ${sysconfdir}/tmpfiles.d \
>>   "
>>   FILES_${PN}-tickadj = "${sbindir}/tickadj"
>>   FILES_${PN}-utils = "${sbindir}"
>

Re: [PATCH 1/1 v2][meta-networking] ntp: fix path to drift file

[Peter A. Bigot]

On 10/11/2014 08:53 AM, Peter A. Bigot wrote:
> On 10/11/2014 06:42 AM, Peter A. Bigot wrote:
>> Thanks for the clarifications.  A couple more comments now that I 
>> understand the goal:
>>
>> On 10/11/2014 05:12 AM, wenzong.fan@windriver.com wrote:
>>> From: Wenzong Fan <wenzong.fan@windriver.com>
>>>
>>> * fix the path to drift file:
>>>
>>> The default path of ntp drift file is /etc/ntp.drift, ntp daemon
>>> maybe fails to create this file since the user ntp is not always
>>> permitted to write /etc.
>>>
>>> Refer to other distributions such as RedHat, Debian, just moving
>>> the file to /var/lib/ntp which the home dir of user ntp.
>>>
>>> * add tmpfile support:
>>>
>>> Make sure the /var/lib/ntp is always created by sysvinit/systemd.
>>>
>>> Signed-off-by: Wenzong Fan <wenzong.fan@windriver.com>
>>> ---
>>>   meta-networking/recipes-support/ntp/ntp.inc |   15 +++++++++++++++
>>>   1 file changed, 15 insertions(+)
>>>
>>> diff --git a/meta-networking/recipes-support/ntp/ntp.inc 
>>> b/meta-networking/recipes-support/ntp/ntp.inc
>>> index f55a39a..f685274 100644
>>> --- a/meta-networking/recipes-support/ntp/ntp.inc
>>> +++ b/meta-networking/recipes-support/ntp/ntp.inc
>>> @@ -53,6 +53,7 @@ PACKAGECONFIG[debug] = 
>>> "--enable-debugging,--disable-debugging"
>>>   do_install_append() {
>>>       install -d ${D}${sysconfdir}/init.d
>>>       install -m 644 ${WORKDIR}/ntp.conf ${D}${sysconfdir}
>>> +    sed -i 's!/etc/ntp.drift!/var/lib/ntp/drift!g' 
>>> ${D}${sysconfdir}/ntp.conf
>>
>> Since ntp.conf is provided by OE in files/ntp.conf, this change 
>> should be made there.  Other users may provide their own ntp.conf 
>> through a bbappend and will not want it modified during installation.
>>
>>>       install -m 755 ${WORKDIR}/ntpd ${D}${sysconfdir}/init.d
>>>       install -d ${D}${bindir}
>>>       install -m 755 ${WORKDIR}/ntpdate ${D}${bindir}/ntpdate-sync
>>> @@ -72,6 +73,18 @@ do_install_append() {
>>>       install -m 644 ${WORKDIR}/ntpdate.default 
>>> ${D}${sysconfdir}/default/ntpdate
>>>       install -m 0644 ${WORKDIR}/sntp ${D}${sysconfdir}/default/
>>>   +    # Create tmpfiles
>>> +    if ${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'true', 
>>> 'false', d)}; then
>>> +        install -d ${D}/${sysconfdir}/default/volatiles
>>> +        echo "d ntp ntp 0755 /var/lib/ntp none" \
>>> +            > ${D}/${sysconfdir}/default/volatiles/99_ntpd
>>> +    fi
>>> +    if ${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 'true', 
>>> 'false', d)}; then
>>> +        install -d ${D}${sysconfdir}/tmpfiles.d
>>> +        echo "d /var/lib/ntp 0755 ntp ntp -" \
>>> +            > ${D}${sysconfdir}/tmpfiles.d/99-ntpd.conf
>>> +    fi
>>> +
>>
>> ntp.drift is not a tmpfile and should not be considered volatile (its 
>> absence will cause NTP to spend the first 15 minutes estimating local 
>> oscillator characteristics, which is generally not desirable in a 
>> time server).  /var/lib is intended for persistent state so putting 
>> the drift file there is perfectly fine, but adding /var/lib/ntp in 
>> tmpfiles/volatiles is incorrect.
>>
>> Since /var/lib/ntp is the ntp user's home directory, surely it's 
>> created automatically anyway and nothing need be done.
>
> I see that in fact it is not created automatically, even though the 
> user is created.  Ignoring the more fundamental and non-ntp question 
> "why not?", 

The "why not" is because that's the way useradd works.  Practically, 
since ntp is not a login account I believe --no-create-home should be 
added to USERADD_PARAM_${PN}.

> adding:
>
>    install -d ${D}/var/lib/ntp
>
> to do_install_append() should cover it.

And it does not.  Even when the missing chown ntp:ntp and packaging is 
added, the directory created when an image with ntp is built has the 
build host uid/gid.  Only when the ntp package is installed from within 
the target environment is the ownership correct.

As it stands using tmpfiles/volatiles might be the only working solution 
to get the correct ownership of home directories on the target, but IMO 
that should be fixed.  I've taken the issue to oe-core: 
http://lists.openembedded.org/pipermail/openembedded-core/2014-October/097895.html

Peter

>
> Peter
>
>>
>> Peter
>>
>>>       install -d ${D}/${sysconfdir}/network/if-up.d
>>>       ln -s ${bindir}/ntpdate-sync ${D}/${sysconfdir}/network/if-up.d
>>>   @@ -112,6 +125,8 @@ RSUGGESTS_${PN} = "iana-etc"
>>>     FILES_${PN} = "${sbindir}/ntpd ${sysconfdir}/ntp.conf 
>>> ${sysconfdir}/init.d/ntpd ${libdir} \
>>>       ${systemd_unitdir}/ntp-units.d/60-ntpd.list \
>>> +    ${sysconfdir}/default/volatiles \
>>> +    ${sysconfdir}/tmpfiles.d \
>>>   "
>>>   FILES_${PN}-tickadj = "${sbindir}/tickadj"
>>>   FILES_${PN}-utils = "${sbindir}"
>>
>