Re: [pbs-devel] [PATCH proxmox-backup v2 4/8] chunk store: return chunk extension and check for used marker

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

On December 11, 2025 4:38 pm, Christian Ebner wrote:
> Clearly distinguish the cases for `bad` and `using` extensions for
> items returned by the chunk iterator. Directory entries with
> filenames which matched the expected length but not the extension
> are now skipped over.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
>  pbs-datastore/src/chunk_store.rs | 48 ++++++++++++++++++++++++--------
>  1 file changed, 37 insertions(+), 11 deletions(-)
> 
> diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
> index 7980938ad..ad517391d 100644
> --- a/pbs-datastore/src/chunk_store.rs
> +++ b/pbs-datastore/src/chunk_store.rs
> @@ -280,7 +280,11 @@ impl ChunkStore {
>          &self,
>      ) -> Result<
>          impl std::iter::FusedIterator<
> -            Item = (Result<proxmox_sys::fs::ReadDirEntry, Error>, usize, bool),
> +            Item = (
> +                Result<proxmox_sys::fs::ReadDirEntry, Error>,
> +                usize,
> +                ChunkExt,
> +            ),
>          >,
>          Error,
>      > {
> @@ -318,14 +322,20 @@ impl ChunkStore {
>  
>                              if bytes.len() == 64 && bytes.iter().take(64).all(u8::is_ascii_hexdigit)
>                              {
> -                                return Some((Ok(entry), percentage, false));
> +                                return Some((Ok(entry), percentage, ChunkExt::None));
>                              }
>  
>                              if bytes.len() == 64 + ".0.bad".len()
>                                  && bytes.iter().take(64).all(u8::is_ascii_hexdigit)
>                              {
> -                                let bad = bytes.ends_with(b".bad");
> -                                return Some((Ok(entry), percentage, bad));
> +                                let chunk_ext = if bytes.ends_with(b".bad") {
> +                                    ChunkExt::Bad
> +                                } else if bytes.ends_with(USING_MARKER_FILENAME_EXT.as_bytes()) {
> +                                    ChunkExt::UsedMarker
> +                                } else {
> +                                    continue;
> +                                };

this should be a match (on length and filename) or multiple conditions
checking length and extension *each*, because this code is just working
by chance (USING_MARKER_FILENAME_EXT and "0.bad" having the same length
is not enforced here!).

> +                                return Some((Ok(entry), percentage, chunk_ext));
>                              }
>  
>                              continue;
> @@ -334,7 +344,7 @@ impl ChunkStore {
>                              // stop after first error
>                              done = true;
>                              // and pass the error through:
> -                            return Some((Err(err), percentage, false));
> +                            return Some((Err(err), percentage, ChunkExt::None));
>                          }
>                          None => (), // open next directory
>                      }
> @@ -367,7 +377,7 @@ impl ChunkStore {
>                          return Some((
>                              Err(format_err!("unable to read subdir '{subdir}' - {err}")),
>                              percentage,
> -                            false,
> +                            ChunkExt::None,
>                          ));
>                      }
>                  }
> @@ -402,7 +412,8 @@ impl ChunkStore {
>          let mut last_percentage = 0;
>          let mut chunk_count = 0;
>  
> -        for (entry, percentage, bad) in self.get_chunk_store_iterator()? {
> +        for (entry, percentage, chunk_ext) in self.get_chunk_store_iterator()? {
> +            let bad = chunk_ext.is_bad();
>              if last_percentage != percentage {
>                  last_percentage = percentage;
>                  info!("processed {percentage}% ({chunk_count} chunks)");
> @@ -433,10 +444,7 @@ impl ChunkStore {
>                      drop(lock);
>                      continue;
>                  }
> -                if filename
> -                    .to_bytes()
> -                    .ends_with(USING_MARKER_FILENAME_EXT.as_bytes())
> -                {
> +                if chunk_ext.is_used_marker() {
>                      unlinkat(Some(dirfd), filename, UnlinkatFlags::NoRemoveDir).map_err(|err| {
>                          format_err!("unlinking chunk using marker {filename:?} failed - {err}")
>                      })?;
> @@ -908,6 +916,24 @@ impl ChunkStore {
>      }
>  }
>  
> +#[derive(PartialEq)]
> +/// Chunk iterator directory entry filename extension
> +enum ChunkExt {
> +    None,
> +    Bad,
> +    UsedMarker,
> +}
> +
> +impl ChunkExt {
> +    fn is_bad(&self) -> bool {
> +        *self == Self::Bad
> +    }
> +
> +    fn is_used_marker(&self) -> bool {
> +        *self == Self::UsedMarker
> +    }
> +}

I am not sure if those two really warrant their own helpers?

> +
>  #[test]
>  fn test_chunk_store1() {
>      let mut path = std::fs::canonicalize(".").unwrap(); // we need absolute path
> -- 
> 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