7:52 p.m.
Hi,
Could you please send your patches directly and not as attachment? I
have to copy the patch over to for the review. The simplest way is to
use 'git send-email' directly.
On Wed, Dec 23, 2020 at 03:19:30PM +0000, VAUTRIN Emmanuel (Canal Plus Prestataire)
wrote:
As our system depends on https requests, and for boot performance
purposes, we need to be informed when a time synchronization (via the
ntp) is performed.
In this aim, please find the attached patch, adding the TimeSynced
signal emitted when the system time has been synced.
From cf311e998c85b18703886b49bc98c06840612c22 Mon Sep 17 00:00:00
2001
From: Emmanuel VAUTRIN <Emmanuel.VAUTRIN(a)cpexterne.org>
Date: Wed, 23 Dec 2020 12:06:04 +0100
Subject: [PATCH] clock: Add TimeSynced signal emitted when the system time has
been synced.
Please add a commit message explaining what problem it solves.
---
doc/clock-api.txt | 4 ++++
include/dbus.h | 2 ++
src/clock.c | 2 ++
src/dbus.c | 30 ++++++++++++++++++++++++++++++
src/timeserver.c | 3 +++
5 files changed, 41 insertions(+)
diff --git a/doc/clock-api.txt b/doc/clock-api.txt
index 6818f5a8..2e368d0a 100644
--- a/doc/clock-api.txt
+++ b/doc/clock-api.txt
@@ -27,6 +27,10 @@ Signals PropertyChanged(string name, variant value) [experimental]
This signal indicates a changed value of the given
property.
+ TimeSynced(uint64 Time) [experimental]
+
+ This signal indicates that the current system time
+ has been synced.
I think it would make more sense to just emit a bool and send out the
Time event along side. In this case it would be the same event for auto
or manual mode. This would be more consistent in my opinion.
Also, TimeserverSynced would be also be better name for it in this case.
Properties uint64 Time [readonly or readwrite] [experimental]
diff --git a/include/dbus.h b/include/dbus.h
index bcab4189..3208adc5 100644
--- a/include/dbus.h
+++ b/include/dbus.h
@@ -85,6 +85,8 @@ dbus_bool_t connman_dbus_setting_changed_array(const char *owner,
const char *path, const char *key, int type,
connman_dbus_append_cb_t function,
void *user_data);
+dbus_bool_t connman_dbus_time_synced(const char *path,
+ const char *interface);
static inline void connman_dbus_dict_open(DBusMessageIter *iter,
DBusMessageIter *dict)
diff --git a/src/clock.c b/src/clock.c
index 0fde2c34..315e908e 100644
--- a/src/clock.c
+++ b/src/clock.c
@@ -381,6 +381,8 @@ static const GDBusMethodTable clock_methods[] = {
static const GDBusSignalTable clock_signals[] = {
{ GDBUS_SIGNAL("PropertyChanged",
GDBUS_ARGS({ "name", "s" }, { "value", "v"
})) },
+ { GDBUS_SIGNAL("TimeSynced",
+ GDBUS_ARGS({ "name", "s" }, { "value", "v"
})) },
{ },
};
diff --git a/src/dbus.c b/src/dbus.c
index d80a46ce..73d1bad6 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -23,6 +23,8 @@
#include <config.h>
#endif
+#include <sys/time.h>
+
#include <string.h>
#include <errno.h>
#include <gdbus.h>
@@ -382,6 +384,34 @@ dbus_bool_t connman_dbus_setting_changed_array(const char *owner,
return TRUE;
}
+dbus_bool_t connman_dbus_time_synced(const char *path, const char *interface)
+{
+ DBusMessage *signal;
+ DBusMessageIter iter;
+ struct timeval tv;
+ dbus_uint64_t val = 0;
+
+ if (!path)
+ return FALSE;
+
+ signal = dbus_message_new_signal(path, interface, "TimeSynced");
+ if (!signal)
+ return FALSE;
+
+ dbus_message_iter_init_append(signal, &iter);
+
+ if (gettimeofday(&tv, NULL) == 0) {
+ val = tv.tv_sec;
+ }
+
+ connman_dbus_property_append_basic(&iter, "Time",
+ DBUS_TYPE_UINT64, &val);
+
+ g_dbus_send_message(connection, signal);
+
+ return TRUE;
+}
+
dbus_bool_t __connman_dbus_append_objpath_dict_array(DBusMessage *msg,
connman_dbus_append_cb_t function, void *user_data)
{
diff --git a/src/timeserver.c b/src/timeserver.c
index decca153..ebd1fab3 100644
--- a/src/timeserver.c
+++ b/src/timeserver.c
@@ -57,6 +57,9 @@ static void ntp_callback(bool success, void *user_data)
if (!success)
sync_next();
+ else
+ connman_dbus_time_synced(CONNMAN_MANAGER_PATH,
+ CONNMAN_CLOCK_INTERFACE);
You should also add it to get_properties.
Daniel
10:22 a.m.
New subject: [PATCH] clock: Add TimeSynced signal emitted when the system time
has been synced
Hello Daniel,
Best wishes!
Could you please send your patches directly and not as attachment? I
have to copy the patch over to for the review.
Allright.
Here is my stabilized version of the patch, no regression detected by our automatical
tests:
------------------------------------------------------------------------------------------
From 613d5989226218806f1da82234926795e510dd2c Mon Sep 17 00:00:00 2001
From: Emmanuel VAUTRIN <Emmanuel.VAUTRIN(a)cpexterne.org>
Date: Thu, 7 Jan 2021 17:27:39 +0100
Subject: [PATCH] clock: Implement Clock API time synchronization
Implement Clock API TimeserverSynced property, which indicates if the
current system time is synced via NTP servers.
Trigger a time synchronization when the TimeUpdate property is set to
auto.
Fix time synchronization when starting with an empty Timeservers list.
Emit a Time PropertyChanged signal on NTP synchronization success.
---
doc/clock-api.txt | 5 +++++
src/clock.c | 15 +++++++++++++++
src/connman.h | 3 +++
src/timeserver.c | 41 ++++++++++++++++++++++++++++++++++++++++-
4 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/doc/clock-api.txt b/doc/clock-api.txt
index 6818f5a8..48d29970 100644
--- a/doc/clock-api.txt
+++ b/doc/clock-api.txt
@@ -85,3 +85,8 @@ Properties uint64 Time [readonly or readwrite] [experimental]
This list of servers is used when TimeUpdates is set
to auto.
+
+ boolean TimeserverSynced [readonly] [experimental]
+
+ This value indicates if the current system time
+ is synced via NTP servers.
diff --git a/src/clock.c b/src/clock.c
index 0fde2c34..906538ac 100644
--- a/src/clock.c
+++ b/src/clock.c
@@ -173,6 +173,7 @@ static DBusMessage *get_properties(DBusConnection *conn,
{
DBusMessage *reply;
DBusMessageIter array, dict;
+ dbus_bool_t is_synced;
struct timeval tv;
const char *str;
@@ -210,6 +211,10 @@ static DBusMessage *get_properties(DBusConnection *conn,
connman_dbus_dict_append_array(&dict, "Timeservers",
DBUS_TYPE_STRING, append_timeservers, NULL);
+ is_synced = __connman_timeserver_is_synced();
+ connman_dbus_dict_append_basic(&dict, "TimeserverSynced",
+ DBUS_TYPE_BOOLEAN, &is_synced);
+
connman_dbus_dict_close(&array, &dict);
return reply;
@@ -258,6 +263,7 @@ static DBusMessage *set_property(DBusConnection *conn,
if (settimeofday(&tv, NULL) < 0)
return __connman_error_invalid_arguments(msg);
+ __connman_timeserver_set_synced(false);
connman_dbus_property_changed_basic(CONNMAN_MANAGER_PATH,
CONNMAN_CLOCK_INTERFACE, "Time",
DBUS_TYPE_UINT64, &newval);
@@ -283,6 +289,13 @@ static DBusMessage *set_property(DBusConnection *conn,
connman_dbus_property_changed_basic(CONNMAN_MANAGER_PATH,
CONNMAN_CLOCK_INTERFACE, "TimeUpdates",
DBUS_TYPE_STRING, &strval);
+
+ if (newval == TIME_UPDATES_AUTO) {
+ struct connman_service *service;
+
+ service = connman_service_get_default();
+ __connman_timeserver_conf_update(service);
+ }
} else if (g_str_equal(name, "Timezone")) {
const char *strval;
@@ -362,6 +375,8 @@ static DBusMessage *set_property(DBusConnection *conn,
connman_dbus_property_changed_array(CONNMAN_MANAGER_PATH,
CONNMAN_CLOCK_INTERFACE, "Timeservers",
DBUS_TYPE_STRING, append_timeservers, NULL);
+ } else if (g_str_equal(name, "TimeserverSynced")) {
+ return __connman_error_permission_denied(msg);
} else
return __connman_error_invalid_property(msg);
diff --git a/src/connman.h b/src/connman.h
index e51d94d0..50d52bbe 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -452,6 +452,9 @@ GSList *__connman_timeserver_get_all(struct connman_service
*service);
void __connman_timeserver_sync(struct connman_service *service);
void __connman_timeserver_conf_update(struct connman_service *service);
+bool __connman_timeserver_is_synced(void);
+void __connman_timeserver_set_synced(bool status);
+
enum __connman_dhcpv6_status {
CONNMAN_DHCPV6_STATUS_FAIL = 0,
CONNMAN_DHCPV6_STATUS_SUCCEED = 1,
diff --git a/src/timeserver.c b/src/timeserver.c
index 5f0aca68..82208442 100644
--- a/src/timeserver.c
+++ b/src/timeserver.c
@@ -29,6 +29,7 @@
#include <stdlib.h>
#include <gweb/gresolv.h>
#include <netdb.h>
+#include <sys/time.h>
#include "connman.h"
@@ -40,6 +41,7 @@ static GSList *ts_list = NULL;
static char *ts_current = NULL;
static int ts_recheck_id = 0;
static int ts_backoff_id = 0;
+static bool ts_is_synced = false;
static GResolv *resolv = NULL;
static int resolv_id = 0;
@@ -55,8 +57,24 @@ static void ntp_callback(bool success, void *user_data)
{
DBG("success %d", success);
+ __connman_timeserver_set_synced(success);
if (!success)
sync_next();
+ else {
+ struct timeval tv;
+
+ if (gettimeofday(&tv, NULL) == 0) {
+ dbus_uint64_t timestamp;
+
+ timestamp = tv.tv_sec;
+ connman_dbus_property_changed_basic(
+ CONNMAN_MANAGER_PATH,
+ CONNMAN_CLOCK_INTERFACE, "Time",
+ DBUS_TYPE_UINT64, ×tamp);
+ } else {
+ connman_warn("Failed to get current time");
+ }
+ }
}
static void save_timeservers(char **servers)
@@ -337,6 +355,8 @@ static void ts_reset(struct connman_service *service)
if (!resolv)
return;
+ __connman_timeserver_set_synced(false);
+
/*
* Before we start creating the new timeserver list we must stop
* any ongoing ntp query and server resolution.
@@ -366,6 +386,8 @@ static void ts_reset(struct connman_service *service)
__connman_service_timeserver_changed(service, timeservers_list);
+ ts_service = service;
+
if (!timeservers_list) {
DBG("No timeservers set.");
return;
@@ -373,7 +395,6 @@ static void ts_reset(struct connman_service *service)
ts_recheck_enable();
- ts_service = service;
timeserver_sync_start();
}
@@ -394,6 +415,24 @@ void __connman_timeserver_conf_update(struct connman_service
*service)
}
+bool __connman_timeserver_is_synced(void)
+{
+ return ts_is_synced;
+}
+
+void __connman_timeserver_set_synced(bool status)
+{
+ if (ts_is_synced != status) {
+ dbus_bool_t is_synced;
+
+ ts_is_synced = status;
+ is_synced = status;
+ connman_dbus_property_changed_basic(CONNMAN_MANAGER_PATH,
+ CONNMAN_CLOCK_INTERFACE, "TimeserverSynced",
+ DBUS_TYPE_BOOLEAN, &is_synced);
+ }
+}
+
static int timeserver_start(struct connman_service *service)
{
char **nameservers;
--
2.25.1
------------------------------------------------------------------------------------------
B.R.
Emmanuel
376
days inactive
391
days old
1 comments
2 participants
participants (2)
-
Daniel Wagner -
VAUTRIN Emmanuel (Canal Plus Prestataire)