all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Stoiko Ivanov <s.ivanov@proxmox.com>
To: Stefan Mayr <stefan@mayr-stefan.de>
Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH common, container, manager and pmg-api v2 0/4] Fix #7175: replace timezone handling with systemd timedatectl
Date: Fri, 20 Feb 2026 12:48:09 +0100	[thread overview]
Message-ID: <20260220124809.076116bb@rosa.proxmox.com> (raw)
In-Reply-To: <20260125210151.1151-1-stefan@mayr-stefan.de>

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 <stefan@mayr-stefan.de> 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 <s.ivanov@proxmox.com>
Reviewed-by: Stoiko Ivanov <s.ivanov@proxmox.com>


> 
> 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 <f.gruenbichler@proxmox.com>
> Suggested-by: Maximiliano Sandroval <m.sandoval@proxmox.com>
> Signed-off-by: Stefan Mayr <stefan@mayr-stefan.de>

[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




      parent reply	other threads:[~2026-02-20 11:47 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-25 21:01 Stefan Mayr
2026-01-25 21:01 ` [pve-devel] [PATCH manager v2 1/4] " Stefan Mayr
2026-01-25 21:01 ` [pve-devel] [PATCH container v2 2/4] " Stefan Mayr
2026-01-25 21:01 ` [pve-devel] [PATCH pmg-api v2 3/4] " Stefan Mayr
2026-01-25 21:01 ` [pve-devel] [PATCH common v2 4/4] " Stefan Mayr
2026-02-18 19:53 ` [pve-devel] [PATCH common, container, manager and pmg-api v2 0/4] " Stefan Mayr
2026-02-19  8:13   ` Fabian Grünbichler
2026-02-20 11:48 ` Stoiko Ivanov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260220124809.076116bb@rosa.proxmox.com \
    --to=s.ivanov@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=stefan@mayr-stefan.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal