Re: [systemd-devel] [PATCH] Add timesync-wait tool
On Wed, 29.10.14 17:50, Lukasz Stelmach (stl...@poczta.fm) wrote: I mean, what is the tool supposed to be waiting on: that the time is set as accurately as possible (in that case watching STA_UNSYNC sounds good, plus waiting for TFD_CANCEL_ON_SET for big jumps and guessing sleep times from adjtimex()'s return values for smaller jumps)? There is no point in guessing. Clearing STA_UNSYNC takes one timesynced cycle (32s, see below). Well, the algorithm I was envisioning there would be one which wouldn#t strictly be tied to timesycnd being the NTP server. By checking what adjtimex returns I figure we could guess when the fix is likely to be done. Or that the time is set accurately enough for adjtimex() to be used for the remaining accuracy (In that case, we'd actually have to make timesyncd report this information to us, maybe using a flag file to watch via inotify)? This is probably what satisfies most users. Or that the time set accurately enough to be monotonic, but not more (in that case just ordering after systemd-timesyncd.service should be enough, no need for any other tool)? timesyncd sends READY=1 quite early. I think it is before it tries to contact time servers. Yes. And that's really the right thing to do. In normal setups we really should not wait for external conditions. Of these three options, I think the first one is not necessarily a good idea, since adjtimex() is really about making time corrections smooth and hence slow. Making this slow, and trying to wait for it is kinda contradictory, no? Not really, that is it isn't that slow. I measured it. First I set the current time off by less than a minute then waited date $(date +%m%d%H%M%Y) sleep 2 time ./systemd-timesync-wait and it always took one timesyncd cycle (32 seconds) to clear STA_UNSYNC. How about waiting for either a flag-file which timesyncd creates after a few seconds timesynced is started or, if a user chooses so, for the real synchronisation? The third one is not a good idea either, since we already have functionality covering that. But if the second option is the relevant one, then I figure neither adjtimex() nor TFP_CANCEL_ON_SET will be useful to us, and instead we need to teach systemd-timesyncd some flag file stuff. Yes, like hey I've already received the time from an NTP server and fed it to the kernel. Create /run/systemd/timesyncd/synchronised here: http://cgit.freedesktop.org/systemd/systemd/tree/src/timesync/timesyncd-manager.c?id=v217#n389 Correct. and unlink it here http://cgit.freedesktop.org/systemd/systemd/tree/src/timesync/timesyncd-manager.c?id=v217#n293 Sure? (see below) and of upon exit. An erroneous one too. Hmm, why remove it on exit or on time change? I mean, I figure this really should be a one-time thing: a way how the tool can wait until the first sync is acquired, and that's it. The same way as network-wait-online has a timeout this tool should probably have one too. If the timeout is reached the tool exits with a non-zero code. Right? That makes sense. Is three minutes OK? I'd use the same default timeout as for systemd-network-wait-online, for whatever that is. Are you sure there is a timeout? The service type is oneshot (timeout is disabled according to systemd.service(5)) without any Timeout* set. Two calls to sd_event_exit() in the code depend manager_all_configured() and no sign of any timer. Indeed. It really should have one though. Added to TODO list now. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Add timesync-wait tool
On 27.10.2014 15:12, Lennart Poettering wrote:
On Fri, 24.10.14 23:13, Lukasz Stelmach (stl...@poczta.fm) wrote:
On 24.10.2014 00:28, Lennart Poettering wrote:
On Thu, 23.10.14 21:24, Łukasz Stelmach (stl...@poczta.fm) wrote:
+int main(int argc, char *argv[]) {
+struct timex tbuf;
+int r;
+
+memset(tbuf, 0, sizeof(tbuf));
Please initialize this with = {} while declaring, instead of using
memset() here.
+r = adjtimex(tbuf);
+
+while (r != TIME_OK) {
This check looks wrong. Should check for tbuf.status STA_UNSYNC, no?
Also, we already have the ntp_synced() call for doing this.
Indeed. I can replace most of the code here with ntp_synced() leaving an
if with a break and the sleep();
+sleep(1);
+/* Unfortunately there seem to be no other way than
+polling to get this information. */
+memset(tbuf, 0, sizeof(tbuf));
In this case, use zero(), it's nicer, simpler and less error prone.
+r = adjtimex(tbuf);
+}
Implementing this with a sleep loop is really ugly. Can't we at least
calculate the expected sync time from the data returned by adjtimex()?
I don't know how to do it exactly (yet). But my guess is that when the
system starts the information you refer are not good enough to predict
anything?
Hmm, thinking about this some more: is STA_UNSYNC actually really what
we should be looking for?
I can image users who would.
I mean, what is the tool supposed to be
waiting on: that the time is set as accurately as possible (in that
case watching STA_UNSYNC sounds good, plus waiting for
TFD_CANCEL_ON_SET for big jumps and guessing sleep times from
adjtimex()'s return values for smaller jumps)?
There is no point in guessing. Clearing STA_UNSYNC takes one
timesynced cycle (32s, see below).
Or that the time is set
accurately enough for adjtimex() to be used for the remaining accuracy
(In that case, we'd actually have to make timesyncd report this
information to us, maybe using a flag file to watch via inotify)?
This is probably what satisfies most users.
Or
that the time set accurately enough to be monotonic, but not more (in
that case just ordering after systemd-timesyncd.service should be
enough, no need for any other tool)?
timesyncd sends READY=1 quite early. I think it is before it tries to
contact time servers.
Of these three options, I think the first one is not necessarily a
good idea, since adjtimex() is really about making time corrections
smooth and hence slow. Making this slow, and trying to wait for it is
kinda contradictory, no?
Not really, that is it isn't that slow. I measured it. First I set the
current time off by less than a minute then waited
date $(date +%m%d%H%M%Y) sleep 2 time ./systemd-timesync-wait
and it always took one timesyncd cycle (32 seconds) to clear STA_UNSYNC.
How about waiting for either a flag-file which timesyncd creates after
a few seconds timesynced is started or, if a user chooses so, for the
real synchronisation?
The third one is not a good idea either, since we already have
functionality covering that.
But if the second option is the relevant one, then I figure neither
adjtimex() nor TFP_CANCEL_ON_SET will be useful to us, and instead we
need to teach systemd-timesyncd some flag file stuff.
Yes, like hey I've already received the time from an NTP server and fed
it to the kernel.
Create /run/systemd/timesyncd/synchronised here:
http://cgit.freedesktop.org/systemd/systemd/tree/src/timesync/timesyncd-manager.c?id=v217#n389
and unlink it here
http://cgit.freedesktop.org/systemd/systemd/tree/src/timesync/timesyncd-manager.c?id=v217#n293
and of upon exit. An erroneous one too.
The same way as network-wait-online has a timeout this tool should
probably have one too.
If the timeout is reached the tool exits with a non-zero code. Right?
That makes sense. Is three minutes OK?
I'd use the same default timeout as for systemd-network-wait-online,
for whatever that is.
Are you sure there is a timeout? The service type is oneshot (timeout is
disabled according to systemd.service(5)) without any Timeout* set. Two
calls to sd_event_exit() in the code depend manager_all_configured() and
no sign of any timer.
--
Było mi bardzo miło. Twoje oczy lubią mnie
Łukasz i to mnie zgubi (c)SNL
REKLAMA: http://ars-fabrica.eu/ sklep z rękodziełem
signature.asc
Description: OpenPGP digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Add timesync-wait tool
On Fri, 24.10.14 23:13, Lukasz Stelmach (stl...@poczta.fm) wrote:
On 24.10.2014 00:28, Lennart Poettering wrote:
On Thu, 23.10.14 21:24, Łukasz Stelmach (stl...@poczta.fm) wrote:
+int main(int argc, char *argv[]) {
+struct timex tbuf;
+int r;
+
+memset(tbuf, 0, sizeof(tbuf));
Please initialize this with = {} while declaring, instead of using
memset() here.
+r = adjtimex(tbuf);
+
+while (r != TIME_OK) {
This check looks wrong. Should check for tbuf.status STA_UNSYNC, no?
Also, we already have the ntp_synced() call for doing this.
Indeed. I can replace most of the code here with ntp_synced() leaving an
if with a break and the sleep();
+sleep(1);
+/* Unfortunately there seem to be no other way than
+polling to get this information. */
+memset(tbuf, 0, sizeof(tbuf));
In this case, use zero(), it's nicer, simpler and less error prone.
+r = adjtimex(tbuf);
+}
Implementing this with a sleep loop is really ugly. Can't we at least
calculate the expected sync time from the data returned by adjtimex()?
I don't know how to do it exactly (yet). But my guess is that when the
system starts the information you refer are not good enough to predict
anything?
Hmm, thinking about this some more: is STA_UNSYNC actually really what
we should be looking for? I mean, what is the tool supposed to be
waiting on: that the time is set as accurately as possible (in that
case watching STA_UNSYNC sounds good, plus waiting for
TFD_CANCEL_ON_SET for big jumps and guessing sleep times from
adjtimex()'s return values for smaller jumps)? Or that the time is set
accurately enough for adjtimex() to be used for the remaining accuracy
(In that case, we'd actually have to make timesyncd report this
information to us, maybe using a flag file to watch via inotify)? Or
that the time set accurately enough to be monotonic, but not more (in
that case just ordering after systemd-timesyncd.service should be
enough, no need for any other tool)?
Of these three options, I think the first one is not necessarily a
good idea, since adjtimex() is really about making time corrections
smooth and hence slow. Making this slow, and trying to wait for it is
kinda contradictory, no?
The third one is not a good idea either, since we already have
functionality covering that.
But if the second option is the relevant one, then I figure neither
adjtimex() nor TFP_CANCEL_ON_SET will be useful to us, and instead we
need to teach systemd-timesyncd some flag file stuff.
The same way as network-wait-online has a timeout this tool should
probably have one too.
If the timeout is reached the tool exits with a non-zero code. Right?
That makes sense. Is three minutes OK?
I'd use the same default timeout as for systemd-network-wait-online,
for whatever that is.
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Add timesync-wait tool
On 24.10.2014 00:28, Lennart Poettering wrote:
On Thu, 23.10.14 21:24, Łukasz Stelmach (stl...@poczta.fm) wrote:
+int main(int argc, char *argv[]) {
+struct timex tbuf;
+int r;
+
+memset(tbuf, 0, sizeof(tbuf));
Please initialize this with = {} while declaring, instead of using
memset() here.
+r = adjtimex(tbuf);
+
+while (r != TIME_OK) {
This check looks wrong. Should check for tbuf.status STA_UNSYNC, no?
Also, we already have the ntp_synced() call for doing this.
Indeed. I can replace most of the code here with ntp_synced() leaving an
if with a break and the sleep();
+sleep(1);
+/* Unfortunately there seem to be no other way than
+polling to get this information. */
+memset(tbuf, 0, sizeof(tbuf));
In this case, use zero(), it's nicer, simpler and less error prone.
+r = adjtimex(tbuf);
+}
Implementing this with a sleep loop is really ugly. Can't we at least
calculate the expected sync time from the data returned by adjtimex()?
I don't know how to do it exactly (yet). But my guess is that when the
system starts the information you refer are not good enough to predict
anything?
The same way as network-wait-online has a timeout this tool should
probably have one too.
If the timeout is reached the tool exits with a non-zero code. Right?
That makes sense. Is three minutes OK?
Give me a few more evenings to prepare the niceties others have requested.
--
Było mi bardzo miło. Twoje oczy lubią mnie
Łukasz i to mnie zgubi (c)SNL
REKLAMA: http://ars-fabrica.eu/ sklep z rękodziełem
signature.asc
Description: OpenPGP digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Add timesync-wait tool
On Thu, Oct 23, 2014 at 09:24:06PM +0200, Łukasz Stelmach wrote:
---
src/timesync/timesync-wait.c | 43 +++
1 file changed, 43 insertions(+)
create mode 100644 src/timesync/timesync-wait.c
I am afraid TFD_TIMER_CANCEL_ON_SET doesn't help much here. You can
watch for time changes but it is not the moment adjtimex() starts to
return TIME_OK and STA_UNSYNC flag unset.
diff --git a/src/timesync/timesync-wait.c b/src/timesync/timesync-wait.c
new file mode 100644
index 000..9648b09
--- /dev/null
+++ b/src/timesync/timesync-wait.c
@@ -0,0 +1,43 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2014 Łukasz Stelmach
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+#include string.h
+#include sys/timex.h
+#include unistd.h
+#include stdio.h
+
+int main(int argc, char *argv[]) {
+struct timex tbuf;
+int r;
Please check that argc == 1.
Can you rearrange the code to initialize tbuf with {}, and also
have just one adjtimex call?
+
+memset(tbuf, 0, sizeof(tbuf));
+r = adjtimex(tbuf);
+
+while (r != TIME_OK) {
+sleep(1);
+/* Unfortunately there seem to be no other way than
+polling to get this information. */
+memset(tbuf, 0, sizeof(tbuf));
+r = adjtimex(tbuf);
+}
+
+return 0;
Where would you like this to be patched in?
Please make it similar to systemd-networkd-wait-online.service.
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Add timesync-wait tool
On Thu, Oct 23, 2014 at 9:24 PM, Łukasz Stelmach stl...@poczta.fm wrote:
---
src/timesync/timesync-wait.c | 43 +++
1 file changed, 43 insertions(+)
create mode 100644 src/timesync/timesync-wait.c
I am afraid TFD_TIMER_CANCEL_ON_SET doesn't help much here. You can
watch for time changes but it is not the moment adjtimex() starts to
return TIME_OK and STA_UNSYNC flag unset.
Where would you like this to be patched in?
diff --git a/src/timesync/timesync-wait.c b/src/timesync/timesync-wait.c
new file mode 100644
index 000..9648b09
--- /dev/null
+++ b/src/timesync/timesync-wait.c
@@ -0,0 +1,43 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2014 Łukasz Stelmach
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+#include string.h
+#include sys/timex.h
+#include unistd.h
+#include stdio.h
+
+int main(int argc, char *argv[]) {
+struct timex tbuf;
+int r;
+
+memset(tbuf, 0, sizeof(tbuf));
+r = adjtimex(tbuf);
+
+while (r != TIME_OK) {
+sleep(1);
+/* Unfortunately there seem to be no other way than
Ugh, this is not really nice. If there is no other way, should we
rather ask the kernel guys to add an API for this?
+polling to get this information. */
+memset(tbuf, 0, sizeof(tbuf));
+r = adjtimex(tbuf);
+}
+
+return 0;
+}
--
2.0.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Add timesync-wait tool
On Thu, 23.10.14 21:24, Łukasz Stelmach (stl...@poczta.fm) wrote:
+int main(int argc, char *argv[]) {
+struct timex tbuf;
+int r;
+
+memset(tbuf, 0, sizeof(tbuf));
Please initialize this with = {} while declaring, instead of using
memset() here.
+r = adjtimex(tbuf);
+
+while (r != TIME_OK) {
This check looks wrong. Should check for tbuf.status STA_UNSYNC, no?
Also, we already have the ntp_synced() call for doing this.
+sleep(1);
+/* Unfortunately there seem to be no other way than
+polling to get this information. */
+memset(tbuf, 0, sizeof(tbuf));
In this case, use zero(), it's nicer, simpler and less error prone.
+r = adjtimex(tbuf);
+}
Implementing this with a sleep loop is really ugly. Can't we at least
calculate the expected sync time from the data returned by adjtimex()?
The same way as network-wait-online has a timeout this tool should
probably have one too.
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Add timesync-wait tool
2014-10-23 21:24 GMT+02:00 Łukasz Stelmach stl...@poczta.fm:
---
src/timesync/timesync-wait.c | 43 +++
1 file changed, 43 insertions(+)
create mode 100644 src/timesync/timesync-wait.c
I am afraid TFD_TIMER_CANCEL_ON_SET doesn't help much here. You can
watch for time changes but it is not the moment adjtimex() starts to
return TIME_OK and STA_UNSYNC flag unset.
Where would you like this to be patched in?
diff --git a/src/timesync/timesync-wait.c b/src/timesync/timesync-wait.c
new file mode 100644
index 000..9648b09
--- /dev/null
+++ b/src/timesync/timesync-wait.c
@@ -0,0 +1,43 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2014 Łukasz Stelmach
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+#include string.h
+#include sys/timex.h
+#include unistd.h
+#include stdio.h
+
+int main(int argc, char *argv[]) {
You should add a minimal parse_argv() like
systemd-networkd-wait-online in order to handle --help and --version,
like all systemd binaries.
+struct timex tbuf;
+int r;
+
+memset(tbuf, 0, sizeof(tbuf));
+r = adjtimex(tbuf);
+
+while (r != TIME_OK) {
+sleep(1);
+/* Unfortunately there seem to be no other way than
+polling to get this information. */
+memset(tbuf, 0, sizeof(tbuf));
+r = adjtimex(tbuf);
+}
+
+return 0;
+}
--
2.0.4
You should add the .service and a minimal man-page for this tool.
Also, you should add a little summary in the commit message explaining
why this is needed and/or a link to the discussion:
http://lists.freedesktop.org/archives/systemd-devel/2014-October/024346.html
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
