From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id C6F611FF187 for ; Mon, 3 Nov 2025 15:51:20 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 486C820247; Mon, 3 Nov 2025 15:51:59 +0100 (CET) Date: Mon, 03 Nov 2025 15:51:23 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion References: <20251103113120.239455-1-c.ebner@proxmox.com> <20251103113120.239455-3-c.ebner@proxmox.com> In-Reply-To: <20251103113120.239455-3-c.ebner@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.17.0 (https://github.com/astroidmail/astroid) Message-Id: <1762174415.18lfpr185h.astroid@yuna.none> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1762181469580 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.048 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment 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 02/17] api/datastore: move group notes setting to the datastore 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: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" On November 3, 2025 12:31 pm, Christian Ebner wrote: > In an effort to abstract away the datastore backend related logic > from the api, move the set_group_notes to a helper method on the > datastore. > > The new helper method now also assures that the exclusive lock on > the backup group is acquired before updating the notes, in order to > avoid possible race conditions, e.g. with backup group destruction. > > Signed-off-by: Christian Ebner > --- > pbs-datastore/src/datastore.rs | 26 ++++++++++++++++++++++++++ > src/api2/admin/datastore.rs | 17 ++++------------- > 2 files changed, 30 insertions(+), 13 deletions(-) > > diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs > index 127ba1c81..45f315aeb 100644 > --- a/pbs-datastore/src/datastore.rs > +++ b/pbs-datastore/src/datastore.rs > @@ -2418,4 +2418,30 @@ impl DataStore { > .map_err(|err| format_err!("{err:#}"))?; > Ok((backend_type, Some(s3_client))) > } > + > + /// Creates or updates the notes associated with a backup group. > + /// Acquires exclusive lock on the backup group. > + pub fn set_group_notes( > + self: &Arc, > + notes: String, > + backup_group: BackupGroup, > + ) -> Result<(), Error> { > + let _lock = backup_group.lock().context("failed to lock backup group")?; this takes an exclusive lock on group, which means all sorts of other operations (including creating new snapshots?) are blocked while it is held > + > + if let DatastoreBackend::S3(s3_client) = self.backend()? { > + let mut path = backup_group.backup_ns().path(); > + path.push(backup_group.group().to_string()); > + let object_key = crate::s3::object_key_from_path(&path, "notes") > + .context("invalid owner file object key")?; > + let data = hyper::body::Bytes::copy_from_slice(notes.as_bytes()); > + let _is_duplicate = proxmox_async::runtime::block_on( > + s3_client.upload_replace_with_retry(object_key, data), but this can take a while, right? FWIW, the same is true of setting the backup owner.. > + ) > + .context("failed to set notes on s3 backend")?; > + } > + let notes_path = self.group_notes_path(backup_group.backup_ns(), backup_group.group()); > + replace_file(notes_path, notes.as_bytes(), CreateOptions::new(), false) > + .context("failed to replace group notes file")?; > + Ok(()) > + } > } > diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs > index d192ee390..131cdae51 100644 > --- a/src/api2/admin/datastore.rs > +++ b/src/api2/admin/datastore.rs > @@ -2009,19 +2009,10 @@ pub fn set_group_notes( > &backup_group, > )?; > > - if let DatastoreBackend::S3(s3_client) = datastore.backend()? { > - let mut path = ns.path(); > - path.push(backup_group.to_string()); > - let object_key = pbs_datastore::s3::object_key_from_path(&path, "notes") > - .context("invalid owner file object key")?; > - let data = hyper::body::Bytes::copy_from_slice(notes.as_bytes()); > - let _is_duplicate = > - proxmox_async::runtime::block_on(s3_client.upload_replace_with_retry(object_key, data)) > - .context("failed to set notes on s3 backend")?; > - } > - let notes_path = datastore.group_notes_path(&ns, &backup_group); > - replace_file(notes_path, notes.as_bytes(), CreateOptions::new(), false)?; > - > + let backup_group = datastore.backup_group(ns, backup_group); > + datastore > + .set_group_notes(notes, backup_group) > + .map_err(|err| format_err!("failed to set group notes - {err:#?}"))?; > Ok(()) > } > > -- > 2.47.3 > > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel > > > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel