[pbs-devel] [PATCH proxmox v2] sys: pass create options as reference

Dietmar Maurer dietmar at proxmox.com
Wed May 29 11:55:02 CEST 2024


Signed-off-by: Dietmar Maurer <dietmar at proxmox.com>
---

Changes in v2: change everywhere, so "cargo build --all --all-features" works again

 proxmox-acme-api/src/account_config.rs        |  2 +-
 proxmox-acme-api/src/config.rs                |  2 +-
 proxmox-openid/src/auth_state.rs              |  4 ++--
 .../src/filesystem_helpers.rs                 | 10 +++++-----
 proxmox-rest-server/src/api_config.rs         |  4 ++--
 proxmox-rest-server/src/daemon.rs             |  2 +-
 proxmox-rest-server/src/file_logger.rs        |  2 +-
 proxmox-rest-server/src/lib.rs                |  2 +-
 proxmox-rest-server/src/worker_task.rs        | 10 +++++-----
 proxmox-rrd/src/cache.rs                      |  4 ++--
 proxmox-rrd/src/cache/journal.rs              |  4 ++--
 proxmox-rrd/src/cache/rrd_map.rs              |  4 ++--
 proxmox-rrd/src/rrd.rs                        |  2 +-
 proxmox-subscription/src/files.rs             |  4 ++--
 proxmox-sys/src/fs/dir.rs                     | 20 +++++++++----------
 proxmox-sys/src/fs/file.rs                    |  8 ++++----
 proxmox-sys/src/logrotate.rs                  |  2 +-
 .../src/dns/resolv_conf.rs                    |  2 +-
 18 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/proxmox-acme-api/src/account_config.rs b/proxmox-acme-api/src/account_config.rs
index 7c7bd8a3..fd71c285 100644
--- a/proxmox-acme-api/src/account_config.rs
+++ b/proxmox-acme-api/src/account_config.rs
@@ -223,7 +223,7 @@ pub(crate) fn save_account_config(
     replace_file(
         account_cfg_filename,
         &data,
-        CreateOptions::new()
+        &CreateOptions::new()
             .perm(nix::sys::stat::Mode::from_bits_truncate(0o600))
             .owner(nix::unistd::ROOT)
             .group(nix::unistd::Gid::from_raw(0)),
diff --git a/proxmox-acme-api/src/config.rs b/proxmox-acme-api/src/config.rs
index 02b2e68a..de5a07df 100644
--- a/proxmox-acme-api/src/config.rs
+++ b/proxmox-acme-api/src/config.rs
@@ -43,7 +43,7 @@ pub(crate) fn create_secret_subdir<P: AsRef<Path>>(dir: P) -> nix::Result<()> {
         .group(nix::unistd::Gid::from_raw(0))
         .perm(nix::sys::stat::Mode::from_bits_truncate(0o700));
 
-    match proxmox_sys::fs::create_dir(dir, root_only) {
+    match proxmox_sys::fs::create_dir(dir, &root_only) {
         Ok(()) => Ok(()),
         Err(err) if err.already_exists() => Ok(()),
         Err(err) => Err(err),
diff --git a/proxmox-openid/src/auth_state.rs b/proxmox-openid/src/auth_state.rs
index f2a299a0..72b7ce4c 100644
--- a/proxmox-openid/src/auth_state.rs
+++ b/proxmox-openid/src/auth_state.rs
@@ -20,7 +20,7 @@ fn load_auth_state_locked(
         lock_path,
         std::time::Duration::new(10, 0),
         true,
-        CreateOptions::new(),
+        &CreateOptions::new(),
     )?;
 
     let mut path = state_dir.to_owned();
@@ -50,7 +50,7 @@ fn replace_auth_state(path: &Path, data: &Vec<Value>) -> Result<(), Error> {
     let options = CreateOptions::new().perm(mode);
     let raw = serde_json::to_string_pretty(data)?;
 
-    replace_file(path, raw.as_bytes(), options, false)?;
+    replace_file(path, raw.as_bytes(), &options, false)?;
 
     Ok(())
 }
diff --git a/proxmox-product-config/src/filesystem_helpers.rs b/proxmox-product-config/src/filesystem_helpers.rs
index 2c486949..461e09bd 100644
--- a/proxmox-product-config/src/filesystem_helpers.rs
+++ b/proxmox-product-config/src/filesystem_helpers.rs
@@ -66,14 +66,14 @@ pub fn replace_privileged_config<P: AsRef<std::path::Path>>(
     data: &[u8],
 ) -> Result<(), Error> {
     let options = privileged_create_options();
-    proxmox_sys::fs::replace_file(path, data, options, true)?;
+    proxmox_sys::fs::replace_file(path, data, &options, true)?;
     Ok(())
 }
 
 /// Atomically write data to file owned by `api-user.uid:api-user.gid` with permission `0660`.
 pub fn replace_config<P: AsRef<std::path::Path>>(path: P, data: &[u8]) -> Result<(), Error> {
     let options = default_create_options();
-    proxmox_sys::fs::replace_file(path, data, options, true)?;
+    proxmox_sys::fs::replace_file(path, data, &options, true)?;
     Ok(())
 }
 
@@ -82,7 +82,7 @@ pub fn replace_config<P: AsRef<std::path::Path>>(path: P, data: &[u8]) -> Result
 /// Only the superuser can read and write those files.
 pub fn replace_secret_config<P: AsRef<std::path::Path>>(path: P, data: &[u8]) -> Result<(), Error> {
     let options = secret_create_options();
-    proxmox_sys::fs::replace_file(path, data, options, true)?;
+    proxmox_sys::fs::replace_file(path, data, &options, true)?;
     Ok(())
 }
 
@@ -92,7 +92,7 @@ pub fn replace_secret_config<P: AsRef<std::path::Path>>(path: P, data: &[u8]) ->
 /// for system configuration files inside "/etc/" (i.e. "/etc/resolv.conf").
 pub fn replace_system_config<P: AsRef<std::path::Path>>(path: P, data: &[u8]) -> Result<(), Error> {
     let options = system_config_create_options();
-    proxmox_sys::fs::replace_file(path, data, options, true)?;
+    proxmox_sys::fs::replace_file(path, data, &options, true)?;
     Ok(())
 }
 
@@ -124,6 +124,6 @@ pub fn open_api_lockfile<P: AsRef<std::path::Path>>(
 ) -> Result<ApiLockGuard, Error> {
     let options = lockfile_create_options();
     let timeout = timeout.unwrap_or(std::time::Duration::new(10, 0));
-    let file = proxmox_sys::fs::open_file_locked(&path, timeout, exclusive, options)?;
+    let file = proxmox_sys::fs::open_file_locked(&path, timeout, exclusive, &options)?;
     Ok(ApiLockGuard(Some(file)))
 }
diff --git a/proxmox-rest-server/src/api_config.rs b/proxmox-rest-server/src/api_config.rs
index 80589446..1ab2130e 100644
--- a/proxmox-rest-server/src/api_config.rs
+++ b/proxmox-rest-server/src/api_config.rs
@@ -213,7 +213,7 @@ impl ApiConfig {
         let path: PathBuf = path.into();
         if let Some(base) = path.parent() {
             if !base.exists() {
-                create_path(base, None, dir_opts).map_err(|err| format_err!("{}", err))?;
+                create_path(base, None, dir_opts.as_ref()).map_err(|err| format_err!("{}", err))?;
             }
         }
 
@@ -252,7 +252,7 @@ impl ApiConfig {
         let path: PathBuf = path.into();
         if let Some(base) = path.parent() {
             if !base.exists() {
-                create_path(base, None, dir_opts).map_err(|err| format_err!("{}", err))?;
+                create_path(base, None, dir_opts.as_ref()).map_err(|err| format_err!("{}", err))?;
             }
         }
 
diff --git a/proxmox-rest-server/src/daemon.rs b/proxmox-rest-server/src/daemon.rs
index fedbf545..098b93f0 100644
--- a/proxmox-rest-server/src/daemon.rs
+++ b/proxmox-rest-server/src/daemon.rs
@@ -186,7 +186,7 @@ impl Reloader {
                     proxmox_sys::fs::replace_file(
                         pid_fn,
                         pid_str.as_bytes(),
-                        CreateOptions::new(),
+                        &CreateOptions::new(),
                         false,
                     )?;
                 }
diff --git a/proxmox-rest-server/src/file_logger.rs b/proxmox-rest-server/src/file_logger.rs
index 2bb1fac6..50a23a02 100644
--- a/proxmox-rest-server/src/file_logger.rs
+++ b/proxmox-rest-server/src/file_logger.rs
@@ -98,7 +98,7 @@ impl FileLogger {
         }
 
         let file =
-            atomic_open_or_create_file(&file_name, flags, &[], options.file_opts.clone(), false)?;
+            atomic_open_or_create_file(&file_name, flags, &[], &options.file_opts, false)?;
 
         Ok(file)
     }
diff --git a/proxmox-rest-server/src/lib.rs b/proxmox-rest-server/src/lib.rs
index ce9e4f15..ac0cc4ce 100644
--- a/proxmox-rest-server/src/lib.rs
+++ b/proxmox-rest-server/src/lib.rs
@@ -80,7 +80,7 @@ pub(crate) fn pstart() -> u64 {
 /// Helper to write the PID into a file
 pub fn write_pid(pid_fn: &str) -> Result<(), Error> {
     let pid_str = format!("{}\n", *PID);
-    proxmox_sys::fs::replace_file(pid_fn, pid_str.as_bytes(), CreateOptions::new(), false)
+    proxmox_sys::fs::replace_file(pid_fn, pid_str.as_bytes(), &CreateOptions::new(), false)
 }
 
 /// Helper to read the PID from a file
diff --git a/proxmox-rest-server/src/worker_task.rs b/proxmox-rest-server/src/worker_task.rs
index 4cf24cc4..b63f7049 100644
--- a/proxmox-rest-server/src/worker_task.rs
+++ b/proxmox-rest-server/src/worker_task.rs
@@ -83,7 +83,7 @@ impl WorkerTaskSetup {
         let timeout = std::time::Duration::new(10, 0);
 
         let file =
-            proxmox_sys::fs::open_file_locked(&self.task_lock_fn, timeout, exclusive, options)?;
+            proxmox_sys::fs::open_file_locked(&self.task_lock_fn, timeout, exclusive, &options)?;
 
         Ok(TaskListLockGuard(file))
     }
@@ -107,7 +107,7 @@ impl WorkerTaskSetup {
             .clone()
             .perm(nix::sys::stat::Mode::from_bits_truncate(0o755));
 
-        create_path(&path, None, Some(dir_opts))?;
+        create_path(&path, None, Some(&dir_opts))?;
 
         path.push(upid.to_string());
         Ok(path)
@@ -166,7 +166,7 @@ impl WorkerTaskSetup {
             .clone()
             .perm(nix::sys::stat::Mode::from_bits_truncate(0o660));
 
-        replace_file(&self.active_tasks_fn, active_raw.as_bytes(), options, false)?;
+        replace_file(&self.active_tasks_fn, active_raw.as_bytes(), &options, false)?;
 
         finish_list.sort_unstable_by(|a, b| match (&a.state, &b.state) {
             (Some(s1), Some(s2)) => s1.cmp(s2),
@@ -185,7 +185,7 @@ impl WorkerTaskSetup {
                 &self.task_archive_fn,
                 OFlag::O_APPEND | OFlag::O_RDWR,
                 &[],
-                options,
+                &options,
                 false,
             )?;
             for info in &finish_list {
@@ -212,7 +212,7 @@ impl WorkerTaskSetup {
                 .clone()
                 .perm(nix::sys::stat::Mode::from_bits_truncate(0o755));
 
-            create_path(&self.taskdir, Some(dir_opts.clone()), Some(dir_opts))?;
+            create_path(&self.taskdir, Some(&dir_opts), Some(&dir_opts))?;
             // fixme:??? create_path(pbs_buildcfg::PROXMOX_BACKUP_RUN_DIR, None, Some(opts))?;
             Ok(())
         })
diff --git a/proxmox-rrd/src/cache.rs b/proxmox-rrd/src/cache.rs
index e3d2f8d5..683c8c4e 100644
--- a/proxmox-rrd/src/cache.rs
+++ b/proxmox-rrd/src/cache.rs
@@ -67,8 +67,8 @@ impl Cache {
 
         create_path(
             &basedir,
-            Some(dir_options.clone()),
-            Some(dir_options.clone()),
+            Some(&dir_options),
+            Some(&dir_options),
         )
         .map_err(|err: Error| format_err!("unable to create rrdb stat dir - {}", err))?;
 
diff --git a/proxmox-rrd/src/cache/journal.rs b/proxmox-rrd/src/cache/journal.rs
index c196b342..447c0690 100644
--- a/proxmox-rrd/src/cache/journal.rs
+++ b/proxmox-rrd/src/cache/journal.rs
@@ -116,7 +116,7 @@ impl JournalState {
             &journal_path,
             flags,
             &[],
-            self.config.file_options.clone(),
+            &self.config.file_options,
             false,
         )?;
         Ok(BufReader::new(journal))
@@ -131,7 +131,7 @@ impl JournalState {
             &journal_path,
             flags,
             &[],
-            config.file_options.clone(),
+            &config.file_options,
             false,
         )?;
         Ok(journal)
diff --git a/proxmox-rrd/src/cache/rrd_map.rs b/proxmox-rrd/src/cache/rrd_map.rs
index 881b3987..8c09e2af 100644
--- a/proxmox-rrd/src/cache/rrd_map.rs
+++ b/proxmox-rrd/src/cache/rrd_map.rs
@@ -46,8 +46,8 @@ impl RRDMap {
             path.push(rel_path);
             create_path(
                 path.parent().unwrap(),
-                Some(self.config.dir_options.clone()),
-                Some(self.config.dir_options.clone()),
+                Some(&self.config.dir_options),
+                Some(&self.config.dir_options),
             )?;
 
             let mut rrd = (self.load_rrd_cb)(&path, rel_path, dst);
diff --git a/proxmox-rrd/src/rrd.rs b/proxmox-rrd/src/rrd.rs
index 440abe06..2e8f0f06 100644
--- a/proxmox-rrd/src/rrd.rs
+++ b/proxmox-rrd/src/rrd.rs
@@ -424,7 +424,7 @@ impl Database {
         options: CreateOptions,
         avoid_page_cache: bool,
     ) -> Result<(), Error> {
-        let (fd, tmp_path) = make_tmp_file(path, options)?;
+        let (fd, tmp_path) = make_tmp_file(path, &options)?;
         let mut file = unsafe { std::fs::File::from_raw_fd(fd.into_raw_fd()) };
 
         let mut try_block = || -> Result<(), Error> {
diff --git a/proxmox-subscription/src/files.rs b/proxmox-subscription/src/files.rs
index 37008f4a..f0ff9bf4 100644
--- a/proxmox-subscription/src/files.rs
+++ b/proxmox-subscription/src/files.rs
@@ -127,7 +127,7 @@ pub fn write_subscription<P: AsRef<Path>>(
         format!("{}\n{}\n{}\n", info.key.as_ref().unwrap(), csum, encoded)
     };
 
-    replace_file(path, raw.as_bytes(), file_opts, true)?;
+    replace_file(path, raw.as_bytes(), &file_opts, true)?;
 
     Ok(())
 }
@@ -152,7 +152,7 @@ pub fn update_apt_auth<P: AsRef<Path>>(
             let conf = format!("machine {url}\n login {}\n password {}\n", key, password,);
 
             // we use a namespaced .conf file, so just overwrite..
-            replace_file(path, conf.as_bytes(), file_opts, true)
+            replace_file(path, conf.as_bytes(), &file_opts, true)
                 .map_err(|e| format_err!("Error saving apt auth config - {}", e))?;
         }
         _ => match std::fs::remove_file(path) {
diff --git a/proxmox-sys/src/fs/dir.rs b/proxmox-sys/src/fs/dir.rs
index c39b9fdf..8e75111a 100644
--- a/proxmox-sys/src/fs/dir.rs
+++ b/proxmox-sys/src/fs/dir.rs
@@ -14,7 +14,7 @@ use crate::fs::{fchown, CreateOptions};
 /// Creates directory at the provided path with specified ownership.
 ///
 /// Errors if the directory already exists.
-pub fn create_dir<P: AsRef<Path>>(path: P, options: CreateOptions) -> Result<(), nix::Error> {
+pub fn create_dir<P: AsRef<Path>>(path: P, options: &CreateOptions) -> Result<(), nix::Error> {
     // clippy bug?: from_bits_truncate is actually a const fn...
     #[allow(clippy::or_fun_call)]
     let mode: stat::Mode = options
@@ -87,7 +87,7 @@ pub fn ensure_dir_exists<P: AsRef<Path>>(
 /// create_path(
 ///     "/var/lib/mytool/wwwdata",
 ///     None,
-///     Some(CreateOptions::new()
+///     Some(&CreateOptions::new()
 ///         .perm(Mode::from_bits(0o777).unwrap())
 ///         .owner(Uid::from_raw(33))
 ///     ),
@@ -97,16 +97,16 @@ pub fn ensure_dir_exists<P: AsRef<Path>>(
 /// ```
 pub fn create_path<P: AsRef<Path>>(
     path: P,
-    intermediate_opts: Option<CreateOptions>,
-    final_opts: Option<CreateOptions>,
+    intermediate_opts: Option<&CreateOptions>,
+    final_opts: Option<&CreateOptions>,
 ) -> Result<bool, Error> {
     create_path_do(path.as_ref(), intermediate_opts, final_opts)
 }
 
 fn create_path_do(
     path: &Path,
-    intermediate_opts: Option<CreateOptions>,
-    final_opts: Option<CreateOptions>,
+    intermediate_opts: Option<&CreateOptions>,
+    final_opts: Option<&CreateOptions>,
 ) -> Result<bool, Error> {
     use std::path::Component;
 
@@ -146,8 +146,8 @@ fn create_path_do(
 fn create_path_at_do(
     mut at: OwnedFd,
     mut iter: std::iter::Peekable<std::path::Components>,
-    intermediate_opts: Option<CreateOptions>,
-    final_opts: Option<CreateOptions>,
+    intermediate_opts: Option<&CreateOptions>,
+    final_opts: Option<&CreateOptions>,
 ) -> Result<bool, Error> {
     let mut created = false;
     loop {
@@ -206,9 +206,9 @@ mod tests {
     fn test_create_path() {
         create_path(
             "testdir/testsub/testsub2/testfinal",
-            Some(CreateOptions::new().perm(stat::Mode::from_bits_truncate(0o755))),
+            Some(&CreateOptions::new().perm(stat::Mode::from_bits_truncate(0o755))),
             Some(
-                CreateOptions::new()
+                &CreateOptions::new()
                     .owner(nix::unistd::Uid::effective())
                     .group(nix::unistd::Gid::effective()),
             ),
diff --git a/proxmox-sys/src/fs/file.rs b/proxmox-sys/src/fs/file.rs
index ac513891..41915ebb 100644
--- a/proxmox-sys/src/fs/file.rs
+++ b/proxmox-sys/src/fs/file.rs
@@ -128,7 +128,7 @@ pub fn file_read_firstline<P: AsRef<Path>>(path: P) -> Result<String, Error> {
 /// a RawFd and PathBuf for it
 pub fn make_tmp_file<P: AsRef<Path>>(
     path: P,
-    options: CreateOptions,
+    options: &CreateOptions,
 ) -> Result<(File, PathBuf), Error> {
     let path = path.as_ref();
 
@@ -159,7 +159,7 @@ pub fn make_tmp_file<P: AsRef<Path>>(
 pub fn replace_file<P: AsRef<Path>>(
     path: P,
     data: &[u8],
-    options: CreateOptions,
+    options: &CreateOptions,
     fsync: bool,
 ) -> Result<(), Error> {
     let (fd, tmp_path) = make_tmp_file(&path, options)?;
@@ -204,7 +204,7 @@ pub fn atomic_open_or_create_file<P: AsRef<Path>>(
     path: P,
     mut oflag: OFlag,
     initial_data: &[u8],
-    options: CreateOptions,
+    options: &CreateOptions,
     fsync: bool,
 ) -> Result<File, Error> {
     let path = path.as_ref();
@@ -410,7 +410,7 @@ pub fn open_file_locked<P: AsRef<Path>>(
     path: P,
     timeout: Duration,
     exclusive: bool,
-    options: CreateOptions,
+    options: &CreateOptions,
 ) -> Result<File, Error> {
     let path = path.as_ref();
 
diff --git a/proxmox-sys/src/logrotate.rs b/proxmox-sys/src/logrotate.rs
index 704a18ce..af09723f 100644
--- a/proxmox-sys/src/logrotate.rs
+++ b/proxmox-sys/src/logrotate.rs
@@ -64,7 +64,7 @@ impl LogRotate {
         options: &CreateOptions,
     ) -> Result<(), Error> {
         let mut source = File::open(source_path)?;
-        let (fd, tmp_path) = make_tmp_file(target_path, options.clone())?;
+        let (fd, tmp_path) = make_tmp_file(target_path, options)?;
         let target = unsafe { File::from_raw_fd(fd.into_raw_fd()) };
         let mut encoder = match zstd::stream::write::Encoder::new(target, 0) {
             Ok(encoder) => encoder,
diff --git a/proxmox-system-management-api/src/dns/resolv_conf.rs b/proxmox-system-management-api/src/dns/resolv_conf.rs
index 50419c50..4016e777 100644
--- a/proxmox-system-management-api/src/dns/resolv_conf.rs
+++ b/proxmox-system-management-api/src/dns/resolv_conf.rs
@@ -138,7 +138,7 @@ pub fn update_dns(
         data.push_str(&options);
     }
 
-    replace_file(RESOLV_CONF_FN, data.as_bytes(), CreateOptions::new(), true)?;
+    replace_file(RESOLV_CONF_FN, data.as_bytes(), &CreateOptions::new(), true)?;
 
     Ok(())
 }
-- 
2.39.2




More information about the pbs-devel mailing list