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 A44B668DF0 for ; Mon, 3 Aug 2020 09:35:24 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 950602B173 for ; Mon, 3 Aug 2020 09:35:24 +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 4CA152B166 for ; Mon, 3 Aug 2020 09:35:23 +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 17DFC430D2 for ; Mon, 3 Aug 2020 09:35:23 +0200 (CEST) Date: Mon, 3 Aug 2020 09:35:16 +0200 (CEST) From: Dietmar Maurer To: Proxmox Backup Server development discussion , Dominik Csapak Message-ID: <1070551874.47.1596440117238@webmail.proxmox.com> In-Reply-To: <20200731124330.30576-4-d.csapak@proxmox.com> References: <20200731124330.30576-1-d.csapak@proxmox.com> <20200731124330.30576-4-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Priority: 3 Importance: Normal X-Mailer: Open-Xchange Mailer v7.10.3-Rev19 X-Originating-Client: open-xchange-appsuite X-SPAM-LEVEL: Spam detection results: 0 AWL 0.114 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment 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_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH proxmox-backup 3/8] config: add JobState helper 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: Mon, 03 Aug 2020 07:35:24 -0000 comments inline > On 07/31/2020 2:43 PM Dominik Csapak wrote: > > > this is intended to be a generic helper to (de)serialize job states > (e.g., sync, verify, and so on) > > writes a json file into '/var/lib/proxmox-backup/jobstates/TYPE-ID.json' > > the api creates the directory with the correct permissions, like > the rrd directory > > Signed-off-by: Dominik Csapak > --- > src/bin/proxmox-backup-api.rs | 1 + > src/config.rs | 1 + > src/config/jobstate.rs | 126 ++++++++++++++++++++++++++++++++++ > 3 files changed, 128 insertions(+) > create mode 100644 src/config/jobstate.rs > > diff --git a/src/bin/proxmox-backup-api.rs b/src/bin/proxmox-backup-api.rs > index 9dde46c0..ea306cf0 100644 > --- a/src/bin/proxmox-backup-api.rs > +++ b/src/bin/proxmox-backup-api.rs > @@ -37,6 +37,7 @@ async fn run() -> Result<(), Error> { > config::update_self_signed_cert(false)?; > > proxmox_backup::rrd::create_rrdb_dir()?; > + proxmox_backup::config::jobstate::create_jobstate_dir()?; > > if let Err(err) = generate_auth_key() { > bail!("unable to generate auth key - {}", err); > diff --git a/src/config.rs b/src/config.rs > index e805c38e..f9e08814 100644 > --- a/src/config.rs > +++ b/src/config.rs > @@ -22,6 +22,7 @@ pub mod acl; > pub mod cached_user_info; > pub mod network; > pub mod sync; > +pub mod jobstate; > > /// Check configuration directory permissions > /// > diff --git a/src/config/jobstate.rs b/src/config/jobstate.rs > new file mode 100644 > index 00000000..fb5a8cbf > --- /dev/null > +++ b/src/config/jobstate.rs > @@ -0,0 +1,126 @@ > +use std::fs::File; > +use std::path::{Path, PathBuf}; > +use std::time::Duration; > + > +use serde::{Serialize, Deserialize}; > +use anyhow::{bail, Error, format_err}; > +use proxmox::tools::fs::{file_read_optional_string, replace_file, create_path, CreateOptions}; > + > +use crate::tools::{epoch_now_u64, open_file_locked}; > + > +#[serde(rename_all="kebab-case")] > +#[derive(Serialize,Deserialize)] > +pub struct JobState { > + #[serde(skip_serializing_if="Option::is_none")] > + pub upid: Option, > + pub starttime: i64, Why is upid optional, and starttime not? Also, Upid contains the startime, so why do we store twice? Also, please add documentation to pub structs and functions. > + #[serde(skip_serializing_if="Option::is_none")] > + pub endtime: Option, > + #[serde(skip_serializing_if="Option::is_none")] > + pub state: Option, > +} > + > +const JOB_STATE_BASEDIR: &str = "/var/lib/proxmox-backup/jobstates"; > + > +/// Create jobstate stat dir with correct permission > +pub fn create_jobstate_dir() -> Result<(), Error> { > + > + let backup_user = crate::backup::backup_user()?; > + let opts = CreateOptions::new() > + .owner(backup_user.uid) > + .group(backup_user.gid); > + > + create_path(JOB_STATE_BASEDIR, None, Some(opts)) > + .map_err(|err: Error| format_err!("unable to create rrdb stat dir - {}", err))?; > + > + Ok(()) > +} > + > +fn get_path(jobtype: &str, jobname: &str) -> Result { > + let mut path = PathBuf::from(JOB_STATE_BASEDIR); > + path.push(format!("{}-{}.json", jobtype, jobname)); > + Ok(path) > +} > + > +fn get_lock

(path: P) -> Result > +where > + P: AsRef > +{ > + let mut path = path.as_ref().to_path_buf(); > + path.set_extension("lck"); > + let lock = open_file_locked(path, Duration::new(10, 0))?; > + Ok(lock) > +} > + > +pub fn remove_state_file(jobtype: &str, jobname: &str) -> Result<(), Error> { > + let path = get_path(jobtype, jobname)?; > + let _lock = get_lock(&path)?; > + match std::fs::remove_file(&path) { > + Ok(()) => Ok(()), > + Err(err) => { > + bail!("cannot remove statefile for {} - {}: {}", jobtype, jobname, err); > + } > + } > +} > + > +impl JobState { > + pub fn new(upid: &str) -> Self { > + Self{ > + upid: Some(upid.to_string()), > + starttime: epoch_now_u64().unwrap_or_else(|_| 0) as i64, upid already contains a start time? > + endtime: None, > + state: None, > + } > + } > + > + pub fn finish(&mut self, state: &str) -> Result<(), Error> { > + self.state = Some(state.to_string()); > + self.endtime = Some(epoch_now_u64()? as i64); > + Ok(()) > + } > + > + pub fn try_read_or_create(jobtype: &str, jobname: &str) -> Result { > + let path = get_path(jobtype, jobname)?; > + > + let lock = get_lock(&path)?; > + > + if let Some(state) = file_read_optional_string(path)? { > + Ok(serde_json::from_str(&state)?) > + } else { > + let state = Self{ > + upid: None, > + endtime: None, > + state: None, > + starttime: epoch_now_u64()? as i64, > + }; why do we create a file with no info? Or is that now() relevant? > + state.write_state(jobtype, jobname, Some(lock))?; > + Ok(state) > + } > + } > + > + pub fn write_state(&self, jobtype: &str, jobname: &str, lock: Option) -> Result<(), Error> { This is a strange pub API. I would remove parameter lock. fn write_state_no_lock(&self, path: &Path) -> Result<(), Error> { let serialized = serde_json::to_string(&self)?; let backup_user = crate::backup::backup_user()?; let mode = nix::sys::stat::Mode::from_bits_truncate(0o0644); // set the correct owner/group/permissions while saving file // owner(rw) = backup, group(r)= backup let options = CreateOptions::new() .perm(mode) .owner(backup_user.uid) .group(backup_user.gid); replace_file( path, serialized.as_bytes(), options, ) } pub fn write_state(&self, jobtype: &str, jobname: &str) -> Result<(), Error> { let path = get_path(jobtype, jobname)?; let _lock = get_lock(&path)?; self.write_state_no_lock(&path) } > + let serialized = serde_json::to_string(&self)?; > + let path = get_path(jobtype, jobname)?; > + > + let _lock = if let Some(lock) = lock { > + lock > + } else { > + get_lock(&path)? > + }; > + > + let backup_user = crate::backup::backup_user()?; > + let mode = nix::sys::stat::Mode::from_bits_truncate(0o0644); > + // set the correct owner/group/permissions while saving file > + // owner(rw) = backup, group(r)= backup > + let options = CreateOptions::new() > + .perm(mode) > + .owner(backup_user.uid) > + .group(backup_user.gid); > + > + replace_file( > + path, > + serialized.as_bytes(), > + options, > + ) > + } > +} > -- > 2.20.1 > > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel