[pbs-devel] [PATCH proxmox-backup 4/4] fix #5853: client: pxar: exclude stale files on metadata read
Christian Ebner
c.ebner at proxmox.com
Tue Nov 5 15:01:53 CET 2024
Skip and warn the user for files which returned a stale file handle
error while reading the metadata associated to that file.
Instead of returning with an error when getting the metadata, return
a boolean flag signaling if a stale file handle has been encountered.
Link to issue in bugtracker:
https://bugzilla.proxmox.com/show_bug.cgi?id=5853
Link to thread in community forum:
https://forum.proxmox.com/threads/156822/
Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
---
pbs-client/src/pxar/create.rs | 100 ++++++++++++++++++++++------------
1 file changed, 66 insertions(+), 34 deletions(-)
diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs
index 2a844922c..85be00db4 100644
--- a/pbs-client/src/pxar/create.rs
+++ b/pbs-client/src/pxar/create.rs
@@ -228,7 +228,7 @@ where
let mut fs_feature_flags = Flags::from_magic(fs_magic);
let stat = nix::sys::stat::fstat(source_dir.as_raw_fd())?;
- let metadata = get_metadata(
+ let (metadata, stale_fd) = get_metadata(
source_dir.as_raw_fd(),
&stat,
feature_flags & fs_feature_flags,
@@ -744,7 +744,7 @@ impl Archiver {
return Ok(());
}
- let metadata = get_metadata(
+ let (metadata, stale_fd) = get_metadata(
fd.as_raw_fd(),
stat,
self.flags(),
@@ -753,6 +753,11 @@ impl Archiver {
self.skip_e2big_xattr,
)?;
+ if stale_fd {
+ log::warn!("Stale filehandle encountered, skip {:?}", self.path);
+ return Ok(());
+ }
+
if self.previous_payload_index.is_none() {
return self
.add_entry_to_archive(encoder, &mut None, c_file_name, stat, fd, &metadata, None)
@@ -1301,7 +1306,14 @@ impl Archiver {
file_name: &Path,
metadata: &Metadata,
) -> Result<(), Error> {
- let dest = nix::fcntl::readlinkat(fd.as_raw_fd(), &b""[..])?;
+ let dest = match nix::fcntl::readlinkat(fd.as_raw_fd(), &b""[..]) {
+ Ok(dest) => dest,
+ Err(Errno::ESTALE) => {
+ log::warn!("Stale file handle encountered, skip {file_name:?}");
+ return Ok(());
+ }
+ Err(err) => return Err(err.into()),
+ };
encoder.add_symlink(metadata, file_name, dest).await?;
Ok(())
}
@@ -1397,9 +1409,10 @@ fn get_metadata(
fs_magic: i64,
fs_feature_flags: &mut Flags,
skip_e2big_xattr: bool,
-) -> Result<Metadata, Error> {
+) -> Result<(Metadata, bool), Error> {
// required for some of these
let proc_path = Path::new("/proc/self/fd/").join(fd.to_string());
+ let mut stale_fd = false;
let mut meta = Metadata {
stat: pxar::Stat {
@@ -1412,18 +1425,27 @@ fn get_metadata(
..Default::default()
};
- get_xattr_fcaps_acl(
+ if get_xattr_fcaps_acl(
&mut meta,
fd,
&proc_path,
flags,
fs_feature_flags,
skip_e2big_xattr,
- )?;
- get_chattr(&mut meta, fd)?;
+ )? {
+ stale_fd = true;
+ log::warn!("Stale filehandle, xattrs incomplete");
+ }
+ if get_chattr(&mut meta, fd)? {
+ stale_fd = true;
+ log::warn!("Stale filehandle, chattr incomplete");
+ }
get_fat_attr(&mut meta, fd, fs_magic)?;
- get_quota_project_id(&mut meta, fd, flags, fs_magic)?;
- Ok(meta)
+ if get_quota_project_id(&mut meta, fd, flags, fs_magic)? {
+ stale_fd = true;
+ log::warn!("Stale filehandle, quota project id incomplete");
+ }
+ Ok((meta, stale_fd))
}
fn get_fcaps(
@@ -1431,22 +1453,23 @@ fn get_fcaps(
fd: RawFd,
flags: Flags,
fs_feature_flags: &mut Flags,
-) -> Result<(), Error> {
+) -> Result<bool, Error> {
if !flags.contains(Flags::WITH_FCAPS) {
- return Ok(());
+ return Ok(false);
}
match xattr::fgetxattr(fd, xattr::XATTR_NAME_FCAPS) {
Ok(data) => {
meta.fcaps = Some(pxar::format::FCaps { data });
- Ok(())
+ Ok(false)
}
- Err(Errno::ENODATA) => Ok(()),
+ Err(Errno::ENODATA) => Ok(false),
Err(Errno::EOPNOTSUPP) => {
fs_feature_flags.remove(Flags::WITH_FCAPS);
- Ok(())
+ Ok(false)
}
- Err(Errno::EBADF) => Ok(()), // symlinks
+ Err(Errno::EBADF) => Ok(false), // symlinks
+ Err(Errno::ESTALE) => Ok(true),
Err(err) => Err(err).context("failed to read file capabilities"),
}
}
@@ -1458,32 +1481,35 @@ fn get_xattr_fcaps_acl(
flags: Flags,
fs_feature_flags: &mut Flags,
skip_e2big_xattr: bool,
-) -> Result<(), Error> {
+) -> Result<bool, Error> {
if !flags.contains(Flags::WITH_XATTRS) {
- return Ok(());
+ return Ok(false);
}
let xattrs = match xattr::flistxattr(fd) {
Ok(names) => names,
Err(Errno::EOPNOTSUPP) => {
fs_feature_flags.remove(Flags::WITH_XATTRS);
- return Ok(());
+ return Ok(false);
}
Err(Errno::E2BIG) => {
match skip_e2big_xattr {
- true => return Ok(()),
+ true => return Ok(false),
false => {
bail!("{} (try --skip-e2big-xattr)", Errno::E2BIG.to_string());
}
};
}
- Err(Errno::EBADF) => return Ok(()), // symlinks
+ Err(Errno::EBADF) => return Ok(false), // symlinks
+ Err(Errno::ESTALE) => return Ok(true),
Err(err) => return Err(err).context("failed to read xattrs"),
};
for attr in &xattrs {
if xattr::is_security_capability(attr) {
- get_fcaps(meta, fd, flags, fs_feature_flags)?;
+ if get_fcaps(meta, fd, flags, fs_feature_flags)? {
+ return Ok(true);
+ }
continue;
}
@@ -1505,35 +1531,37 @@ fn get_xattr_fcaps_acl(
Err(Errno::EBADF) => (), // symlinks, shouldn't be able to reach this either
Err(Errno::E2BIG) => {
match skip_e2big_xattr {
- true => return Ok(()),
+ true => return Ok(false),
false => {
bail!("{} (try --skip-e2big-xattr)", Errno::E2BIG.to_string());
}
};
}
+ Err(Errno::ESTALE) => return Ok(true), // symlinks
Err(err) => {
return Err(err).context(format!("error reading extended attribute {attr:?}"))
}
}
}
- Ok(())
+ Ok(false)
}
-fn get_chattr(metadata: &mut Metadata, fd: RawFd) -> Result<(), Error> {
+fn get_chattr(metadata: &mut Metadata, fd: RawFd) -> Result<bool, Error> {
let mut attr: libc::c_long = 0;
match unsafe { fs::read_attr_fd(fd, &mut attr) } {
Ok(_) => (),
+ Err(Errno::ESTALE) => return Ok(true),
Err(errno) if errno_is_unsupported(errno) => {
- return Ok(());
+ return Ok(false);
}
Err(err) => return Err(err).context("failed to read file attributes"),
}
metadata.stat.flags |= Flags::from_chattr(attr).bits();
- Ok(())
+ Ok(false)
}
fn get_fat_attr(metadata: &mut Metadata, fd: RawFd, fs_magic: i64) -> Result<(), Error> {
@@ -1564,30 +1592,34 @@ fn get_quota_project_id(
fd: RawFd,
flags: Flags,
magic: i64,
-) -> Result<(), Error> {
+) -> Result<bool, Error> {
if !(metadata.is_dir() || metadata.is_regular_file()) {
- return Ok(());
+ return Ok(false);
}
if !flags.contains(Flags::WITH_QUOTA_PROJID) {
- return Ok(());
+ return Ok(false);
}
use proxmox_sys::linux::magic::*;
match magic {
EXT4_SUPER_MAGIC | XFS_SUPER_MAGIC | FUSE_SUPER_MAGIC | ZFS_SUPER_MAGIC => (),
- _ => return Ok(()),
+ _ => return Ok(false),
}
let mut fsxattr = fs::FSXAttr::default();
let res = unsafe { fs::fs_ioc_fsgetxattr(fd, &mut fsxattr) };
+ if let Err(Errno::ESTALE) = res {
+ return Ok(true);
+ }
+
// On some FUSE filesystems it can happen that ioctl is not supported.
// For these cases projid is set to 0 while the error is ignored.
if let Err(errno) = res {
if errno_is_unsupported(errno) {
- return Ok(());
+ return Ok(false);
} else {
return Err(errno).context("error while reading quota project id");
}
@@ -1597,7 +1629,7 @@ fn get_quota_project_id(
if projid != 0 {
metadata.quota_project_id = Some(pxar::format::QuotaProjectId { projid });
}
- Ok(())
+ Ok(false)
}
fn get_acl(
@@ -1840,7 +1872,7 @@ mod tests {
let fs_magic = detect_fs_type(dir.as_raw_fd()).unwrap();
let stat = nix::sys::stat::fstat(dir.as_raw_fd()).unwrap();
let mut fs_feature_flags = Flags::from_magic(fs_magic);
- let metadata = get_metadata(
+ let (metadata, _) = get_metadata(
dir.as_raw_fd(),
&stat,
fs_feature_flags,
@@ -1937,7 +1969,7 @@ mod tests {
let stat = nix::sys::stat::fstat(source_dir.as_raw_fd()).unwrap();
let mut fs_feature_flags = Flags::from_magic(fs_magic);
- let metadata = get_metadata(
+ let (metadata, _) = get_metadata(
source_dir.as_raw_fd(),
&stat,
fs_feature_flags,
--
2.39.5
More information about the pbs-devel
mailing list