* [pbs-devel] [PATCH proxmox-backup 01/10] tools/systemd/time: let libc normalize time for us
2020-09-03 11:39 [pbs-devel] [PATCH proxmox-backup 00/10] implement dates for calendarevents Dominik Csapak
@ 2020-09-03 11:39 ` Dominik Csapak
2020-09-04 4:52 ` Dietmar Maurer
2020-09-04 4:57 ` Dietmar Maurer
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 02/10] tools/systemd/time: move continue out of the if/else Dominik Csapak
` (8 subsequent siblings)
9 siblings, 2 replies; 16+ messages in thread
From: Dominik Csapak @ 2020-09-03 11:39 UTC (permalink / raw)
To: pbs-devel
mktime/gmtime can normalize time and even can handle special timezone
cases like the fact that the time 2:30 on specific day/timezone combos
do not exists
we have to convert the signature of all functions that use
normalize_time since mktime/gmtime can return an EOVERFLOW
but if this happens there is no way we can find a good time anyway
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/tools/systemd/time.rs | 16 ++---
src/tools/systemd/tm_editor.rs | 114 +++++++++------------------------
2 files changed, 37 insertions(+), 93 deletions(-)
diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
index 773a1509..edf26505 100644
--- a/src/tools/systemd/time.rs
+++ b/src/tools/systemd/time.rs
@@ -182,11 +182,11 @@ pub fn compute_next_event(
.find(|d| event.days.contains(WeekDays::from_bits(1<<d).unwrap()))
{
// try next day
- t.add_days(n - day_num, true);
+ t.add_days(n - day_num, true)?;
continue;
} else {
// try next week
- t.add_days(7 - day_num, true);
+ t.add_days(7 - day_num, true)?;
continue;
}
}
@@ -200,11 +200,11 @@ pub fn compute_next_event(
} else {
if let Some(n) = DateTimeValue::find_next(&event.hour, hour) {
// test next hour
- t.set_time(n as libc::c_int, 0, 0);
+ t.set_time(n as libc::c_int, 0, 0)?;
continue;
} else {
// test next day
- t.add_days(1, true);
+ t.add_days(1, true)?;
continue;
}
}
@@ -218,11 +218,11 @@ pub fn compute_next_event(
} else {
if let Some(n) = DateTimeValue::find_next(&event.minute, minute) {
// test next minute
- t.set_min_sec(n as libc::c_int, 0);
+ t.set_min_sec(n as libc::c_int, 0)?;
continue;
} else {
// test next hour
- t.set_time(t.hour() + 1, 0, 0);
+ t.set_time(t.hour() + 1, 0, 0)?;
continue;
}
}
@@ -236,11 +236,11 @@ pub fn compute_next_event(
} else {
if let Some(n) = DateTimeValue::find_next(&event.second, second) {
// test next second
- t.set_sec(n as libc::c_int);
+ t.set_sec(n as libc::c_int)?;
continue;
} else {
// test next min
- t.set_min_sec(t.min() + 1, 0);
+ t.set_min_sec(t.min() + 1, 0)?;
continue;
}
}
diff --git a/src/tools/systemd/tm_editor.rs b/src/tools/systemd/tm_editor.rs
index 3b81d1cb..62f8d1d0 100644
--- a/src/tools/systemd/tm_editor.rs
+++ b/src/tools/systemd/tm_editor.rs
@@ -22,24 +22,6 @@ pub struct TmEditor {
pub changes: TMChanges,
}
-fn is_leap_year(year: libc::c_int) -> bool {
- if year % 4 != 0 { return false; }
- if year % 100 != 0 { return true; }
- if year % 400 != 0 { return false; }
- return true;
-}
-
-fn days_in_month(mon: libc::c_int, year: libc::c_int) -> libc::c_int {
-
- let mon = mon % 12;
-
- static MAP: &[libc::c_int] = &[31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31];
-
- if mon == 1 && is_leap_year(year) { return 29; }
-
- MAP[mon as usize]
-}
-
impl TmEditor {
pub fn new(epoch: i64, utc: bool) -> Result<Self, Error> {
@@ -50,12 +32,12 @@ impl TmEditor {
pub fn into_epoch(mut self) -> Result<i64, Error> {
self.t.tm_year -= 1900;
- let epoch = if self.utc { timegm(self.t)? } else { timelocal(self.t)? };
+ let epoch = if self.utc { timegm(&mut self.t)? } else { timelocal(&mut self.t)? };
Ok(epoch)
}
- pub fn add_days(&mut self, days: libc::c_int, reset_time: bool) {
- if days == 0 { return; }
+ pub fn add_days(&mut self, days: libc::c_int, reset_time: bool) -> Result<(), Error> {
+ if days == 0 { return Ok(()); }
if reset_time {
self.t.tm_hour = 0;
self.t.tm_min = 0;
@@ -65,7 +47,7 @@ impl TmEditor {
self.t.tm_mday += days;
self.t.tm_wday += days;
self.changes.insert(TMChanges::MDAY|TMChanges::WDAY);
- self.wrap_time();
+ self.normalize_time()
}
pub fn hour(&self) -> libc::c_int { self.t.tm_hour }
@@ -77,109 +59,71 @@ impl TmEditor {
(self.t.tm_wday + 6) % 7
}
- pub fn set_time(&mut self, hour: libc::c_int, min: libc::c_int, sec: libc::c_int) {
+ pub fn set_time(&mut self, hour: libc::c_int, min: libc::c_int, sec: libc::c_int) -> Result<(), Error> {
self.t.tm_hour = hour;
self.t.tm_min = min;
self.t.tm_sec = sec;
self.changes.insert(TMChanges::HOUR|TMChanges::MIN|TMChanges::SEC);
- self.wrap_time();
+ self.normalize_time()
}
- pub fn set_min_sec(&mut self, min: libc::c_int, sec: libc::c_int) {
+ pub fn set_min_sec(&mut self, min: libc::c_int, sec: libc::c_int) -> Result<(), Error> {
self.t.tm_min = min;
self.t.tm_sec = sec;
self.changes.insert(TMChanges::MIN|TMChanges::SEC);
- self.wrap_time();
+ self.normalize_time()
}
- fn wrap_time(&mut self) {
-
- // sec: 0..59
- if self.t.tm_sec >= 60 {
- self.t.tm_min += self.t.tm_sec / 60;
- self.t.tm_sec %= 60;
- self.changes.insert(TMChanges::SEC|TMChanges::MIN);
+ fn normalize_time(&mut self) -> Result<(), Error> {
+ // libc normalizes it for us
+ if self.utc {
+ timegm(&mut self.t)?;
+ } else {
+ timelocal(&mut self.t)?;
}
-
- // min: 0..59
- if self.t.tm_min >= 60 {
- self.t.tm_hour += self.t.tm_min / 60;
- self.t.tm_min %= 60;
- self.changes.insert(TMChanges::MIN|TMChanges::HOUR);
- }
-
- // hour: 0..23
- if self.t.tm_hour >= 24 {
- self.t.tm_mday += self.t.tm_hour / 24;
- self.t.tm_wday += self.t.tm_hour / 24;
- self.t.tm_hour %= 24;
- self.changes.insert(TMChanges::HOUR|TMChanges::MDAY|TMChanges::WDAY);
- }
-
- // Translate to 0..($days_in_mon-1)
- self.t.tm_mday -= 1;
- loop {
- let days_in_mon = days_in_month(self.t.tm_mon, self.t.tm_year);
- if self.t.tm_mday < days_in_mon { break; }
- // Wrap one month
- self.t.tm_mday -= days_in_mon;
- self.t.tm_mon += 1;
- self.changes.insert(TMChanges::MDAY|TMChanges::WDAY|TMChanges::MON);
- }
-
- // Translate back to 1..$days_in_mon
- self.t.tm_mday += 1;
-
- // mon: 0..11
- if self.t.tm_mon >= 12 {
- self.t.tm_year += self.t.tm_mon / 12;
- self.t.tm_mon %= 12;
- self.changes.insert(TMChanges::MON|TMChanges::YEAR);
- }
-
- self.t.tm_wday %= 7;
+ Ok(())
}
- pub fn set_sec(&mut self, v: libc::c_int) {
+ pub fn set_sec(&mut self, v: libc::c_int) -> Result<(), Error> {
self.t.tm_sec = v;
self.changes.insert(TMChanges::SEC);
- self.wrap_time();
+ self.normalize_time()
}
- pub fn set_min(&mut self, v: libc::c_int) {
+ pub fn set_min(&mut self, v: libc::c_int) -> Result<(), Error> {
self.t.tm_min = v;
self.changes.insert(TMChanges::MIN);
- self.wrap_time();
+ self.normalize_time()
}
- pub fn set_hour(&mut self, v: libc::c_int) {
+ pub fn set_hour(&mut self, v: libc::c_int) -> Result<(), Error> {
self.t.tm_hour = v;
self.changes.insert(TMChanges::HOUR);
- self.wrap_time();
+ self.normalize_time()
}
- pub fn set_mday(&mut self, v: libc::c_int) {
+ pub fn set_mday(&mut self, v: libc::c_int) -> Result<(), Error> {
self.t.tm_mday = v;
self.changes.insert(TMChanges::MDAY);
- self.wrap_time();
+ self.normalize_time()
}
- pub fn set_mon(&mut self, v: libc::c_int) {
+ pub fn set_mon(&mut self, v: libc::c_int) -> Result<(), Error> {
self.t.tm_mon = v;
self.changes.insert(TMChanges::MON);
- self.wrap_time();
+ self.normalize_time()
}
- pub fn set_year(&mut self, v: libc::c_int) {
+ pub fn set_year(&mut self, v: libc::c_int) -> Result<(), Error> {
self.t.tm_year = v;
self.changes.insert(TMChanges::YEAR);
- self.wrap_time();
+ self.normalize_time()
}
- pub fn set_wday(&mut self, v: libc::c_int) {
+ pub fn set_wday(&mut self, v: libc::c_int) -> Result<(), Error> {
self.t.tm_wday = v;
self.changes.insert(TMChanges::WDAY);
- self.wrap_time();
+ self.normalize_time()
}
}
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [pbs-devel] [PATCH proxmox-backup 02/10] tools/systemd/time: move continue out of the if/else
2020-09-03 11:39 [pbs-devel] [PATCH proxmox-backup 00/10] implement dates for calendarevents Dominik Csapak
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 01/10] tools/systemd/time: let libc normalize time for us Dominik Csapak
@ 2020-09-03 11:39 ` Dominik Csapak
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 03/10] tools/systemd/time: convert the resulting timestamp into an option Dominik Csapak
` (7 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Dominik Csapak @ 2020-09-03 11:39 UTC (permalink / raw)
To: pbs-devel
will be called anyway
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/tools/systemd/time.rs | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
index edf26505..a6db8a92 100644
--- a/src/tools/systemd/time.rs
+++ b/src/tools/systemd/time.rs
@@ -183,12 +183,11 @@ pub fn compute_next_event(
{
// try next day
t.add_days(n - day_num, true)?;
- continue;
} else {
// try next week
t.add_days(7 - day_num, true)?;
- continue;
}
+ continue;
}
}
@@ -201,12 +200,11 @@ pub fn compute_next_event(
if let Some(n) = DateTimeValue::find_next(&event.hour, hour) {
// test next hour
t.set_time(n as libc::c_int, 0, 0)?;
- continue;
} else {
// test next day
t.add_days(1, true)?;
- continue;
}
+ continue;
}
}
@@ -219,12 +217,11 @@ pub fn compute_next_event(
if let Some(n) = DateTimeValue::find_next(&event.minute, minute) {
// test next minute
t.set_min_sec(n as libc::c_int, 0)?;
- continue;
} else {
// test next hour
t.set_time(t.hour() + 1, 0, 0)?;
- continue;
}
+ continue;
}
}
@@ -237,12 +234,11 @@ pub fn compute_next_event(
if let Some(n) = DateTimeValue::find_next(&event.second, second) {
// test next second
t.set_sec(n as libc::c_int)?;
- continue;
} else {
// test next min
t.set_min_sec(t.min() + 1, 0)?;
- continue;
}
+ continue;
}
}
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [pbs-devel] [PATCH proxmox-backup 03/10] tools/systemd/time: convert the resulting timestamp into an option
2020-09-03 11:39 [pbs-devel] [PATCH proxmox-backup 00/10] implement dates for calendarevents Dominik Csapak
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 01/10] tools/systemd/time: let libc normalize time for us Dominik Csapak
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 02/10] tools/systemd/time: move continue out of the if/else Dominik Csapak
@ 2020-09-03 11:39 ` Dominik Csapak
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 04/10] tools/systemd/tm_editor: remove reset_time from add_days and document it Dominik Csapak
` (6 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Dominik Csapak @ 2020-09-03 11:39 UTC (permalink / raw)
To: pbs-devel
we want to use dates for the calendarspec, and with that there are some
impossible combinations that cannot be detected during parsing
(e.g. some datetimes do not exist in some timezones, and the timezone
can change after setting the schedule)
so finding no timestamp is not an error anymore but a valid result
we omit logging in that case (since it is not an error anymore)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/api2/admin/sync.rs | 3 ++-
src/bin/proxmox-backup-proxy.rs | 9 ++++++---
src/tools/systemd/time.rs | 16 ++++++++++------
3 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/src/api2/admin/sync.rs b/src/api2/admin/sync.rs
index ff1e5ebf..bea52a0a 100644
--- a/src/api2/admin/sync.rs
+++ b/src/api2/admin/sync.rs
@@ -57,7 +57,8 @@ pub fn list_sync_jobs(
job.next_run = (|| -> Option<i64> {
let schedule = job.schedule.as_ref()?;
let event = parse_calendar_event(&schedule).ok()?;
- compute_next_event(&event, last, false).ok()
+ // ignore errors
+ compute_next_event(&event, last, false).unwrap_or_else(|_| None)
})();
}
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index dd081dfe..2b43c5ac 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -301,7 +301,8 @@ async fn schedule_datastore_garbage_collection() {
};
let next = match compute_next_event(&event, last, false) {
- Ok(next) => next,
+ Ok(Some(next)) => next,
+ Ok(None) => continue,
Err(err) => {
eprintln!("compute_next_event for '{}' failed - {}", event_str, err);
continue;
@@ -412,7 +413,8 @@ async fn schedule_datastore_prune() {
};
let next = match compute_next_event(&event, last, false) {
- Ok(next) => next,
+ Ok(Some(next)) => next,
+ Ok(None) => continue,
Err(err) => {
eprintln!("compute_next_event for '{}' failed - {}", event_str, err);
continue;
@@ -520,7 +522,8 @@ async fn schedule_datastore_sync_jobs() {
};
let next = match compute_next_event(&event, last, false) {
- Ok(next) => next,
+ Ok(Some(next)) => next,
+ Ok(None) => continue,
Err(err) => {
eprintln!("compute_next_event for '{}' failed - {}", event_str, err);
continue;
diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
index a6db8a92..6c7cefec 100644
--- a/src/tools/systemd/time.rs
+++ b/src/tools/systemd/time.rs
@@ -1,4 +1,4 @@
-use anyhow::{bail, Error};
+use anyhow::Error;
use bitflags::bitflags;
pub use super::parse_time::*;
@@ -155,7 +155,7 @@ pub fn compute_next_event(
event: &CalendarEvent,
last: i64,
utc: bool,
-) -> Result<i64, Error> {
+) -> Result<Option<i64>, Error> {
let last = last + 1; // at least one second later
@@ -166,8 +166,9 @@ pub fn compute_next_event(
let mut count = 0;
loop {
- if count > 1000 { // should not happen
- bail!("unable to compute next calendar event");
+ // cancel after 1000 loops
+ if count > 1000 {
+ return Ok(None);
} else {
count += 1;
}
@@ -243,13 +244,15 @@ pub fn compute_next_event(
}
let next = t.into_epoch()?;
- return Ok(next)
+ return Ok(Some(next))
}
}
#[cfg(test)]
mod test {
+ use anyhow::bail;
+
use super::*;
use proxmox::tools::time::*;
@@ -276,7 +279,7 @@ mod test {
};
match compute_next_event(&event, last, true) {
- Ok(next) => {
+ Ok(Some(next)) => {
if next == expect {
println!("next {:?} => {}", event, next);
} else {
@@ -284,6 +287,7 @@ mod test {
event, gmtime(next), gmtime(expect));
}
}
+ Ok(None) => bail!("next {:?} failed to find a timestamp", event),
Err(err) => bail!("compute next for '{}' failed - {}", v, err),
}
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [pbs-devel] [PATCH proxmox-backup 04/10] tools/systemd/tm_editor: remove reset_time from add_days and document it
2020-09-03 11:39 [pbs-devel] [PATCH proxmox-backup 00/10] implement dates for calendarevents Dominik Csapak
` (2 preceding siblings ...)
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 03/10] tools/systemd/time: convert the resulting timestamp into an option Dominik Csapak
@ 2020-09-03 11:39 ` Dominik Csapak
2020-09-04 5:12 ` Dietmar Maurer
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 05/10] tools/systemd/parse_time: error out on invalid ranges Dominik Csapak
` (5 subsequent siblings)
9 siblings, 1 reply; 16+ messages in thread
From: Dominik Csapak @ 2020-09-03 11:39 UTC (permalink / raw)
To: pbs-devel
we never passed 'false' to it anyway so remove it
(we can add it again if we should ever need it)
also remove the adding of wday (gets normalized anyway)
and set changes to all, since adding days can wrap the month and year too
(all other fields get set by us here explicitely)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/tools/systemd/time.rs | 6 +++---
src/tools/systemd/tm_editor.rs | 15 ++++++---------
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
index 6c7cefec..b6899b0c 100644
--- a/src/tools/systemd/time.rs
+++ b/src/tools/systemd/time.rs
@@ -183,10 +183,10 @@ pub fn compute_next_event(
.find(|d| event.days.contains(WeekDays::from_bits(1<<d).unwrap()))
{
// try next day
- t.add_days(n - day_num, true)?;
+ t.add_days(n - day_num,)?;
} else {
// try next week
- t.add_days(7 - day_num, true)?;
+ t.add_days(7 - day_num)?;
}
continue;
}
@@ -203,7 +203,7 @@ pub fn compute_next_event(
t.set_time(n as libc::c_int, 0, 0)?;
} else {
// test next day
- t.add_days(1, true)?;
+ t.add_days(1)?;
}
continue;
}
diff --git a/src/tools/systemd/tm_editor.rs b/src/tools/systemd/tm_editor.rs
index 62f8d1d0..a1cf2464 100644
--- a/src/tools/systemd/tm_editor.rs
+++ b/src/tools/systemd/tm_editor.rs
@@ -36,17 +36,14 @@ impl TmEditor {
Ok(epoch)
}
- pub fn add_days(&mut self, days: libc::c_int, reset_time: bool) -> Result<(), Error> {
+ /// increases the day by 'days' and resets all smaller fields to their minimum
+ pub fn add_days(&mut self, days: libc::c_int) -> Result<(), Error> {
if days == 0 { return Ok(()); }
- if reset_time {
- self.t.tm_hour = 0;
- self.t.tm_min = 0;
- self.t.tm_sec = 0;
- self.changes.insert(TMChanges::HOUR|TMChanges::MIN|TMChanges::SEC);
- }
+ self.t.tm_hour = 0;
+ self.t.tm_min = 0;
+ self.t.tm_sec = 0;
self.t.tm_mday += days;
- self.t.tm_wday += days;
- self.changes.insert(TMChanges::MDAY|TMChanges::WDAY);
+ self.changes = TMChanges::all(); // adding a day can wrap the month/year too
self.normalize_time()
}
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [pbs-devel] [PATCH proxmox-backup 05/10] tools/systemd/parse_time: error out on invalid ranges
2020-09-03 11:39 [pbs-devel] [PATCH proxmox-backup 00/10] implement dates for calendarevents Dominik Csapak
` (3 preceding siblings ...)
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 04/10] tools/systemd/tm_editor: remove reset_time from add_days and document it Dominik Csapak
@ 2020-09-03 11:39 ` Dominik Csapak
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 06/10] tools/systemd/time: fix selection for multiple options Dominik Csapak
` (4 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Dominik Csapak @ 2020-09-03 11:39 UTC (permalink / raw)
To: pbs-devel
if the range is reverse (bigger..smaller) we will never find a value,
so error out during parsing
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/tools/systemd/parse_time.rs | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/tools/systemd/parse_time.rs b/src/tools/systemd/parse_time.rs
index 93657b21..051c4968 100644
--- a/src/tools/systemd/parse_time.rs
+++ b/src/tools/systemd/parse_time.rs
@@ -145,6 +145,9 @@ fn parse_date_time_comp(max: usize) -> impl Fn(&str) -> IResult<&str, DateTimeVa
let (i, value) = parse_time_comp(max)(i)?;
if let (i, Some(end)) = opt(preceded(tag(".."), parse_time_comp(max)))(i)? {
+ if value > end {
+ return Err(parse_error(i, "range start is bigger than end"));
+ }
return Ok((i, DateTimeValue::Range(value, end)))
}
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [pbs-devel] [PATCH proxmox-backup 06/10] tools/systemd/time: fix selection for multiple options
2020-09-03 11:39 [pbs-devel] [PATCH proxmox-backup 00/10] implement dates for calendarevents Dominik Csapak
` (4 preceding siblings ...)
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 05/10] tools/systemd/parse_time: error out on invalid ranges Dominik Csapak
@ 2020-09-03 11:39 ` Dominik Csapak
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 07/10] tools/systemd/time: use i32 for DateTimeValues instead of u32 Dominik Csapak
` (3 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Dominik Csapak @ 2020-09-03 11:39 UTC (permalink / raw)
To: pbs-devel
if we give multiple options/ranges for a value, e.g.
2,4,8
we always choose the biggest, instead of the smallest that is next
this happens because in DateTimeValue::find_next(value)
'next' can be set multiple times and we set it when the new
value was *bigger* than the last found 'next' value, when in reality
we have to choose the *smallest* next we can find
reverse the comparison operator to fix this
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/tools/systemd/time.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
index b6899b0c..83ae19e8 100644
--- a/src/tools/systemd/time.rs
+++ b/src/tools/systemd/time.rs
@@ -54,7 +54,7 @@ impl DateTimeValue {
let mut next: Option<u32> = None;
let mut set_next = |v: u32| {
if let Some(n) = next {
- if v > n { next = Some(v); }
+ if v < n { next = Some(v); }
} else {
next = Some(v);
}
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [pbs-devel] [PATCH proxmox-backup 07/10] tools/systemd/time: use i32 for DateTimeValues instead of u32
2020-09-03 11:39 [pbs-devel] [PATCH proxmox-backup 00/10] implement dates for calendarevents Dominik Csapak
` (5 preceding siblings ...)
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 06/10] tools/systemd/time: fix selection for multiple options Dominik Csapak
@ 2020-09-03 11:39 ` Dominik Csapak
2020-09-04 5:31 ` Dietmar Maurer
2020-09-04 5:39 ` Dietmar Maurer
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 08/10] tools/systemd/tm_editor: remove conversion of the year Dominik Csapak
` (2 subsequent siblings)
9 siblings, 2 replies; 16+ messages in thread
From: Dominik Csapak @ 2020-09-03 11:39 UTC (permalink / raw)
To: pbs-devel
for most of the fields it does not matter
(mon,day,hour,min,sec)
but for years it matters, since negative years in a tm struct are valid
so to be able to handle that, convert those to i32
on parsing, we never allow negative numbers anyway
also, this way we can drop the manual casts (which would have been very
wrong if the underlying tm struct would have contained negative values)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/tools/systemd/parse_time.rs | 2 +-
src/tools/systemd/time.rs | 22 +++++++++++-----------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/tools/systemd/parse_time.rs b/src/tools/systemd/parse_time.rs
index 051c4968..810a9883 100644
--- a/src/tools/systemd/parse_time.rs
+++ b/src/tools/systemd/parse_time.rs
@@ -86,7 +86,7 @@ lazy_static! {
map
};
}
-fn parse_time_comp(max: usize) -> impl Fn(&str) -> IResult<&str, u32> {
+fn parse_time_comp(max: usize) -> impl Fn(&str) -> IResult<&str, i32> {
move |i: &str| {
let (i, v) = map_res(recognize(digit1), str::parse)(i)?;
if (v as usize) >= max {
diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
index 83ae19e8..6ce881e7 100644
--- a/src/tools/systemd/time.rs
+++ b/src/tools/systemd/time.rs
@@ -19,14 +19,14 @@ bitflags!{
#[derive(Debug)]
pub enum DateTimeValue {
- Single(u32),
- Range(u32, u32),
- Repeated(u32, u32),
+ Single(i32),
+ Range(i32, i32),
+ Repeated(i32, i32),
}
impl DateTimeValue {
// Test if the entry contains the value
- pub fn contains(&self, value: u32) -> bool {
+ pub fn contains(&self, value: i32) -> bool {
match self {
DateTimeValue::Single(v) => *v == value,
DateTimeValue::Range(start, end) => value >= *start && value <= *end,
@@ -45,14 +45,14 @@ impl DateTimeValue {
}
}
- pub fn list_contains(list: &[DateTimeValue], value: u32) -> bool {
+ pub fn list_contains(list: &[DateTimeValue], value: i32) -> bool {
list.iter().find(|spec| spec.contains(value)).is_some()
}
// Find an return an entry greater than value
- pub fn find_next(list: &[DateTimeValue], value: u32) -> Option<u32> {
- let mut next: Option<u32> = None;
- let mut set_next = |v: u32| {
+ pub fn find_next(list: &[DateTimeValue], value: i32) -> Option<i32> {
+ let mut next: Option<i32> = None;
+ let mut set_next = |v: i32| {
if let Some(n) = next {
if v < n { next = Some(v); }
} else {
@@ -194,7 +194,7 @@ pub fn compute_next_event(
// this day
if !event.hour.is_empty() && t.changes.contains(TMChanges::HOUR) {
- let hour = t.hour() as u32;
+ let hour = t.hour();
if DateTimeValue::list_contains(&event.hour, hour) {
t.changes.remove(TMChanges::HOUR);
} else {
@@ -211,7 +211,7 @@ pub fn compute_next_event(
// this hour
if !event.minute.is_empty() && t.changes.contains(TMChanges::MIN) {
- let minute = t.min() as u32;
+ let minute = t.min();
if DateTimeValue::list_contains(&event.minute, minute) {
t.changes.remove(TMChanges::MIN);
} else {
@@ -228,7 +228,7 @@ pub fn compute_next_event(
// this minute
if !event.second.is_empty() && t.changes.contains(TMChanges::SEC) {
- let second = t.sec() as u32;
+ let second = t.sec();
if DateTimeValue::list_contains(&event.second, second) {
t.changes.remove(TMChanges::SEC);
} else {
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup 07/10] tools/systemd/time: use i32 for DateTimeValues instead of u32
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 07/10] tools/systemd/time: use i32 for DateTimeValues instead of u32 Dominik Csapak
@ 2020-09-04 5:31 ` Dietmar Maurer
2020-09-04 5:39 ` Dietmar Maurer
1 sibling, 0 replies; 16+ messages in thread
From: Dietmar Maurer @ 2020-09-04 5:31 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
> for most of the fields it does not matter
> (mon,day,hour,min,sec)
>
> but for years it matters, since negative years in a tm struct are valid
> so to be able to handle that, convert those to i32
Sorry, but how can we get negative years? Simply disallow that is easier?
> on parsing, we never allow negative numbers anyway
>
> also, this way we can drop the manual casts (which would have been very
> wrong if the underlying tm struct would have contained negative values)
I suggest to use try_from instead.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 07/10] tools/systemd/time: use i32 for DateTimeValues instead of u32
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 07/10] tools/systemd/time: use i32 for DateTimeValues instead of u32 Dominik Csapak
2020-09-04 5:31 ` Dietmar Maurer
@ 2020-09-04 5:39 ` Dietmar Maurer
1 sibling, 0 replies; 16+ messages in thread
From: Dietmar Maurer @ 2020-09-04 5:39 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
> #[derive(Debug)]
> pub enum DateTimeValue {
> - Single(u32),
> - Range(u32, u32),
> - Repeated(u32, u32),
> + Single(i32),
> + Range(i32, i32),
> + Repeated(i32, i32),
Even the repetition value is now signed. I cannot see why that makes sense?
You want to repeat backwards in time?
^ permalink raw reply [flat|nested] 16+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 08/10] tools/systemd/tm_editor: remove conversion of the year
2020-09-03 11:39 [pbs-devel] [PATCH proxmox-backup 00/10] implement dates for calendarevents Dominik Csapak
` (6 preceding siblings ...)
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 07/10] tools/systemd/time: use i32 for DateTimeValues instead of u32 Dominik Csapak
@ 2020-09-03 11:39 ` Dominik Csapak
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 09/10] tools/systemd/tm_editor: add setter/getter for months/years/days Dominik Csapak
2020-09-03 11:40 ` [pbs-devel] [PATCH proxmox-backup 10/10] tools/systemd/time: enable dates for calendarevents Dominik Csapak
9 siblings, 0 replies; 16+ messages in thread
From: Dominik Csapak @ 2020-09-03 11:39 UTC (permalink / raw)
To: pbs-devel
the tm struct contains the year - 1900 but we added that
if we want to use the libc normalization correctly, the tm struct
must have the correct year in it, else the computations for timezones,
etc. fail
instead add a getter that adds the years and a setter that subtracts it again
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/tools/systemd/tm_editor.rs | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/tools/systemd/tm_editor.rs b/src/tools/systemd/tm_editor.rs
index a1cf2464..529e17f6 100644
--- a/src/tools/systemd/tm_editor.rs
+++ b/src/tools/systemd/tm_editor.rs
@@ -25,13 +25,11 @@ pub struct TmEditor {
impl TmEditor {
pub fn new(epoch: i64, utc: bool) -> Result<Self, Error> {
- let mut t = if utc { gmtime(epoch)? } else { localtime(epoch)? };
- t.tm_year += 1900; // real years for clarity
+ let t = if utc { gmtime(epoch)? } else { localtime(epoch)? };
Ok(Self { utc, t, changes: TMChanges::all() })
}
pub fn into_epoch(mut self) -> Result<i64, Error> {
- self.t.tm_year -= 1900;
let epoch = if self.utc { timegm(&mut self.t)? } else { timelocal(&mut self.t)? };
Ok(epoch)
}
@@ -47,6 +45,7 @@ impl TmEditor {
self.normalize_time()
}
+ pub fn year(&self) -> libc::c_int { self.t.tm_year + 1900 } // see man mktime
pub fn hour(&self) -> libc::c_int { self.t.tm_hour }
pub fn min(&self) -> libc::c_int { self.t.tm_min }
pub fn sec(&self) -> libc::c_int { self.t.tm_sec }
@@ -112,7 +111,7 @@ impl TmEditor {
}
pub fn set_year(&mut self, v: libc::c_int) -> Result<(), Error> {
- self.t.tm_year = v;
+ self.t.tm_year = v - 1900;
self.changes.insert(TMChanges::YEAR);
self.normalize_time()
}
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [pbs-devel] [PATCH proxmox-backup 09/10] tools/systemd/tm_editor: add setter/getter for months/years/days
2020-09-03 11:39 [pbs-devel] [PATCH proxmox-backup 00/10] implement dates for calendarevents Dominik Csapak
` (7 preceding siblings ...)
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 08/10] tools/systemd/tm_editor: remove conversion of the year Dominik Csapak
@ 2020-09-03 11:39 ` Dominik Csapak
2020-09-03 11:40 ` [pbs-devel] [PATCH proxmox-backup 10/10] tools/systemd/time: enable dates for calendarevents Dominik Csapak
9 siblings, 0 replies; 16+ messages in thread
From: Dominik Csapak @ 2020-09-03 11:39 UTC (permalink / raw)
To: pbs-devel
add_* are modeled after add_days
subtract one for set_mon to have a consistent interface for all fields
(i.e. getter/setter return/expect the 'real' number, not the ones
in the tm struct)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/tools/systemd/tm_editor.rs | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/src/tools/systemd/tm_editor.rs b/src/tools/systemd/tm_editor.rs
index 529e17f6..871e4c4c 100644
--- a/src/tools/systemd/tm_editor.rs
+++ b/src/tools/systemd/tm_editor.rs
@@ -34,6 +34,31 @@ impl TmEditor {
Ok(epoch)
}
+ /// increases the year by 'years' and resets all smaller fields to their minimum
+ pub fn add_years(&mut self, years: libc::c_int) -> Result<(), Error> {
+ if years == 0 { return Ok(()); }
+ self.t.tm_mon = 0;
+ self.t.tm_mday = 1;
+ self.t.tm_hour = 0;
+ self.t.tm_min = 0;
+ self.t.tm_sec = 0;
+ self.t.tm_year += years;
+ self.changes = TMChanges::all();
+ self.normalize_time()
+ }
+
+ /// increases the month by 'months' and resets all smaller fields to their minimum
+ pub fn add_months(&mut self, months: libc::c_int) -> Result<(), Error> {
+ if months == 0 { return Ok(()); }
+ self.t.tm_mday = 1;
+ self.t.tm_hour = 0;
+ self.t.tm_min = 0;
+ self.t.tm_sec = 0;
+ self.t.tm_mon += months;
+ self.changes = TMChanges::all(); // adding a month can wrap the year
+ self.normalize_time()
+ }
+
/// increases the day by 'days' and resets all smaller fields to their minimum
pub fn add_days(&mut self, days: libc::c_int) -> Result<(), Error> {
if days == 0 { return Ok(()); }
@@ -46,6 +71,8 @@ impl TmEditor {
}
pub fn year(&self) -> libc::c_int { self.t.tm_year + 1900 } // see man mktime
+ pub fn month(&self) -> libc::c_int { self.t.tm_mon + 1 }
+ pub fn day(&self) -> libc::c_int { self.t.tm_mday }
pub fn hour(&self) -> libc::c_int { self.t.tm_hour }
pub fn min(&self) -> libc::c_int { self.t.tm_min }
pub fn sec(&self) -> libc::c_int { self.t.tm_sec }
@@ -105,7 +132,7 @@ impl TmEditor {
}
pub fn set_mon(&mut self, v: libc::c_int) -> Result<(), Error> {
- self.t.tm_mon = v;
+ self.t.tm_mon = v - 1;
self.changes.insert(TMChanges::MON);
self.normalize_time()
}
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [pbs-devel] [PATCH proxmox-backup 10/10] tools/systemd/time: enable dates for calendarevents
2020-09-03 11:39 [pbs-devel] [PATCH proxmox-backup 00/10] implement dates for calendarevents Dominik Csapak
` (8 preceding siblings ...)
2020-09-03 11:39 ` [pbs-devel] [PATCH proxmox-backup 09/10] tools/systemd/tm_editor: add setter/getter for months/years/days Dominik Csapak
@ 2020-09-03 11:40 ` Dominik Csapak
9 siblings, 0 replies; 16+ messages in thread
From: Dominik Csapak @ 2020-09-03 11:40 UTC (permalink / raw)
To: pbs-devel
this implements parsing and calculating calendarevents that have a
basic date component (year-mon-day) with the usual syntax options
(*, ranges, lists)
and some special events:
monthly
yearly/annually (like systemd)
quarterly
semiannually,semi-annually (like systemd)
includes some regression tests
the ~ syntax for days (the last x days of the month) is not yet
implemented
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/tools/systemd/parse_time.rs | 77 +++++++++++++++++++++++++++++++--
src/tools/systemd/time.rs | 76 +++++++++++++++++++++++++++++++-
2 files changed, 147 insertions(+), 6 deletions(-)
diff --git a/src/tools/systemd/parse_time.rs b/src/tools/systemd/parse_time.rs
index 810a9883..578e253b 100644
--- a/src/tools/systemd/parse_time.rs
+++ b/src/tools/systemd/parse_time.rs
@@ -186,6 +186,25 @@ fn parse_time_spec(i: &str) -> IResult<&str, (Vec<DateTimeValue>, Vec<DateTimeVa
}
}
+fn parse_date_spec(i: &str) -> IResult<&str, (Vec<DateTimeValue>, Vec<DateTimeValue>, Vec<DateTimeValue>)> {
+
+ // TODO: implement ~ for days (man systemd.time)
+ if let Ok((i, (year, month, day))) = tuple((
+ parse_date_time_comp_list(2200), // the upper limit for systemd, stay compatible
+ preceded(tag("-"), parse_date_time_comp_list(13)),
+ preceded(tag("-"), parse_date_time_comp_list(32)),
+ ))(i) {
+ Ok((i, (year, month, day)))
+ } else if let Ok((i, (month, day))) = tuple((
+ parse_date_time_comp_list(13),
+ preceded(tag("-"), parse_date_time_comp_list(32)),
+ ))(i) {
+ Ok((i, (Vec::new(), month, day)))
+ } else {
+ Err(parse_error(i, "invalid date spec"))
+ }
+}
+
pub fn parse_calendar_event(i: &str) -> Result<CalendarEvent, Error> {
parse_complete_line("calendar event", i, parse_calendar_event_incomplete)
}
@@ -194,7 +213,7 @@ fn parse_calendar_event_incomplete(mut i: &str) -> IResult<&str, CalendarEvent>
let mut has_dayspec = false;
let mut has_timespec = false;
- let has_datespec = false;
+ let mut has_datespec = false;
let mut event = CalendarEvent::default();
@@ -231,8 +250,52 @@ fn parse_calendar_event_incomplete(mut i: &str) -> IResult<&str, CalendarEvent>
..Default::default()
}));
}
- "monthly" | "yearly" | "quarterly" | "semiannually" => {
- return Err(parse_error(i, "unimplemented date or time specification"));
+ "monthly" => {
+ return Ok(("", CalendarEvent {
+ hour: vec![DateTimeValue::Single(0)],
+ minute: vec![DateTimeValue::Single(0)],
+ second: vec![DateTimeValue::Single(0)],
+ day: vec![DateTimeValue::Single(1)],
+ ..Default::default()
+ }));
+ }
+ "yearly" | "annually" => {
+ return Ok(("", CalendarEvent {
+ hour: vec![DateTimeValue::Single(0)],
+ minute: vec![DateTimeValue::Single(0)],
+ second: vec![DateTimeValue::Single(0)],
+ day: vec![DateTimeValue::Single(1)],
+ month: vec![DateTimeValue::Single(1)],
+ ..Default::default()
+ }));
+ }
+ "quarterly" => {
+ return Ok(("", CalendarEvent {
+ hour: vec![DateTimeValue::Single(0)],
+ minute: vec![DateTimeValue::Single(0)],
+ second: vec![DateTimeValue::Single(0)],
+ day: vec![DateTimeValue::Single(1)],
+ month: vec![
+ DateTimeValue::Single(1),
+ DateTimeValue::Single(4),
+ DateTimeValue::Single(7),
+ DateTimeValue::Single(10),
+ ],
+ ..Default::default()
+ }));
+ }
+ "semiannually" | "semi-annually" => {
+ return Ok(("", CalendarEvent {
+ hour: vec![DateTimeValue::Single(0)],
+ minute: vec![DateTimeValue::Single(0)],
+ second: vec![DateTimeValue::Single(0)],
+ day: vec![DateTimeValue::Single(1)],
+ month: vec![
+ DateTimeValue::Single(1),
+ DateTimeValue::Single(7),
+ ],
+ ..Default::default()
+ }));
}
_ => { /* continue */ }
}
@@ -249,7 +312,13 @@ fn parse_calendar_event_incomplete(mut i: &str) -> IResult<&str, CalendarEvent>
for range in range_list { event.days.insert(range); }
}
- // todo: support date specs
+ if let (n, Some((year, month, day))) = opt(parse_date_spec)(i)? {
+ event.year = year;
+ event.month = month;
+ event.day = day;
+ has_datespec = true;
+ i = space0(n)?.0;
+ }
if let (n, Some((hour, minute, second))) = opt(parse_time_spec)(i)? {
event.hour = hour;
diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
index 6ce881e7..4993a30c 100644
--- a/src/tools/systemd/time.rs
+++ b/src/tools/systemd/time.rs
@@ -101,14 +101,12 @@ pub struct CalendarEvent {
pub minute: Vec<DateTimeValue>,
/// the hour(s) this event should trigger
pub hour: Vec<DateTimeValue>,
-/* FIXME: TODO
/// the day(s) in a month this event should trigger
pub day: Vec<DateTimeValue>,
/// the month(s) in a year this event should trigger
pub month: Vec<DateTimeValue>,
/// the years(s) this event should trigger
pub year: Vec<DateTimeValue>,
-*/
}
#[derive(Default)]
@@ -173,6 +171,51 @@ pub fn compute_next_event(
count += 1;
}
+ if !event.year.is_empty() && t.changes.contains(TMChanges::YEAR) {
+ let year = t.year();
+ if DateTimeValue::list_contains(&event.year, year) {
+ t.changes.remove(TMChanges::YEAR);
+ } else {
+ if let Some(n) = DateTimeValue::find_next(&event.year, year) {
+ t.add_years(n - year)?;
+ continue;
+ } else {
+ // if we have no valid year, we cannot find a correct timestamp
+ return Ok(None);
+ }
+ }
+ }
+
+ if !event.month.is_empty() && t.changes.contains(TMChanges::MON) {
+ let month = t.month();
+ if DateTimeValue::list_contains(&event.month, month) {
+ t.changes.remove(TMChanges::MON);
+ } else {
+ if let Some(n) = DateTimeValue::find_next(&event.month, month) {
+ t.add_months(n - month)?;
+ } else {
+ // if we could not find valid month, retry next year
+ t.add_years(1)?;
+ }
+ continue;
+ }
+ }
+
+ if !event.day.is_empty() && t.changes.contains(TMChanges::MDAY) {
+ let day = t.day();
+ if DateTimeValue::list_contains(&event.day, day) {
+ t.changes.remove(TMChanges::MDAY);
+ } else {
+ if let Some(n) = DateTimeValue::find_next(&event.day, day) {
+ t.add_days(n - day)?;
+ } else {
+ // if we could not find valid mday, retry next month
+ t.add_months(1)?;
+ }
+ continue;
+ }
+ }
+
if !all_days && t.changes.contains(TMChanges::WDAY) { // match day first
let day_num = t.day_num();
let day = WeekDays::from_bits(1<<day_num).unwrap();
@@ -294,6 +337,18 @@ mod test {
Ok(expect)
};
+ let test_never = |v: &'static str, last: i64| -> Result<(), Error> {
+ let event = match parse_calendar_event(v) {
+ Ok(event) => event,
+ Err(err) => bail!("parsing '{}' failed - {}", v, err),
+ };
+
+ match compute_next_event(&event, last, true)? {
+ None => Ok(()),
+ Some(next) => bail!("compute next for '{}' succeeded, but expected fail - result {}", v, next),
+ }
+ };
+
const MIN: i64 = 60;
const HOUR: i64 = 3600;
const DAY: i64 = 3600*24;
@@ -361,6 +416,23 @@ mod test {
n = test_value("1:0", n, THURSDAY_00_00 + i*DAY + HOUR)?;
}
+ // test date functionality
+
+ test_value("2020-07-31", 0, JUL_31_2020)?;
+ test_value("02-28", 0, (31+27)*DAY)?;
+ test_value("02-29", 0, 2*365*DAY + (31+28)*DAY)?; // 1972-02-29
+ test_value("1965/5-01-01", -1, THURSDAY_00_00)?;
+ test_value("2020-7..9-2/2", JUL_31_2020, JUL_31_2020 + 2*DAY)?;
+ test_value("2020,2021-12-31", JUL_31_2020, DEC_31_2020)?;
+
+ test_value("monthly", 0, 31*DAY)?;
+ test_value("quarterly", 0, (31+28+31)*DAY)?;
+ test_value("semiannually", 0, (31+28+31+30+31+30)*DAY)?;
+ test_value("yearly", 0, (365)*DAY)?;
+
+ test_never("2021-02-29", 0)?;
+ test_never("02-30", 0)?;
+
Ok(())
}
--
2.20.1
^ permalink raw reply [flat|nested] 16+ messages in thread