all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Christian Ebner <c.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup 1/2] api: datastore: make group notes path helper a DataStore method
Date: Wed,  7 May 2025 17:38:33 +0200	[thread overview]
Message-ID: <20250507153834.758840-2-c.ebner@proxmox.com> (raw)
In-Reply-To: <20250507153834.758840-1-c.ebner@proxmox.com>

Move and make the helper function to get a backup groups notes file
path a `DataStore` method instead. This allows it to be reused when
access to the notes path is required from the datastore itself.

Further, use the plural `notes` wording also in the helper to be
consistent with the rest of the codebase.

In preparation for correctly removing the notes file from the backup
group on destruction.

No functional changes intended.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/datastore.rs | 11 +++++++++++
 src/api2/admin/datastore.rs    | 26 +++++++-------------------
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index cbf78ecb6..91c7e76be 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -40,6 +40,8 @@ use crate::DataBlob;
 static DATASTORE_MAP: LazyLock<Mutex<HashMap<String, Arc<DataStoreImpl>>>> =
     LazyLock::new(|| Mutex::new(HashMap::new()));
 
+const GROUP_NOTES_FILE_NAME: &str = "notes";
+
 /// checks if auth_id is owner, or, if owner is a token, if
 /// auth_id is the user of the token
 pub fn check_backup_owner(owner: &Authid, auth_id: &Authid) -> Result<(), Error> {
@@ -524,6 +526,15 @@ impl DataStore {
         full_path
     }
 
+    /// Returns the absolute path of a backup groups notes file
+    pub fn group_notes_path(
+        &self,
+        ns: &BackupNamespace,
+        group: &pbs_api_types::BackupGroup,
+    ) -> PathBuf {
+        self.group_path(ns, group).join(GROUP_NOTES_FILE_NAME)
+    }
+
     /// Returns the absolute path for backup_dir
     pub fn snapshot_path(
         &self,
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 392494488..cc7e17a29 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -4,7 +4,7 @@ use std::collections::HashSet;
 use std::ffi::OsStr;
 use std::ops::Deref;
 use std::os::unix::ffi::OsStrExt;
-use std::path::{Path, PathBuf};
+use std::path::Path;
 use std::sync::Arc;
 
 use anyhow::{bail, format_err, Context, Error};
@@ -77,18 +77,6 @@ use crate::backup::{
 
 use crate::server::jobstate::{compute_schedule_status, Job, JobState};
 
-const GROUP_NOTES_FILE_NAME: &str = "notes";
-
-fn get_group_note_path(
-    store: &DataStore,
-    ns: &BackupNamespace,
-    group: &pbs_api_types::BackupGroup,
-) -> PathBuf {
-    let mut note_path = store.group_path(ns, group);
-    note_path.push(GROUP_NOTES_FILE_NAME);
-    note_path
-}
-
 // helper to unify common sequence of checks:
 // 1. check privs on NS (full or limited access)
 // 2. load datastore
@@ -244,8 +232,8 @@ pub fn list_groups(
                 })
                 .to_owned();
 
-            let note_path = get_group_note_path(&datastore, &ns, group.as_ref());
-            let comment = file_read_firstline(note_path).ok();
+            let notes_path = datastore.group_notes_path(&ns, group.as_ref());
+            let comment = file_read_firstline(notes_path).ok();
 
             group_info.push(GroupListItem {
                 backup: group.into(),
@@ -2053,8 +2041,8 @@ pub fn get_group_notes(
         &backup_group,
     )?;
 
-    let note_path = get_group_note_path(&datastore, &ns, &backup_group);
-    Ok(file_read_optional_string(note_path)?.unwrap_or_else(|| "".to_owned()))
+    let notes_path = datastore.group_notes_path(&ns, &backup_group);
+    Ok(file_read_optional_string(notes_path)?.unwrap_or_else(|| "".to_owned()))
 }
 
 #[api(
@@ -2101,8 +2089,8 @@ pub fn set_group_notes(
         &backup_group,
     )?;
 
-    let note_path = get_group_note_path(&datastore, &ns, &backup_group);
-    replace_file(note_path, notes.as_bytes(), CreateOptions::new(), false)?;
+    let notes_path = datastore.group_notes_path(&ns, &backup_group);
+    replace_file(notes_path, notes.as_bytes(), CreateOptions::new(), false)?;
 
     Ok(())
 }
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


  reply	other threads:[~2025-05-07 15:38 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-07 15:38 [pbs-devel] [PATCH proxmox-backup 0/2] fix #6358: group removal fails if group notes exist Christian Ebner
2025-05-07 15:38 ` Christian Ebner [this message]
2025-05-07 15:38 ` [pbs-devel] [PATCH proxmox-backup 2/2] fix #6358: remove group note file if present on group destroy Christian Ebner
2025-06-04 11:31 ` [pbs-devel] applied-series: [PATCH proxmox-backup 0/2] fix #6358: group removal fails if group notes exist Fabian Grünbichler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250507153834.758840-2-c.ebner@proxmox.com \
    --to=c.ebner@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal