all lists on 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal