[pbs-devel] [PATCH proxmox-backup 00/10] implement dates for calendarevents

[pbs-devel] [PATCH proxmox-backup 00/10] implement dates for calendarevents

[Dominik Csapak]

implements Dates and more special words for calendar events
and fixes some small bugs

needs my patch for the proxmox crate (tools/time: give tm struct as mut reference)
and a bump/dependency update for that

not implemented for calendarevents are Timezones, Unix Timestamps and
the ~ operator for the last days of the month

Dominik Csapak (10):
  tools/systemd/time: let libc normalize time for us
  tools/systemd/time: move continue out of the if/else
  tools/systemd/time: convert the resulting timestamp into an option
  tools/systemd/tm_editor: remove reset_time from add_days and document it
  tools/systemd/parse_time: error out on invalid ranges
  tools/systemd/time: fix selection for multiple options
  tools/systemd/time: use i32 for DateTimeValues instead of u32
  tools/systemd/tm_editor: remove conversion of the year
  tools/systemd/tm_editor: add setter/getter for months/years/days
  tools/systemd/time: enable dates for calendarevents

 src/api2/admin/sync.rs          |   3 +-
 src/bin/proxmox-backup-proxy.rs |   9 +-
 src/tools/systemd/parse_time.rs |  82 +++++++++++++++-
 src/tools/systemd/time.rs       | 144 +++++++++++++++++++++-------
 src/tools/systemd/tm_editor.rs  | 163 +++++++++++++-------------------
 5 files changed, 258 insertions(+), 143 deletions(-)

-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 01/10] tools/systemd/time: let libc normalize time for us

[Dominik Csapak]

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

[pbs-devel] [PATCH proxmox-backup 02/10] tools/systemd/time: move continue out of the if/else

[Dominik Csapak]

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

[pbs-devel] [PATCH proxmox-backup 03/10] tools/systemd/time: convert the resulting timestamp into an option

[Dominik Csapak]

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

[pbs-devel] [PATCH proxmox-backup 04/10] tools/systemd/tm_editor: remove reset_time from add_days and document it

[Dominik Csapak]

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

[pbs-devel] [PATCH proxmox-backup 05/10] tools/systemd/parse_time: error out on invalid ranges

[Dominik Csapak]

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

[pbs-devel] [PATCH proxmox-backup 06/10] tools/systemd/time: fix selection for multiple options

[Dominik Csapak]

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

[pbs-devel] [PATCH proxmox-backup 07/10] tools/systemd/time: use i32 for DateTimeValues instead of u32

[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

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

[pbs-devel] [PATCH proxmox-backup 08/10] tools/systemd/tm_editor: remove conversion of the year

[Dominik Csapak]

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

[pbs-devel] [PATCH proxmox-backup 09/10] tools/systemd/tm_editor: add setter/getter for months/years/days

[Dominik Csapak]

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

[pbs-devel] [PATCH proxmox-backup 10/10] tools/systemd/time: enable dates for calendarevents

[Dominik Csapak]

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

Re: [pbs-devel] [PATCH proxmox-backup 01/10] tools/systemd/time: let libc normalize time for us

[Dietmar Maurer]

This does not work as expected, because the new normalize_time() does not 
record what fields have changed (self.changes).

Re: [pbs-devel] [PATCH proxmox-backup 01/10] tools/systemd/time: let libc normalize time for us

[Dietmar Maurer]

> 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

And how much slower is that compared to the previous approach?

Re: [pbs-devel] [PATCH proxmox-backup 04/10] tools/systemd/tm_editor: remove reset_time from add_days and document it

[Dietmar Maurer]

> and set changes to all, since adding days can wrap the month and year too
> (all other fields get set by us here explicitely)

The same is true for any other function, for example set_secs() can also modify
all fields.

Re: [pbs-devel] [PATCH proxmox-backup 07/10] tools/systemd/time: use i32 for DateTimeValues instead of u32

[Dietmar Maurer]

> 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.

Re: [pbs-devel] [PATCH proxmox-backup 07/10] tools/systemd/time: use i32 for DateTimeValues instead of u32

[Dietmar Maurer]

>  #[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?