[pbs-devel] [PATCH proxmox-backup] fix #5967: datastore lookup: check for disappeared datastores

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

some code paths will only read the datastore base - if the datastore
disappeared (e.g., a lazy unmount, or some network storage disappearing without
blocking) then we should at least detect the issue at lookup time before
re-using an already opened datastore.

any code path that tried to access the chunks themselves would already have
failed without this additional safeguard, but in particular queries listing
namespaces or groups in the root namespace don't, which might cause a remote
client such as pull sync to mistakenly think that the datastore is empty.

limit the additional check to `Read` or `Write` lookups, since any code path
doing those will most likely want to do I/O on the datastore afterwards, as
opposed to a `Lookup` lookup or one without an operation, which might not
require the datastore to be fully functional.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
there is of course still a gap beween the lookup and the readdir after it.. we
could track down all API endpoints that access the datastore but don't touch
the chunk dir, and before returning, check a second time that the datastore
hasn't vanished, but I am not sure that is worth the effort?

 pbs-datastore/src/chunk_store.rs | 2 +-
 pbs-datastore/src/datastore.rs   | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
index 29d5874a1..189c4ebcd 100644
--- a/pbs-datastore/src/chunk_store.rs
+++ b/pbs-datastore/src/chunk_store.rs
@@ -158,7 +158,7 @@ impl ChunkStore {
 
     /// Check if the chunkstore path is absolute and that we can
     /// access it. Returns the absolute '.chunks' path on success.
-    fn chunk_dir_accessible(base: &Path) -> Result<PathBuf, Error> {
+    pub(crate) fn chunk_dir_accessible(base: &Path) -> Result<PathBuf, Error> {
         if !base.is_absolute() {
             bail!("expected absolute path - got {:?}", base);
         }
diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 0801b4bf6..bb9b4471f 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -232,6 +232,12 @@ impl DataStore {
         // reuse chunk store so that we keep using the same process locker instance!
         let chunk_store = if let Some(datastore) = &entry {
             let last_digest = datastore.last_digest.as_ref();
+            match operation {
+                Some(Operation::Read) | Some(Operation::Write) => {
+                    ChunkStore::chunk_dir_accessible(&datastore.chunk_store.base_path())?;
+                }
+                Some(Operation::Lookup) | None => {}
+            }
             if let Some(true) = last_digest.map(|last_digest| last_digest == &digest) {
                 if let Some(operation) = operation {
                     update_active_operations(name, operation, 1)?;
-- 
2.39.5



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