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 50CA61FF170 for ; Thu, 24 Jul 2025 10:02:03 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A3AC53283E; Thu, 24 Jul 2025 10:03:22 +0200 (CEST) From: Christian Ebner To: pbs-devel@lists.proxmox.com Date: Thu, 24 Jul 2025 10:02:33 +0200 Message-ID: <20250724080233.282783-1-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.2 MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1753344167089 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.043 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH proxmox-backup 1/1] api: datastore: fix cache store creation when reusing s3 backend 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" Commit 3cc3c10d ("datastore: mark store as in-use by setting marker on s3 backend") introduced the marker object on datastores used by another instance. The check was however flawed as it made the local chunk store creation dependent on the s3 client instantiation. Therefore, instead factor out the DatastoreBackendType determination, use that for the check and never assume the local cache store to be pre-existing. Also, since contents from the s3 store are refreshed anyway, local contents in the cache store will be removed, except chunks which are now cleaned up on create. Fixes: 3cc3c10d ("datastore: mark store as in-use by setting marker on s3 backend") Signed-off-by: Christian Ebner --- src/api2/config/datastore.rs | 119 +++++++++++++++++++---------------- 1 file changed, 64 insertions(+), 55 deletions(-) diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs index f7b852cb7..d24323152 100644 --- a/src/api2/config/datastore.rs +++ b/src/api2/config/datastore.rs @@ -131,57 +131,54 @@ pub(crate) fn do_create_datastore( .parse_property_string(datastore.tuning.as_deref().unwrap_or(""))?, )?; - let mut backend_s3_client = None; - if let Some(ref backend_config) = datastore.backend { - let backend_config: DatastoreBackendConfig = backend_config.parse()?; - match backend_config.ty.unwrap_or_default() { - DatastoreBackendType::Filesystem => (), - DatastoreBackendType::S3 => { - if !overwrite_in_use { - let s3_client_id = backend_config - .client - .as_ref() - .ok_or_else(|| format_err!("missing required client"))?; - let bucket = backend_config - .bucket - .clone() - .ok_or_else(|| format_err!("missing required bucket"))?; - let (config, _config_digest) = - pbs_config::s3::config().context("failed to get s3 config")?; - let config: S3ClientConf = config - .lookup(S3_CFG_TYPE_ID, s3_client_id) - .with_context(|| format!("no '{s3_client_id}' in config"))?; - let options = S3ClientOptions::from_config( - config.config, - config.secret_key, - bucket, - datastore.name.to_owned(), - ); - let s3_client = S3Client::new(options).context("failed to create s3 client")?; - - let object_key = S3ObjectKey::try_from(S3_DATASTORE_IN_USE_MARKER) - .context("failed to generate s3 object key")?; - if let Some(response) = - proxmox_async::runtime::block_on(s3_client.get_object(object_key.clone())) - .context("failed to get in-use marker from bucket")? - { - let content = proxmox_async::runtime::block_on(response.content.collect()) - .unwrap_or_default(); - let content = - String::from_utf8(content.to_bytes().to_vec()).unwrap_or_default(); - let in_use: InUseContent = - serde_json::from_str(&content).unwrap_or_default(); - if let Some(hostname) = in_use.hostname { - bail!("Bucket already contains datastore in use by host {hostname}"); - } else { - bail!("Bucket already contains datastore in use"); - } - } - backend_s3_client = Some(Arc::new(s3_client)); + let backend_config: DatastoreBackendConfig = + datastore.backend.as_deref().unwrap_or("").parse()?; + let backend_type = backend_config.ty.unwrap_or_default(); + let backend_s3_client = if backend_type == DatastoreBackendType::S3 { + let s3_client_id = backend_config + .client + .as_ref() + .ok_or_else(|| format_err!("missing required client"))?; + let bucket = backend_config + .bucket + .clone() + .ok_or_else(|| format_err!("missing required bucket"))?; + let (config, _config_digest) = + pbs_config::s3::config().context("failed to get s3 config")?; + let config: S3ClientConf = config + .lookup(S3_CFG_TYPE_ID, s3_client_id) + .with_context(|| format!("no '{s3_client_id}' in config"))?; + let options = S3ClientOptions::from_config( + config.config, + config.secret_key, + bucket, + datastore.name.to_owned(), + ); + let s3_client = S3Client::new(options).context("failed to create s3 client")?; + + if !overwrite_in_use { + let object_key = S3ObjectKey::try_from(S3_DATASTORE_IN_USE_MARKER) + .context("failed to generate s3 object key")?; + if let Some(response) = + proxmox_async::runtime::block_on(s3_client.get_object(object_key.clone())) + .context("failed to get in-use marker from bucket")? + { + let content = proxmox_async::runtime::block_on(response.content.collect()) + .unwrap_or_default(); + let content = String::from_utf8(content.to_bytes().to_vec()).unwrap_or_default(); + let in_use: InUseContent = serde_json::from_str(&content).unwrap_or_default(); + if let Some(hostname) = in_use.hostname { + bail!("Bucket already contains datastore in use by host {hostname}"); + } else { + bail!("Bucket already contains datastore in use"); } } } - } + + Some(Arc::new(s3_client)) + } else { + None + }; let unmount_guard = if datastore.backing_device.is_some() { do_mount_device(datastore.clone())?; @@ -190,7 +187,7 @@ pub(crate) fn do_create_datastore( UnmountGuard::new(None) }; - let chunk_store = if reuse_datastore && backend_s3_client.is_none() { + let chunk_store = if reuse_datastore && backend_type == DatastoreBackendType::Filesystem { ChunkStore::verify_chunkstore(&path).and_then(|_| { // Must be the only instance accessing and locking the chunk store, // dropping will close all other locks from this process on the lockfile as well. @@ -201,12 +198,24 @@ pub(crate) fn do_create_datastore( ) })? } else { - if let Ok(dir) = std::fs::read_dir(&path) { - for file in dir { - let name = file?.file_name(); - let name = name.to_str(); - if !name.is_some_and(|name| name.starts_with('.') || name == "lost+found") { - bail!("datastore path not empty"); + if !reuse_datastore && backend_type == DatastoreBackendType::Filesystem { + if let Ok(dir) = std::fs::read_dir(&path) { + for file in dir { + let name = file?.file_name(); + let name = name.to_str(); + if !name.is_some_and(|name| name.starts_with('.') || name == "lost+found") { + bail!("datastore path not empty"); + } + } + } + } + if reuse_datastore && backend_type == DatastoreBackendType::S3 { + let chunks_path = path.join(".chunks"); + if let Err(err) = std::fs::remove_dir_all(&chunks_path) { + if err.kind() != std::io::ErrorKind::NotFound { + return Err(err).with_context(|| { + format!("failed to remove pre-existing chunks in {chunks_path:?}") + }); } } } -- 2.47.2 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel