Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

needs-restarting: fix for RTC not in UTC #518

Conversation

inknos
Copy link
Contributor

@inknos inknos commented Jan 18, 2024

For systems where the RTC is not in UTC, the boot time is computed in the future if the timezone is +something.
The correct time can be determined by btime in /proc/stat. Such systems are usually configured with timedatectl set-local-rtc 1 which changes the 3rd line of /etc/adjtime from UTC to LOCAL. See man timedatectl.

For systems where the RTC is not in UTC, the boot time is computed
in the future if the timezone is +something.
The correct time can be determined by btime in /proc/stat.
Such systems are usually configured with `timedatectl set-local-rtc 1`
which changes the 3rd line of /etc/adjtime from UTC to LOCAL.
See man timedatectl.
with open('/proc/stat', 'r') as f:
btime = [line for line in f if 'btime' in line]
proc_stat_boot_time = int(btime[0].strip().split(' ')[1])
return max(proc_1_boot_time, proc_stat_boot_time)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't doing the max here not work if (1) proc_1_boot_time is X hours ahead of the correct boot time, due to set-local-rtc 1 and (2) proc_stat_boot_time is correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right. actually, returning proc_stat_boot_time should be enough since it should work every time local rtc is on... if not in a container, like you mention here (rpm-software-management/dnf5#1184)

Copy link
Member

@evan-goode evan-goode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the btime from /proc/stat give us any more info than /proc/uptime? My current understanding is:

  1. When we are NOT in a container:
    • mtime of /proc/1 is unreliable because an RTC may be unavailable, or it may be incorrect due to set-local-rtc 1.
    • current time minus /proc/uptime is correct
    • /proc/stat btime is correct
  2. When we ARE in a container:
    • mtime of /proc/1 is correct
    • /proc/stat btime is incorrect, has uptime of host system
    • /proc/uptime is incorrect, has uptime of the host system

I propose getting UserspaceTimestampMonotonic, which is essentially uptime, from systemd (I mentioned here also). If systemd is available, this number will definitely be correct. If systemd is not available, we are probably in a container, and we fall back to the old method of max(proc_1_boot_time, proc_uptime_boot_time).

EDIT: I misunderstood UserspaceTimestampMonotonic: rpm-software-management/dnf5#1184 (comment)

@inknos
Copy link
Contributor Author

inknos commented Jan 22, 2024

I propose getting UserspaceTimestampMonotonic, which is essentially uptime, from systemd (I mentioned rpm-software-management/dnf5#1184 (comment) also). If systemd is available, this number will definitely be correct. If systemd is not available, we are probably in a container, and we fall back to the old method of max(proc_1_boot_time, proc_uptime_boot_time).

this would definitely improve the situation, and it makes sense to see it implemented in dnf4/5.

from my pessimistic point of view, I believe we'll hit more cases in the future that will not fall in this generalization, but I would say yours is a solid approach.

@evan-goode
Copy link
Member

See rpm-software-management/dnf5#1184 (comment) and rpm-software-management/dnf5#1198, I think the same fix could be applied here to DNF 4.

@inknos
Copy link
Contributor Author

inknos commented Mar 13, 2024

Closing in favor of this #527

@inknos inknos closed this Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants