[pbs-devel] [PATCH proxmox-backup 3/3] config/jobstate: replace Job:load with create_state_file

[Dominik Csapak]

it really is not necessary, since the only time we are interested in
loading the state from the file is when we list it, and there
we use JobState::load directly to avoid the lock

we still need to create the file on syncjob creation though, so
that we have the correct time for the schedule

to do this we add a new create_state_file that overwrites it on creation
of a syncjob

for safety, we subtract 30 seconds from the in-memory state in case
the statefile is missing

since we call create_state_file from  proxmox-backup-api,
we have to chown the lock file after creating to the backup user,
else the sync job scheduling cannot aquire the lock

also we remove the lock file on statefile removal

Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
---
 src/api2/admin/sync.rs          |  3 +--
 src/api2/config/sync.rs         |  2 ++
 src/bin/proxmox-backup-proxy.rs |  8 +-----
 src/config/jobstate.rs          | 45 +++++++++++++++------------------
 4 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/src/api2/admin/sync.rs b/src/api2/admin/sync.rs
index 0985e26f..ff1e5ebf 100644
--- a/src/api2/admin/sync.rs
+++ b/src/api2/admin/sync.rs
@@ -87,8 +87,7 @@ fn run_sync_job(
 
     let userid: Userid = rpcenv.get_user().unwrap().parse()?;
 
-    let mut job = Job::new("syncjob", &id)?;
-    job.load()?;
+    let job = Job::new("syncjob", &id)?;
 
     let upid_str = do_sync_job(job, sync_job, &userid, None)?;
 
diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index 8b16192c..57192d90 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -83,6 +83,8 @@ pub fn create_sync_job(param: Value) -> Result<(), Error> {
 
     sync::save_config(&config)?;
 
+    crate::config::jobstate::create_state_file("syncjob", &sync_job.id)?;
+
     Ok(())
 }
 
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 1a30bc6f..b75cc763 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -536,13 +536,7 @@ async fn schedule_datastore_sync_jobs() {
         if next > now  { continue; }
 
         let job = match Job::new(worker_type, &job_id) {
-            Ok(mut job) => match job.load() {
-                Ok(_) => job,
-                Err(err) => {
-                    eprintln!("error loading jobstate for {} -  {}: {}", worker_type, &job_id, err);
-                    continue;
-                }
-            }
+            Ok(job) => job,
             Err(_) => continue, // could not get lock
         };
 
diff --git a/src/config/jobstate.rs b/src/config/jobstate.rs
index 8931563b..e5c701d7 100644
--- a/src/config/jobstate.rs
+++ b/src/config/jobstate.rs
@@ -25,13 +25,7 @@
 //!     Err(err) => bail!("could not lock jobstate"),
 //! };
 //!
-//! // job holds the lock
-//! match job.load() {
-//!     Ok(()) => {},
-//!     Err(err) => bail!("could not load state {}", err),
-//! }
-//!
-//! // now the job is loaded;
+//! // job holds the lock, we can start it
 //! job.start("someupid")?;
 //! // do something
 //! let task_state = some_code();
@@ -102,16 +96,32 @@ where
 {
     let mut path = path.as_ref().to_path_buf();
     path.set_extension("lck");
-    open_file_locked(path, Duration::new(10, 0))
+    let lock = open_file_locked(&path, Duration::new(10, 0))?;
+    let backup_user = crate::backup::backup_user()?;
+    nix::unistd::chown(&path, Some(backup_user.uid), Some(backup_user.gid))?;
+    Ok(lock)
 }
 
 /// Removes the statefile of a job, this is useful if we delete a job
 pub fn remove_state_file(jobtype: &str, jobname: &str) -> Result<(), Error> {
-    let path = get_path(jobtype, jobname);
+    let mut path = get_path(jobtype, jobname);
     let _lock = get_lock(&path)?;
     std::fs::remove_file(&path).map_err(|err|
         format_err!("cannot remove statefile for {} - {}: {}", jobtype, jobname, err)
-    )
+    )?;
+    path.set_extension("lck");
+    // ignore errors
+    let _ = std::fs::remove_file(&path).map_err(|err|
+        format_err!("cannot remove lockfile for {} - {}: {}", jobtype, jobname, err)
+    );
+    Ok(())
+}
+
+/// Creates the statefile with the state 'Created'
+/// overwrites if it exists already
+pub fn create_state_file(jobtype: &str, jobname: &str) -> Result<(), Error> {
+    let mut job = Job::new(jobtype, jobname)?;
+    job.write_state()
 }
 
 /// Returns the last run time of a job by reading the statefile
@@ -158,7 +168,7 @@ impl JobState {
             }
         } else {
             Ok(JobState::Created {
-                time: epoch_now_u64()? as i64
+                time: epoch_now_u64()? as i64 - 30,
             })
         }
     }
@@ -185,19 +195,6 @@ impl Job {
         })
     }
 
-    /// Loads the state from the statefile if it exists.
-    /// If not, it gets created. Updates 'Started' State to 'Finished'
-    /// if we detect the UPID already stopped
-    pub fn load(&mut self) -> Result<(), Error> {
-        self.state = JobState::load(&self.jobtype, &self.jobname)?;
-
-        if let Err(err) = self.write_state() {
-            bail!("could not write statefile: {}", err);
-        }
-
-        Ok(())
-    }
-
     /// Start the job and update the statefile accordingly
     /// Fails if the job was already started
     pub fn start(&mut self, upid: &str) -> Result<(), Error> {
-- 
2.20.1