Add host.get_ntp_synchronized and host.set_server_localtime #6743

minglumlu · 2025-11-05T06:25:30Z

host.get_ntp_synchronized:
Simply return true or false by parsing "System clock synchronized" or "NTP synchronized" from the output of "timedatectl status" like the following, no matter the NTP being enabled or not.

# timedatectl status
               Local time: Wed 2025-11-05 06:10:42 UTC
           Universal time: Wed 2025-11-05 06:10:42 UTC
                 RTC time: Wed 2025-11-05 05:00:11
                Time zone: UTC (UTC, +0000)
System clock synchronized: yes
              NTP service: active
          RTC in local TZ: no

host.set_server_localtime:
Use "timedatectl set-time" to set local time on the host in its local timezone only when NTP is disabled. The value is of DateTime and can be serialized as something like "20251105T05:00:00".

Signed-off-by: Ming Lu <[email protected]>

changlei-li · 2025-11-05T06:38:56Z

Can you paste some examples and their outputs when call the APIs?

changlei-li · 2025-11-05T06:57:26Z

ocaml/xapi/xapi_host.ml

+        Printf.sprintf "%04i-%02i-%02i %02i:%02i:%02i" y mon d h min s
+    | _ ->
+        raise
+          (Api_errors.Server_error (Api_errors.not_allowed_tz_in_localtime, []))


I don't quite understand the timezone handlling here. I expected the time format "YYYY-MM-DD HH:MM:SS" which is localtime. It should have no business with timezone. Where does the timezone come from? The DateTime type?

Yes. The DateTime contains timezone.

Then why can't distinguish the local time and UTC.
20251105T16:11:55 is local time without tz,
20251105T08:11:55Z is UTC time with tz=0,
We need the tz = None branch is enough, isn't it right?

The API uses best_effort_iso8601_to_rfc3339 to assign a UTC if the datetime argument doesn't contain any timezone information. This happens in deserialization automatically.

minglumlu · 2025-11-05T07:16:33Z

Can you paste some examples and their outputs when call the APIs?

print JSON return in Python:

host.get_ntp_synchronized:

{'id': 'mingl dev test', 'jsonrpc': '2.0', 'result': False}

host.set_server_localtime :

{'id': 'mingl dev test', 'jsonrpc': '2.0', 'result': ''}

or

{'error': {'code': 1, 'data': [], 'message': 'NOT_ALLOWED_TZ_IN_LOCALTIME'},
 'id': 'mingl dev test',
 'jsonrpc': '2.0'}

changlei-li · 2025-11-05T09:22:23Z

ocaml/xapi/xapi_host_ntp.ml

+let is_synchronized () =
+  let patterns = ["System clock synchronized: yes"; "NTP synchronized: yes"] in
+  try
+    Helpers.call_script !Xapi_globs.timedatectl ["status"]


Another option is to use c function ntp_gettime to get the maxerror of the system. That can avoid the timedatectl output changes in different versions. Although the current method is easy enough too. I'm OK for both.

psafont · 2025-11-05T10:00:02Z

ocaml/idl/datamodel_host.ml

+        , "A datetime without timezone information. If UTC is specified, the \
+           timezone will be ignored."
+        )


There's not enough information in the commit message to understand the requirements of set_server_localtime. Please explain your decisions in there.

This is the writer function for localtime, which has an existing reader function host.get_server_localtime. So it is a datetime without timezone as the timezone is managed via other APIs.
For example, a local time + local timeznoe is 2025-11-05T11:00:00+08:00, the datetime here is just 2025-11-05T11:00:00. The timezone +08:00 will be managed separately.

get_server_locatime's way of serving information is outright dangerous, please treat it as a deprecated function that should not be used. I'd rather have a new function to get the host's server time to accompany the new function being introduced.

In particular the function is dangerous because it does not show the timezone, a correct function must return the time in this format: 2025-11-05T11:00:00+08:00.

I raise the the issue about lack of requirements is there because we need to know what the user needs to do, exactly. For example:

Set the correct time (only)

Set the timezone so the server is able to change the time difference when the timezone tables say so.

Note that the second one means that the timezone data needs to be packaged and updated periodically, which I don't think xenserver nor xcp-ng do.

My recommendation would be to avoid letting users setting the timezone for the server. UTC is more than enough and avoid complexity.

Fortunately the time can be set always correctly, even if the user provides a timestamp with a timezone difference like +07:45, so I would stick with that.

avoid letting users setting the timezone for the server.

I'm afraid this is a valid request and use case which can't be avoided.

I'd rather have a new function to get the host's server time to accompany the new function being introduced.

Generally I agree to set the time + timezone together always. But the existing two APIs:
host.get_servertime which returns time + UTC, and
host.get_server_localtime which returns time only,
make me feel that I should be careful on introducing new function. I would prefer to let the new setter function paired with one of two above, instead of introducing a new getter function.

Hmm. I think I can change the implementation of host.get_servertime to make it return time + timezone. The doc of the API doesn't say it will always return UTC although it does currently.
So the setter API could be named as host.set_servertime and accept time + timezone.

@psafont May I please know your thoughts on above proposal?

Hmm. I think I can change the implementation of host.get_servertime to make it return time + timezone. The doc of the API doesn't say it will always return UTC although it does currently.
So the setter API could be named as host.set_servertime and accept time + timezone.

This is what I would prefer as well.

I probably would also add two other timezone-related functions:

list available timezones

set a timezone

I have to repeat that I would prefer if the server was always in UTC, it can disrupts timestamps in logs and other debug information, which can be confusing. I have not found a real need to set a concrete timezone, this is because API clients can convert times to the timezone of the machine the user is using.

This is what I would prefer as well.

Thanks.

I have to repeat that I would prefer if the server was always in UTC, ..., I have not found a real need to set a concrete timezone, this is because API clients can convert time to the timezone of the machine the user is using.

There are real user's requests for setting timezone on servers. And a user who doesn't want to set timezone can ignore the new setting APIs.

it can disrupts timestamps in logs and other debug information, which can be confusing.

IMO, this is the unavoidable consequence of the user's action of setting timezone.

psafont · 2025-11-05T10:05:52Z

ocaml/idl/datamodel_host.ml

+let set_server_localtime =
+  call ~name:"set_server_localtime" ~lifecycle:[]
+    ~doc:
+      "Set the host's system clock in its local timezone when NTP is disabled."


Using localtime is confusing because it's used for a very specific thing in UNIX: time + timezone, but this call is _not_setting the timezone. Please use something that doesn't use the term localtime, for example:

Suggested change

let set_server_localtime =

call ~name:"set_server_localtime" ~lifecycle:[]

~doc:

"Set the host's system clock in its local timezone when NTP is disabled."

call ~name:"set_time" ~lifecycle:[]

~doc:

"Set the host's system clock, errors out when NTP is enabled."

There will be timezone management in the following PR, see the feature branch name.
The ntp can be disabled, so offer an api to set host local time.

This is exactly the writer regarding the existing reader: host.get_server_localtime, which returning only time without timezone.
I would think it should be used together with host.set_timezone.

There will be timezone management in the following PR, see the feature branch name.
The ntp can be disabled, so offer an api to set host local time.

This is a feature and it's missing a design a requirements, we've talked about this before :(

OK. I will push the feature user cases and design later.

changlei-li

There is existing api host.get_server_localtime and host.get_servertime. You may complete your test examples to help others know the function works well.

psafont · 2025-11-05T10:16:37Z

ocaml/xapi/xapi_host.ml

+  let param =
+    match value with
+    | Date.{t; tz= Some 0} | Date.{t; tz= None} ->
+        (* Ideally it should be of a new type like NaiveDateTime. For
+           simplicity, reuse DateTime here. But it can't tell if the UTC is
+           specified explicitly or not. Just ignore it in that case. *)
+        let (y, mon, d), ((h, min, s), _) = Ptime.to_date_time t in
+        Printf.sprintf "%04i-%02i-%02i %02i:%02i:%02i" y mon d h min s
+    | _ ->
+        raise
+          (Api_errors.Server_error (Api_errors.not_allowed_tz_in_localtime, []))


Why is all this code needed? Reading the manual of timedatectl, it should be able to accept any date in RFC3339 format, see man 7 systemd.time.

Additionally, datetimes without a timezone reference must be refused, and not assumed to be UTC. We've had this issue in xapi for a very long long time, I don't want to perpetuate it.

But we hope to accept a local time without timezone as there is another API to set the timezone.

But we hope to accept a local time without timezone as there is another API to set the timezone.

That API is old and outright dangerous, please treat it as deprecated. A new, sane API function should be exposed.

Maybe better not reuse the datetime type but just string that fit The timestamp may be specified in the format "2012-10-30 18:17:16 in man timedatectl. While later the host.timezone in xapi db will be IANA Timezone format. It's just like the xsconsole's set-timezone and set-time-manually and also most time setting logic in UI.

changlei-li

I push the doc on #6745. @psafont We may have a full view and discuss there and back to this PR again.

Need further discuss about the localtime format

minglumlu added 2 commits November 4, 2025 16:58

CP-308856 host.get_ntp_synchronized

105e2e1

Signed-off-by: Ming Lu <[email protected]>

CP-308855: host.set_server_localtime

4e4ee67

Signed-off-by: Ming Lu <[email protected]>

changlei-li reviewed Nov 5, 2025

View reviewed changes

psafont reviewed Nov 5, 2025

View reviewed changes

changlei-li previously approved these changes Nov 5, 2025

View reviewed changes

psafont requested changes Nov 5, 2025

View reviewed changes

changlei-li reviewed Nov 6, 2025

View reviewed changes

Add host.get_ntp_synchronized and host.set_server_localtime #6743

Are you sure you want to change the base?

Add host.get_ntp_synchronized and host.set_server_localtime #6743

Uh oh!

Conversation

minglumlu commented Nov 5, 2025

Uh oh!

changlei-li commented Nov 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

changlei-li Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

minglumlu Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

minglumlu commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

psafont Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

minglumlu Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

minglumlu Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

minglumlu Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

psafont Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

changlei-li left a comment

Choose a reason for hiding this comment

Uh oh!

psafont Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

changlei-li left a comment

Choose a reason for hiding this comment

Uh oh!

changlei-li Nov 5, 2025 •

edited

Loading

minglumlu Nov 5, 2025 •

edited

Loading

minglumlu commented Nov 5, 2025 •

edited

Loading

psafont Nov 5, 2025 •

edited

Loading

minglumlu Nov 5, 2025 •

edited

Loading

minglumlu Nov 5, 2025 •

edited

Loading

minglumlu Nov 6, 2025 •

edited

Loading

psafont Nov 5, 2025 •

edited

Loading

psafont Nov 5, 2025 •

edited

Loading