public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH v1 proxmox-backup 0/3] index file inspection fixes
@ 2026-04-28 12:21 Robert Obkircher
  2026-04-28 12:21 ` [PATCH v1 proxmox-backup 1/3] datastore: read ctime from didx header instead of using current time Robert Obkircher
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Robert Obkircher @ 2026-04-28 12:21 UTC (permalink / raw)
  To: pbs-devel

Just to minor fixes for index file inspection and a small cleanup.

Robert Obkircher (3):
  datastore: read ctime from didx header instead of using current time
  datastore: remove unnecessary pointer dereference in didx reader
  bin: debug: produce deterministic output when inspecting index files

 pbs-datastore/src/dynamic_index.rs      | 6 ++----
 src/bin/proxmox_backup_debug/inspect.rs | 4 ++--
 2 files changed, 4 insertions(+), 6 deletions(-)

-- 
2.47.3





^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v1 proxmox-backup 1/3] datastore: read ctime from didx header instead of using current time
  2026-04-28 12:21 [PATCH v1 proxmox-backup 0/3] index file inspection fixes Robert Obkircher
@ 2026-04-28 12:21 ` Robert Obkircher
  2026-04-28 14:13   ` Dominik Csapak
  2026-04-28 12:21 ` [PATCH v1 proxmox-backup 2/3] datastore: remove unnecessary pointer dereference in didx reader Robert Obkircher
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Robert Obkircher @ 2026-04-28 12:21 UTC (permalink / raw)
  To: pbs-devel

It looks like this was accidentally changed during a refactor in 2020.

The ctime field is public, so it is not obvious whether anyone depends
on the broken behavior. The index_ctime accessor only seems to be used
by inspect_file in proxmox_backup_debug.

Fixes: 6a7be83efe ("avoid chrono dependency, depend on proxmox 0.3.8")
Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
---
 pbs-datastore/src/dynamic_index.rs | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/pbs-datastore/src/dynamic_index.rs b/pbs-datastore/src/dynamic_index.rs
index e1ca0eae..288f38b5 100644
--- a/pbs-datastore/src/dynamic_index.rs
+++ b/pbs-datastore/src/dynamic_index.rs
@@ -119,8 +119,6 @@ impl DynamicIndexReader {
             bail!("got unknown magic number");
         }
 
-        let ctime = proxmox_time::epoch_i64();
-
         let index_size = stat.st_size as usize - header_size;
         let index_count = index_size / 40;
         if index_count * 40 != index_size {
@@ -141,7 +139,7 @@ impl DynamicIndexReader {
             _file: file,
             size,
             index,
-            ctime,
+            ctime: header.ctime,
             uuid: header.uuid,
             index_csum: header.index_csum,
         })
-- 
2.47.3





^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v1 proxmox-backup 2/3] datastore: remove unnecessary pointer dereference in didx reader
  2026-04-28 12:21 [PATCH v1 proxmox-backup 0/3] index file inspection fixes Robert Obkircher
  2026-04-28 12:21 ` [PATCH v1 proxmox-backup 1/3] datastore: read ctime from didx header instead of using current time Robert Obkircher
@ 2026-04-28 12:21 ` Robert Obkircher
  2026-04-28 12:21 ` [PATCH v1 proxmox-backup 3/3] bin: debug: produce deterministic output when inspecting index files Robert Obkircher
  2026-04-28 13:59 ` applied: [PATCH v1 proxmox-backup 0/3] index file inspection fixes Thomas Lamprecht
  3 siblings, 0 replies; 6+ messages in thread
From: Robert Obkircher @ 2026-04-28 12:21 UTC (permalink / raw)
  To: pbs-devel

The inner method already returns the correct reference type.

Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
---
 pbs-datastore/src/dynamic_index.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pbs-datastore/src/dynamic_index.rs b/pbs-datastore/src/dynamic_index.rs
index 288f38b5..5959b0c8 100644
--- a/pbs-datastore/src/dynamic_index.rs
+++ b/pbs-datastore/src/dynamic_index.rs
@@ -196,7 +196,7 @@ impl IndexFile for DynamicIndexReader {
         if pos >= self.index.len() {
             None
         } else {
-            Some(unsafe { &*(self.chunk_digest(pos).as_ptr() as *const [u8; 32]) })
+            Some(self.chunk_digest(pos))
         }
     }
 
-- 
2.47.3





^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v1 proxmox-backup 3/3] bin: debug: produce deterministic output when inspecting index files
  2026-04-28 12:21 [PATCH v1 proxmox-backup 0/3] index file inspection fixes Robert Obkircher
  2026-04-28 12:21 ` [PATCH v1 proxmox-backup 1/3] datastore: read ctime from didx header instead of using current time Robert Obkircher
  2026-04-28 12:21 ` [PATCH v1 proxmox-backup 2/3] datastore: remove unnecessary pointer dereference in didx reader Robert Obkircher
@ 2026-04-28 12:21 ` Robert Obkircher
  2026-04-28 13:59 ` applied: [PATCH v1 proxmox-backup 0/3] index file inspection fixes Thomas Lamprecht
  3 siblings, 0 replies; 6+ messages in thread
From: Robert Obkircher @ 2026-04-28 12:21 UTC (permalink / raw)
  To: pbs-devel

Sort the output to make it deterministic and easier to compare.

Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
---
 src/bin/proxmox_backup_debug/inspect.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/bin/proxmox_backup_debug/inspect.rs b/src/bin/proxmox_backup_debug/inspect.rs
index b5a7d3f2..221c9ebd 100644
--- a/src/bin/proxmox_backup_debug/inspect.rs
+++ b/src/bin/proxmox_backup_debug/inspect.rs
@@ -1,4 +1,4 @@
-use std::collections::HashSet;
+use std::collections::BTreeSet;
 use std::fs::File;
 use std::io::{Read, Seek, SeekFrom, Write};
 use std::path::Path;
@@ -294,7 +294,7 @@ fn inspect_file(
                 ctime_str = s;
             }
 
-            let mut chunk_digests = HashSet::new();
+            let mut chunk_digests = BTreeSet::new();
 
             for pos in 0..index.index_count() {
                 let digest = index.index_digest(pos).unwrap();
-- 
2.47.3





^ permalink raw reply related	[flat|nested] 6+ messages in thread

* applied: [PATCH v1 proxmox-backup 0/3] index file inspection fixes
  2026-04-28 12:21 [PATCH v1 proxmox-backup 0/3] index file inspection fixes Robert Obkircher
                   ` (2 preceding siblings ...)
  2026-04-28 12:21 ` [PATCH v1 proxmox-backup 3/3] bin: debug: produce deterministic output when inspecting index files Robert Obkircher
@ 2026-04-28 13:59 ` Thomas Lamprecht
  3 siblings, 0 replies; 6+ messages in thread
From: Thomas Lamprecht @ 2026-04-28 13:59 UTC (permalink / raw)
  To: pbs-devel, Robert Obkircher

On Tue, 28 Apr 2026 14:21:30 +0200, Robert Obkircher wrote:
> Just to minor fixes for index file inspection and a small cleanup.
> 
> Robert Obkircher (3):
>   datastore: read ctime from didx header instead of using current time
>   datastore: remove unnecessary pointer dereference in didx reader
>   bin: debug: produce deterministic output when inspecting index files
> 
> [...]

Applied, with minor follow-up to hedge us targetting a big-endiam platform
someday (lets hope not), thanks!

[1/3] datastore: read ctime from didx header instead of using current time
      commit: edc4bc6684bc052b5d9d72a074ed8ff6f9cf1717
[2/3] datastore: remove unnecessary pointer dereference in didx reader
      commit: 8563716bdef157b82413bb75b7f9dc38b75a6004
[3/3] bin: debug: produce deterministic output when inspecting index files
      commit: d80323360f56f7df829361f00aecba16af267729




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 proxmox-backup 1/3] datastore: read ctime from didx header instead of using current time
  2026-04-28 12:21 ` [PATCH v1 proxmox-backup 1/3] datastore: read ctime from didx header instead of using current time Robert Obkircher
@ 2026-04-28 14:13   ` Dominik Csapak
  0 siblings, 0 replies; 6+ messages in thread
From: Dominik Csapak @ 2026-04-28 14:13 UTC (permalink / raw)
  To: Robert Obkircher, pbs-devel



On 4/28/26 2:25 PM, Robert Obkircher wrote:
> It looks like this was accidentally changed during a refactor in 2020.
> 
> The ctime field is public, so it is not obvious whether anyone depends
> on the broken behavior. The index_ctime accessor only seems to be used
> by inspect_file in proxmox_backup_debug.

not a review, but I made the ctime field private and
compiled with `cargo build --all` and `cargo test --all`
and it showed now errors, so we can assume this field was never used
directly only via the index_ctime accessor

that said, the fix LGTM

> 
> Fixes: 6a7be83efe ("avoid chrono dependency, depend on proxmox 0.3.8")
> Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
> ---
>   pbs-datastore/src/dynamic_index.rs | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/pbs-datastore/src/dynamic_index.rs b/pbs-datastore/src/dynamic_index.rs
> index e1ca0eae..288f38b5 100644
> --- a/pbs-datastore/src/dynamic_index.rs
> +++ b/pbs-datastore/src/dynamic_index.rs
> @@ -119,8 +119,6 @@ impl DynamicIndexReader {
>               bail!("got unknown magic number");
>           }
>   
> -        let ctime = proxmox_time::epoch_i64();
> -
>           let index_size = stat.st_size as usize - header_size;
>           let index_count = index_size / 40;
>           if index_count * 40 != index_size {
> @@ -141,7 +139,7 @@ impl DynamicIndexReader {
>               _file: file,
>               size,
>               index,
> -            ctime,
> +            ctime: header.ctime,
>               uuid: header.uuid,
>               index_csum: header.index_csum,
>           })





^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-04-28 14:14 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-28 12:21 [PATCH v1 proxmox-backup 0/3] index file inspection fixes Robert Obkircher
2026-04-28 12:21 ` [PATCH v1 proxmox-backup 1/3] datastore: read ctime from didx header instead of using current time Robert Obkircher
2026-04-28 14:13   ` Dominik Csapak
2026-04-28 12:21 ` [PATCH v1 proxmox-backup 2/3] datastore: remove unnecessary pointer dereference in didx reader Robert Obkircher
2026-04-28 12:21 ` [PATCH v1 proxmox-backup 3/3] bin: debug: produce deterministic output when inspecting index files Robert Obkircher
2026-04-28 13:59 ` applied: [PATCH v1 proxmox-backup 0/3] index file inspection fixes Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal