From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 04319609C2 for ; Thu, 13 Aug 2020 14:30:59 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EA1161A4F4 for ; Thu, 13 Aug 2020 14:30:28 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 70D0B1A4DB for ; Thu, 13 Aug 2020 14:30:24 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 37CD9445EA for ; Thu, 13 Aug 2020 14:30:24 +0200 (CEST) From: Dominik Csapak To: pbs-devel@lists.proxmox.com Date: Thu, 13 Aug 2020 14:30:19 +0200 Message-Id: <20200813123019.473-3-d.csapak@proxmox.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200813123019.473-1-d.csapak@proxmox.com> References: <20200813123019.473-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.045 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods NO_DNS_FOR_FROM 0.379 Envelope sender has no MX or A DNS records RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Subject: [pbs-devel] [PATCH proxmox-backup 3/3] config/jobstate: replace Job:load with create_state_file X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Aug 2020 12:30:59 -0000 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 --- 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