From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 9585E1FF133 for ; Mon, 11 May 2026 11:23:58 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0730DF71B; Mon, 11 May 2026 11:23:58 +0200 (CEST) Message-ID: <1b037d5b-8dca-479d-a6f3-79a96d6e5b60@proxmox.com> Date: Mon, 11 May 2026 11:23:49 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox-backup 3/4] datastore: move try_ensure_sync_level() to DataStoreImpl To: Christian Ebner , pbs-devel@lists.proxmox.com References: <20260508122912.395304-1-c.ebner@proxmox.com> <20260508122912.395304-4-c.ebner@proxmox.com> Content-Language: en-US, de-AT From: Robert Obkircher In-Reply-To: <20260508122912.395304-4-c.ebner@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1778491318278 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.055 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [datastore.rs] Message-ID-Hash: 4BTXWPIRHIYGSTVPBIRSZX4ZO57DJPB2 X-Message-ID-Hash: 4BTXWPIRHIYGSTVPBIRSZX4ZO57DJPB2 X-MailFrom: r.obkircher@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 08.05.26 14:28, Christian Ebner wrote: > Keep the public interface in place, but move the method to > DataStoreImpl so it can be reused from the inner context as well. > > This is in preparation for fixing the sync level updates not being > propagated to the chunk store. > > Signed-off-by: Christian Ebner > --- > pbs-datastore/src/datastore.rs | 24 ++++++++++++++---------- > 1 file changed, 14 insertions(+), 10 deletions(-) > > diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs > index bc218cb94..8f69dd7ac 100644 > --- a/pbs-datastore/src/datastore.rs > +++ b/pbs-datastore/src/datastore.rs > @@ -234,6 +234,19 @@ impl DataStoreImpl { > thresholds_exceeded_callback: None, > }) > } > + > + fn try_ensure_sync_level(&self) -> Result<(), Error> { > + if self.sync_level != DatastoreFSyncLevel::Filesystem { If the level in the chunk store changes from File to Filesystem this still wouldn't sync the chunks that have been written afterwards. It may be better to just move the entire method into the chunk store. > + return Ok(()); > + } > + let file = std::fs::File::open(self.chunk_store.base_path())?; > + let fd = file.as_raw_fd(); > + log::info!("syncing filesystem"); > + if unsafe { libc::syncfs(fd) } < 0 { > + bail!("error during syncfs: {}", std::io::Error::last_os_error()); > + } > + Ok(()) > + } > } > > pub type ThresholdsExceededCallback = fn(&str, &str, u64, u64) -> Result<(), Error>; > @@ -3015,16 +3028,7 @@ impl DataStore { > /// Syncs the filesystem of the datastore if 'sync_level' is set to > /// [`DatastoreFSyncLevel::Filesystem`]. Uses syncfs(2). > pub fn try_ensure_sync_level(&self) -> Result<(), Error> { > - if self.inner.sync_level != DatastoreFSyncLevel::Filesystem { > - return Ok(()); > - } > - let file = std::fs::File::open(self.base_path())?; > - let fd = file.as_raw_fd(); > - log::info!("syncing filesystem"); > - if unsafe { libc::syncfs(fd) } < 0 { > - bail!("error during syncfs: {}", std::io::Error::last_os_error()); > - } > - Ok(()) > + self.inner.try_ensure_sync_level() > } > > /// Destroy a datastore. This requires that there are no active operations on the datastore.