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

[Christian Ebner <c.ebner@proxmox.com>]

On 1/13/26 11:24 AM, Fabian Grünbichler wrote:
> 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!).

Okay, as also mentioned in the previous patch, since this is all being 
done in the unlikely brach, adding much stricter checks here is probably 
fine, so I will extend them to do more precise filename extension matching.

> 
>> +                                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?

Ack, will inline them instead.

> 
>> +
>>   #[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
> 
> 



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