[PATCH v2 proxmox-backup 0/3] improve fstatat error handling

[PATCH v2 proxmox-backup 0/3] improve fstatat error handling

[Robert Obkircher]

Changes since v1:
* shortened Err(Error::from(err).context to Err(err).context
* prefixed log message with "warning: "
* added the bugzilla id and url

[v1] https://lore.proxmox.com/pbs-devel/20260506115016.171006-1-r.obkircher@proxmox.com

Robert Obkircher (3):
  client: pxar: improve variable names
  client: pxar: consistently ignore vanished files and warn about them
  fix #7658: client: pxar: skip files on fstatat permission errors

 pbs-client/src/pxar/create.rs | 45 ++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 24 deletions(-)

-- 
2.47.3

[PATCH v2 proxmox-backup 1/3] client: pxar: improve variable names

[Robert Obkircher]

The Option can hold at most one result, and the closure computes more
than just the mode.

Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
---
 pbs-client/src/pxar/create.rs | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs
index 304de4d83..cab18e45b 100644
--- a/pbs-client/src/pxar/create.rs
+++ b/pbs-client/src/pxar/create.rs
@@ -676,9 +676,9 @@ impl Archiver {
 
             let match_path = PathBuf::from("/").join(full_path.clone());
 
-            let mut stat_results: Option<FileStat> = None;
+            let mut stat_result: Option<FileStat> = None;
 
-            let get_file_mode = || {
+            let do_stat = || {
                 nix::sys::stat::fstatat(
                     Some(dir_fd),
                     file_name,
@@ -689,9 +689,9 @@ impl Archiver {
             let match_result = self
                 .patterns
                 .matches(match_path.as_os_str().as_bytes(), || {
-                    Ok::<_, Errno>(match &stat_results {
+                    Ok::<_, Errno>(match &stat_result {
                         Some(result) => result.st_mode,
-                        None => stat_results.insert(get_file_mode()?).st_mode,
+                        None => stat_result.insert(do_stat()?).st_mode,
                     })
                 });
 
@@ -711,10 +711,10 @@ impl Archiver {
                 }
             }
 
-            let stat = match stat_results {
-                Some(mode) => mode,
-                None => match get_file_mode() {
-                    Ok(mode) => mode,
+            let stat = match stat_result {
+                Some(stat) => stat,
+                None => match do_stat() {
+                    Ok(stat) => stat,
                     Err(Errno::ESTALE) => {
                         self.report_stale_file_handle(Some(&full_path));
                         continue;
-- 
2.47.3

[PATCH v2 proxmox-backup 2/3] client: pxar: consistently ignore vanished files and warn about them

[Robert Obkircher]

Combine the error paths and treat deleted entries equally in both
cases. Since this used to be a hard error in some cases, it also makes
sense to emit a warning about it.

Use a slightly different message than the report_vanished_file helper
to make it clear that this error happened during the initial directory
listing.

Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
---
 pbs-client/src/pxar/create.rs | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs
index cab18e45b..2492eaf00 100644
--- a/pbs-client/src/pxar/create.rs
+++ b/pbs-client/src/pxar/create.rs
@@ -695,36 +695,28 @@ impl Archiver {
                     })
                 });
 
-            match match_result {
+            let stat_result = match match_result {
                 Ok(Some(MatchType::Exclude)) => {
                     debug!("matched by exclude pattern '{full_path:?}'");
                     continue;
                 }
-                Ok(_) => (),
-                Err(err) if err.not_found() => continue,
+                Ok(_) => stat_result.map_or_else(do_stat, Ok),
+                Err(e) => Err(e),
+            };
+
+            let stat = match stat_result {
+                Ok(stat) => stat,
+                Err(err) if err.not_found() => {
+                    warn!("warning: file vanished while reading directory: {full_path:?}");
+                    continue;
+                }
                 Err(Errno::ESTALE) => {
                     self.report_stale_file_handle(Some(&full_path));
                     continue;
                 }
                 Err(err) => {
-                    return Err(err).with_context(|| format!("stat failed on {full_path:?}"));
+                    return Err(err).context(format!("stat failed on {full_path:?}"));
                 }
-            }
-
-            let stat = match stat_result {
-                Some(stat) => stat,
-                None => match do_stat() {
-                    Ok(stat) => stat,
-                    Err(Errno::ESTALE) => {
-                        self.report_stale_file_handle(Some(&full_path));
-                        continue;
-                    }
-                    Err(err) => {
-                        return Err(
-                            Error::from(err).context(format!("stat failed on {full_path:?}"))
-                        );
-                    }
-                },
             };
 
             self.entry_counter += 1;
-- 
2.47.3

[PATCH v2 proxmox-backup 3/3] fix #7658: client: pxar: skip files on fstatat permission errors

[Robert Obkircher]

Log a warning instead of aborting the entire backup if fstatat returns
a permission error. This is preferable when running backups without
full knowledge or control over the contents [1, 2] and consistent with
how we treat EACCES when opening files.

Note that it was already possible to explicitly exclude such files[3].

[1] https://bugzilla.proxmox.com/show_bug.cgi?id=7658
[2] https://forum.proxmox.com/threads/182315
[3] https://bugzilla.proxmox.com/show_bug.cgi?id=4380

Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
---
 pbs-client/src/pxar/create.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs
index 2492eaf00..cb66b70b8 100644
--- a/pbs-client/src/pxar/create.rs
+++ b/pbs-client/src/pxar/create.rs
@@ -710,6 +710,11 @@ impl Archiver {
                     warn!("warning: file vanished while reading directory: {full_path:?}");
                     continue;
                 }
+                Err(Errno::EACCES) => {
+                    // may happen for fuse mountpoints without allow_other or allow_root
+                    warn!("warning: failed to stat file: {full_path:?}: access denied");
+                    continue;
+                }
                 Err(Errno::ESTALE) => {
                     self.report_stale_file_handle(Some(&full_path));
                     continue;
-- 
2.47.3

Re: [PATCH v2 proxmox-backup 0/3] improve fstatat error handling

[Christian Ebner]

On 6/3/26 6:19 PM, Robert Obkircher wrote:
> Changes since v1:
> * shortened Err(Error::from(err).context to Err(err).context
> * prefixed log message with "warning: "
> * added the bugzilla id and url
> 
> [v1] https://lore.proxmox.com/pbs-devel/20260506115016.171006-1-r.obkircher@proxmox.com
> 
> Robert Obkircher (3):
>    client: pxar: improve variable names
>    client: pxar: consistently ignore vanished files and warn about them
>    fix #7658: client: pxar: skip files on fstatat permission errors
> 
>   pbs-client/src/pxar/create.rs | 45 ++++++++++++++++-------------------
>   1 file changed, 21 insertions(+), 24 deletions(-)
> 

Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Christian Ebner <c.ebner@proxmox.com>

General side note: Please pick up R-b and/or T-b trailers for patches 
which did not change in-between versions and have been reviewed or 
tested, thanks!