Thanks for the patches, and your continued effort!

Changes look good to me in principle. 
Gave the series a test-run on a PVE 9 and a PMG 9 - it works as advertised.

comments inline:
On Sun, 25 Jan 2026 22:01:47 +0100
Stefan Mayr <[email protected]> wrote:

> Second try to fix #7175. These patches remove direct /etc/timezone
> handling, add timezone functions to the Systemd module and replaces all
> uses of the functions you pointed me to.
> 
> This time it is with a verification of valid timezones.
> 
> It does not address hardcoded timezones in the javascript for the UI or
> the rust code handling timezones.
Replacing the hardcoded list in our frontend-code with a call to an
API-method, which wraps your PVE::Systemd::list_timezones seems like a good
further improvement.

Regarding the rust side of our code and products - from a quick check
those do fall-back to `/etc/localtime` if `/etc/timezone` is not available:
PDM:
https://git.proxmox.com/?p=proxmox-datacenter-manager.git;a=blob;f=server/src/api/nodes/time.rs;h=7f711c95ecd0b810fda360212d1c38a77093c920;hb=d5bfa9b8fa429cfed57dc9a1528e7662ff4c787f
uses proxmox-time-api:
https://git.proxmox.com/?p=proxmox.git;a=blob;f=proxmox-time-api/src/time_impl.rs;h=383bae1168eaab2d503c8da609c532d0974bc0b9;hb=HEAD
(although writing /etc/timezone could also be done only conditionally (on
its existence), and dropped with the next debian-release[0].)
PBS has its own implementation:
https://git.proxmox.com/?p=proxmox-backup.git;a=blob;f=src/api2/node/time.rs;h=d7ca10e31a14332b384518a03b270a7706b1538e;hb=HEAD
(which looks equal to the one in proxmox-time api and could also be
replaced by that from a quick look).

But none of the suggested future improvements are needed for this - so they
can be done independently (by you, if you're interested, else someone here
can take that part).

Splitting the pve-common patch - one introducing the new helpers (and
adding a comment that the ones in PVE::INotify are deprecated), and a
second one actually dropping the old ones, would make this easier to
roll-out (no need to roll out all call-sites (pve-container, pve-manager,
pmg-api) at the same time, to each of our repositories).

apart from the splitting of the patch for pve-common, consider this:
Tested-by: Stoiko Ivanov <[email protected]>
Reviewed-by: Stoiko Ivanov <[email protected]>


> 
> Trying to provide patches to Proxmox is the first time I have to use
> an email based patch workflow. I'm pretty sure sending these patches for
> four different repos the way I do it currently is not 100% how it is
> intended to be done. So please forgive me.
> 
> Suggested-by: Fabian Grünbichler <[email protected]>
> Suggested-by: Maximiliano Sandroval <[email protected]>
> Signed-off-by: Stefan Mayr <[email protected]>

[0] forky will remove /etc/timezone unconditionally, while /etc/localtime
remains:
https://metadata.ftp-master.debian.org/changelogs//main/t/tzdata/tzdata_2025c-3_changelog



Reply via email to