* [pbs-devel] [PATCH proxmox/proxmox-backup] fix #2915: stat when necessary
@ 2022-06-28 11:13 Dominik Csapak
2022-06-28 11:13 ` [pbs-devel] [PATCH proxmox 1/1] partially fix #2915: proxmox-sys: scandir: stat if file_type is unknown Dominik Csapak
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Dominik Csapak @ 2022-06-28 11:13 UTC (permalink / raw)
To: pbs-devel
this series implements a fallback to stat'ing files when the
readdir entry does not contain the filetype (since only some
filesystems support that)
i opted for adding the 'FileStat' to the callback in 'scandir'
(even if we don't use it yet) so that we'll not stat again
sometimes when using that. If we want, we can of course omit
that part, then the interface stays as it currently is
we could also write our own 'ReadDir' that stats on demand, but given
how few cases we have where we actually use it, i am not so sure
if the benefits justify the work
proxmox:
Dominik Csapak (1):
partially fix #2915: proxmox-sys: scandir: stat if file_type is
unknown
proxmox-sys/src/fs/read_dir.rs | 36 ++++++++++++++++++++++++++++++----
1 file changed, 32 insertions(+), 4 deletions(-)
proxmox-backup:
Dominik Csapak (2):
adapt to changed callback signature of 'scandir'
partially fix #2915: 'stat' in case ReadDirEntry does not contain type
pbs-datastore/src/backup_info.rs | 23 ++++++++-----
pbs-datastore/src/chunk_store.rs | 23 ++++++-------
pbs-datastore/src/hierarchy.rs | 56 ++++++++++++++++++++++++++++++++
src/tools/disks/zfs.rs | 2 +-
4 files changed, 81 insertions(+), 23 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pbs-devel] [PATCH proxmox 1/1] partially fix #2915: proxmox-sys: scandir: stat if file_type is unknown
2022-06-28 11:13 [pbs-devel] [PATCH proxmox/proxmox-backup] fix #2915: stat when necessary Dominik Csapak
@ 2022-06-28 11:13 ` Dominik Csapak
2022-06-28 11:45 ` Wolfgang Bumiller
2022-06-28 11:13 ` [pbs-devel] [PATCH proxmox-backup 1/2] adapt to changed callback signature of 'scandir' Dominik Csapak
2022-06-28 11:13 ` [pbs-devel] [PATCH proxmox-backup 2/2] partially fix #2915: 'stat' in case ReadDirEntry does not contain type Dominik Csapak
2 siblings, 1 reply; 9+ messages in thread
From: Dominik Csapak @ 2022-06-28 11:13 UTC (permalink / raw)
To: pbs-devel
when using readdir/getdents the file type might be 'DT_UNKNOWN'
(depending on the filesystem). Do a fstatat in that case to
get the file type. Since maybe the callback wants to do
a stat anyway, pass it there (if done)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
proxmox-sys/src/fs/read_dir.rs | 36 ++++++++++++++++++++++++++++++----
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/proxmox-sys/src/fs/read_dir.rs b/proxmox-sys/src/fs/read_dir.rs
index f687748..785a412 100644
--- a/proxmox-sys/src/fs/read_dir.rs
+++ b/proxmox-sys/src/fs/read_dir.rs
@@ -141,20 +141,48 @@ pub fn scandir<P, F>(
mut callback: F,
) -> Result<(), Error>
where
- F: FnMut(RawFd, &str, nix::dir::Type) -> Result<(), Error>,
+ F: FnMut(RawFd, &str, nix::dir::Type, Option<nix::sys::stat::FileStat>) -> Result<(), Error>,
P: ?Sized + nix::NixPath,
{
for entry in scan_subdir(dirfd, path, regex)? {
let entry = entry?;
- let file_type = match entry.file_type() {
- Some(file_type) => file_type,
- None => bail!("unable to detect file type"),
+ let (file_type, stat) = match entry.file_type() {
+ Some(file_type) => (file_type, None),
+ None => {
+ use nix::sys::stat::SFlag;
+ let stat = nix::sys::stat::fstatat(
+ entry.parent_fd(),
+ entry.file_name(),
+ nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW,
+ )?;
+ let type_flag = SFlag::from_bits_truncate(stat.st_mode);
+ let file_type = if type_flag.contains(SFlag::S_IFDIR) {
+ nix::dir::Type::Directory
+ } else if type_flag.contains(SFlag::S_IFIFO) {
+ nix::dir::Type::Fifo
+ } else if type_flag.contains(SFlag::S_IFCHR) {
+ nix::dir::Type::CharacterDevice
+ } else if type_flag.contains(SFlag::S_IFBLK) {
+ nix::dir::Type::BlockDevice
+ } else if type_flag.contains(SFlag::S_IFREG) {
+ nix::dir::Type::File
+ } else if type_flag.contains(SFlag::S_IFLNK) {
+ nix::dir::Type::Symlink
+ } else if type_flag.contains(SFlag::S_IFSOCK) {
+ nix::dir::Type::Socket
+ } else {
+ bail!("unable to detect file type")
+ };
+
+ (file_type, Some(stat))
+ }
};
callback(
entry.parent_fd(),
unsafe { entry.file_name_utf8_unchecked() },
file_type,
+ stat,
)?;
}
Ok(())
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 1/2] adapt to changed callback signature of 'scandir'
2022-06-28 11:13 [pbs-devel] [PATCH proxmox/proxmox-backup] fix #2915: stat when necessary Dominik Csapak
2022-06-28 11:13 ` [pbs-devel] [PATCH proxmox 1/1] partially fix #2915: proxmox-sys: scandir: stat if file_type is unknown Dominik Csapak
@ 2022-06-28 11:13 ` Dominik Csapak
2022-06-28 11:47 ` Fabian Grünbichler
2022-06-28 11:13 ` [pbs-devel] [PATCH proxmox-backup 2/2] partially fix #2915: 'stat' in case ReadDirEntry does not contain type Dominik Csapak
2 siblings, 1 reply; 9+ messages in thread
From: Dominik Csapak @ 2022-06-28 11:13 UTC (permalink / raw)
To: pbs-devel
we now get an Option<FileStat> too
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
pbs-datastore/src/backup_info.rs | 23 ++++++++++++++---------
src/tools/disks/zfs.rs | 2 +-
2 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index 10320a35..21fe4602 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -87,7 +87,7 @@ impl BackupGroup {
libc::AT_FDCWD,
&path,
&BACKUP_DATE_REGEX,
- |l2_fd, backup_time, file_type| {
+ |l2_fd, backup_time, file_type, _stat| {
if file_type != nix::dir::Type::Directory {
return Ok(());
}
@@ -127,7 +127,7 @@ impl BackupGroup {
libc::AT_FDCWD,
&path,
&BACKUP_DATE_REGEX,
- |l2_fd, backup_time, file_type| {
+ |l2_fd, backup_time, file_type, _stat| {
if file_type != nix::dir::Type::Directory {
return Ok(());
}
@@ -635,13 +635,18 @@ fn list_backup_files<P: ?Sized + nix::NixPath>(
) -> Result<Vec<String>, Error> {
let mut files = vec![];
- proxmox_sys::fs::scandir(dirfd, path, &BACKUP_FILE_REGEX, |_, filename, file_type| {
- if file_type != nix::dir::Type::File {
- return Ok(());
- }
- files.push(filename.to_owned());
- Ok(())
- })?;
+ proxmox_sys::fs::scandir(
+ dirfd,
+ path,
+ &BACKUP_FILE_REGEX,
+ |_, filename, file_type, _stat| {
+ if file_type != nix::dir::Type::File {
+ return Ok(());
+ }
+ files.push(filename.to_owned());
+ Ok(())
+ },
+ )?;
Ok(files)
}
diff --git a/src/tools/disks/zfs.rs b/src/tools/disks/zfs.rs
index 30a6cc0c..5f09c2cf 100644
--- a/src/tools/disks/zfs.rs
+++ b/src/tools/disks/zfs.rs
@@ -178,7 +178,7 @@ pub(crate) fn update_zfs_objset_map(pool: &str) -> Result<(), Error> {
libc::AT_FDCWD,
&path,
&OBJSET_REGEX,
- |_l2_fd, filename, _type| {
+ |_l2_fd, filename, _type, _stat| {
let (name, _) = parse_objset_stat(pool, filename)?;
map.insert(name, (pool.to_string(), filename.to_string()));
Ok(())
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/2] partially fix #2915: 'stat' in case ReadDirEntry does not contain type
2022-06-28 11:13 [pbs-devel] [PATCH proxmox/proxmox-backup] fix #2915: stat when necessary Dominik Csapak
2022-06-28 11:13 ` [pbs-devel] [PATCH proxmox 1/1] partially fix #2915: proxmox-sys: scandir: stat if file_type is unknown Dominik Csapak
2022-06-28 11:13 ` [pbs-devel] [PATCH proxmox-backup 1/2] adapt to changed callback signature of 'scandir' Dominik Csapak
@ 2022-06-28 11:13 ` Dominik Csapak
2022-06-28 11:47 ` Fabian Grünbichler
2022-06-28 11:49 ` Wolfgang Bumiller
2 siblings, 2 replies; 9+ messages in thread
From: Dominik Csapak @ 2022-06-28 11:13 UTC (permalink / raw)
To: pbs-devel
readdir/getdents may return 'DT_UNKNOWN' for the file type
(which corresponds to 'None' in nix::dir::Entry), so stat the file and
check the type
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
pbs-datastore/src/chunk_store.rs | 23 ++++++-------
pbs-datastore/src/hierarchy.rs | 56 ++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+), 13 deletions(-)
diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
index c9714c1e..2c281b0c 100644
--- a/pbs-datastore/src/chunk_store.rs
+++ b/pbs-datastore/src/chunk_store.rs
@@ -375,24 +375,21 @@ impl ChunkStore {
),
};
- let file_type = match entry.file_type() {
- Some(file_type) => file_type,
- None => bail!(
- "unsupported file system type on chunk store '{}'",
- self.name
- ),
- };
- if file_type != nix::dir::Type::File {
- continue;
- }
-
- chunk_count += 1;
-
let filename = entry.file_name();
let lock = self.mutex.lock();
if let Ok(stat) = fstatat(dirfd, filename, nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW) {
+ if match entry.file_type() {
+ Some(file_type) => file_type != nix::dir::Type::File,
+ None => stat.st_mode & libc::S_IFREG == 0,
+ } {
+ drop(lock);
+ continue;
+ }
+
+ chunk_count += 1;
+
if stat.st_atime < min_atime {
//let age = now - stat.st_atime;
//println!("UNLINK {} {:?}", age/(3600*24), filename);
diff --git a/pbs-datastore/src/hierarchy.rs b/pbs-datastore/src/hierarchy.rs
index d5007b07..a97a1d2a 100644
--- a/pbs-datastore/src/hierarchy.rs
+++ b/pbs-datastore/src/hierarchy.rs
@@ -9,6 +9,16 @@ use pbs_api_types::{BackupNamespace, BackupType, BACKUP_DATE_REGEX, BACKUP_ID_RE
use crate::backup_info::{BackupDir, BackupGroup};
use crate::DataStore;
+fn dir_entry_is_path(entry: &proxmox_sys::fs::ReadDirEntry) -> Result<bool, Error> {
+ let stat = nix::sys::stat::fstatat(
+ entry.parent_fd(),
+ entry.file_name(),
+ nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW,
+ )?;
+ let is_dir = stat.st_mode & libc::S_IFDIR > 0;
+ Ok(is_dir)
+}
+
/// A iterator for all BackupDir's (Snapshots) in a BackupGroup
pub struct ListSnapshots {
group: BackupGroup,
@@ -36,6 +46,17 @@ impl Iterator for ListSnapshots {
Ok(ref entry) => {
match entry.file_type() {
Some(nix::dir::Type::Directory) => entry, // OK
+ None => match dir_entry_is_path(&entry) {
+ Ok(true) => entry,
+ Ok(false) => continue,
+ Err(err) => {
+ log::info!(
+ "error listing snapshots for {}: {err}",
+ self.group.group()
+ );
+ continue;
+ }
+ },
_ => continue,
}
}
@@ -93,6 +114,17 @@ impl Iterator for ListGroups {
Ok(ref entry) => {
match entry.file_type() {
Some(nix::dir::Type::Directory) => entry, // OK
+ None => match dir_entry_is_path(&entry) {
+ Ok(true) => entry,
+ Ok(false) => continue,
+ Err(err) => {
+ log::info!(
+ "error listing groups for {}: {err}",
+ self.store.name()
+ );
+ continue;
+ }
+ },
_ => continue,
}
}
@@ -114,6 +146,17 @@ impl Iterator for ListGroups {
Ok(ref entry) => {
match entry.file_type() {
Some(nix::dir::Type::Directory) => entry, // OK
+ None => match dir_entry_is_path(&entry) {
+ Ok(true) => entry,
+ Ok(false) => continue,
+ Err(err) => {
+ log::info!(
+ "error listing groups for {}: {err}",
+ self.store.name()
+ );
+ continue;
+ }
+ },
_ => continue,
}
}
@@ -176,6 +219,19 @@ impl Iterator for ListNamespaces {
Ok(ref entry) => {
match entry.file_type() {
Some(nix::dir::Type::Directory) => entry, // OK
+ None => match dir_entry_is_path(&entry) {
+ Ok(true) => entry,
+ Ok(false) => continue,
+ Err(err) => {
+ let mut base_path = self.base_path.to_owned();
+ if !self.ns.is_root() {
+ base_path.push(self.ns.path());
+ }
+ base_path.push("ns");
+ log::info!("error listing dirs in {:?}: {err}", base_path);
+ continue;
+ }
+ },
_ => continue,
}
}
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pbs-devel] [PATCH proxmox 1/1] partially fix #2915: proxmox-sys: scandir: stat if file_type is unknown
2022-06-28 11:13 ` [pbs-devel] [PATCH proxmox 1/1] partially fix #2915: proxmox-sys: scandir: stat if file_type is unknown Dominik Csapak
@ 2022-06-28 11:45 ` Wolfgang Bumiller
0 siblings, 0 replies; 9+ messages in thread
From: Wolfgang Bumiller @ 2022-06-28 11:45 UTC (permalink / raw)
To: Dominik Csapak; +Cc: pbs-devel
On Tue, Jun 28, 2022 at 01:13:16PM +0200, Dominik Csapak wrote:
> when using readdir/getdents the file type might be 'DT_UNKNOWN'
> (depending on the filesystem). Do a fstatat in that case to
> get the file type. Since maybe the callback wants to do
> a stat anyway, pass it there (if done)
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> proxmox-sys/src/fs/read_dir.rs | 36 ++++++++++++++++++++++++++++++----
> 1 file changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/proxmox-sys/src/fs/read_dir.rs b/proxmox-sys/src/fs/read_dir.rs
> index f687748..785a412 100644
> --- a/proxmox-sys/src/fs/read_dir.rs
> +++ b/proxmox-sys/src/fs/read_dir.rs
> @@ -141,20 +141,48 @@ pub fn scandir<P, F>(
> mut callback: F,
> ) -> Result<(), Error>
> where
> - F: FnMut(RawFd, &str, nix::dir::Type) -> Result<(), Error>,
> + F: FnMut(RawFd, &str, nix::dir::Type, Option<nix::sys::stat::FileStat>) -> Result<(), Error>,
> P: ?Sized + nix::NixPath,
> {
> for entry in scan_subdir(dirfd, path, regex)? {
> let entry = entry?;
> - let file_type = match entry.file_type() {
> - Some(file_type) => file_type,
> - None => bail!("unable to detect file type"),
> + let (file_type, stat) = match entry.file_type() {
> + Some(file_type) => (file_type, None),
> + None => {
> + use nix::sys::stat::SFlag;
> + let stat = nix::sys::stat::fstatat(
> + entry.parent_fd(),
> + entry.file_name(),
> + nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW,
> + )?;
> + let type_flag = SFlag::from_bits_truncate(stat.st_mode);
> + let file_type = if type_flag.contains(SFlag::S_IFDIR) {
Using `.contains()` here is wrong. The file type is not actually a
bitfield, but a number *within* a bitfield.
You mask out the type via `st_mode & S_IFMT` and can then use a `match`
statement to map it to `dir::Type`. (Which also makes for shorter code
;-) )
> + nix::dir::Type::Directory
> + } else if type_flag.contains(SFlag::S_IFIFO) {
> + nix::dir::Type::Fifo
> + } else if type_flag.contains(SFlag::S_IFCHR) {
> + nix::dir::Type::CharacterDevice
> + } else if type_flag.contains(SFlag::S_IFBLK) {
> + nix::dir::Type::BlockDevice
> + } else if type_flag.contains(SFlag::S_IFREG) {
> + nix::dir::Type::File
> + } else if type_flag.contains(SFlag::S_IFLNK) {
> + nix::dir::Type::Symlink
> + } else if type_flag.contains(SFlag::S_IFSOCK) {
> + nix::dir::Type::Socket
> + } else {
> + bail!("unable to detect file type")
> + };
> +
> + (file_type, Some(stat))
> + }
> };
>
> callback(
> entry.parent_fd(),
> unsafe { entry.file_name_utf8_unchecked() },
> file_type,
> + stat,
> )?;
> }
> Ok(())
> --
> 2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/2] partially fix #2915: 'stat' in case ReadDirEntry does not contain type
2022-06-28 11:13 ` [pbs-devel] [PATCH proxmox-backup 2/2] partially fix #2915: 'stat' in case ReadDirEntry does not contain type Dominik Csapak
@ 2022-06-28 11:47 ` Fabian Grünbichler
2022-06-28 11:49 ` Wolfgang Bumiller
1 sibling, 0 replies; 9+ messages in thread
From: Fabian Grünbichler @ 2022-06-28 11:47 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
On June 28, 2022 1:13 pm, Dominik Csapak wrote:
> readdir/getdents may return 'DT_UNKNOWN' for the file type
> (which corresponds to 'None' in nix::dir::Entry), so stat the file and
> check the type
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> pbs-datastore/src/chunk_store.rs | 23 ++++++-------
> pbs-datastore/src/hierarchy.rs | 56 ++++++++++++++++++++++++++++++++
> 2 files changed, 66 insertions(+), 13 deletions(-)
>
> diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
> index c9714c1e..2c281b0c 100644
> --- a/pbs-datastore/src/chunk_store.rs
> +++ b/pbs-datastore/src/chunk_store.rs
> @@ -375,24 +375,21 @@ impl ChunkStore {
> ),
> };
>
> - let file_type = match entry.file_type() {
> - Some(file_type) => file_type,
> - None => bail!(
> - "unsupported file system type on chunk store '{}'",
> - self.name
> - ),
> - };
> - if file_type != nix::dir::Type::File {
> - continue;
> - }
> -
> - chunk_count += 1;
> -
> let filename = entry.file_name();
>
> let lock = self.mutex.lock();
>
> if let Ok(stat) = fstatat(dirfd, filename, nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW) {
> + if match entry.file_type() {
> + Some(file_type) => file_type != nix::dir::Type::File,
> + None => stat.st_mode & libc::S_IFREG == 0,
nit: since we need the stat here anyway, this could just drop the
entry.file_type() altogether?
> + } {
> + drop(lock);
> + continue;
> + }
> +
> + chunk_count += 1;
> +
> if stat.st_atime < min_atime {
> //let age = now - stat.st_atime;
> //println!("UNLINK {} {:?}", age/(3600*24), filename);
> diff --git a/pbs-datastore/src/hierarchy.rs b/pbs-datastore/src/hierarchy.rs
> index d5007b07..a97a1d2a 100644
> --- a/pbs-datastore/src/hierarchy.rs
> +++ b/pbs-datastore/src/hierarchy.rs
> @@ -9,6 +9,16 @@ use pbs_api_types::{BackupNamespace, BackupType, BACKUP_DATE_REGEX, BACKUP_ID_RE
> use crate::backup_info::{BackupDir, BackupGroup};
> use crate::DataStore;
>
> +fn dir_entry_is_path(entry: &proxmox_sys::fs::ReadDirEntry) -> Result<bool, Error> {
> + let stat = nix::sys::stat::fstatat(
> + entry.parent_fd(),
> + entry.file_name(),
> + nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW,
> + )?;
> + let is_dir = stat.st_mode & libc::S_IFDIR > 0;
> + Ok(is_dir)
> +}
nit: why *is_path*? everything's a path ;) this should probably be called
`dir_entry_is_dir` or `dir_entry_is_subdir`?
we could also re-use the matching from proxmox_sys -> have a helper that
takes an entry, returns the file_type if possible, stats otherwise (the
parent fd and name are in the entry after all), returns the stat result
transposed to Option<FileType> (or change the call sites to expect
Result<FileType>), if we want to support this across the board a single
place for the stat implementation is probably better than multiple
ones..
> +
> /// A iterator for all BackupDir's (Snapshots) in a BackupGroup
> pub struct ListSnapshots {
> group: BackupGroup,
> @@ -36,6 +46,17 @@ impl Iterator for ListSnapshots {
> Ok(ref entry) => {
> match entry.file_type() {
> Some(nix::dir::Type::Directory) => entry, // OK
> + None => match dir_entry_is_path(&entry) {
> + Ok(true) => entry,
> + Ok(false) => continue,
> + Err(err) => {
> + log::info!(
> + "error listing snapshots for {}: {err}",
> + self.group.group()
> + );
> + continue;
> + }
> + },
> _ => continue,
> }
> }
> @@ -93,6 +114,17 @@ impl Iterator for ListGroups {
> Ok(ref entry) => {
> match entry.file_type() {
> Some(nix::dir::Type::Directory) => entry, // OK
> + None => match dir_entry_is_path(&entry) {
> + Ok(true) => entry,
> + Ok(false) => continue,
> + Err(err) => {
> + log::info!(
> + "error listing groups for {}: {err}",
> + self.store.name()
> + );
> + continue;
> + }
> + },
> _ => continue,
> }
> }
> @@ -114,6 +146,17 @@ impl Iterator for ListGroups {
> Ok(ref entry) => {
> match entry.file_type() {
> Some(nix::dir::Type::Directory) => entry, // OK
> + None => match dir_entry_is_path(&entry) {
> + Ok(true) => entry,
> + Ok(false) => continue,
> + Err(err) => {
> + log::info!(
> + "error listing groups for {}: {err}",
> + self.store.name()
> + );
> + continue;
> + }
> + },
> _ => continue,
> }
> }
> @@ -176,6 +219,19 @@ impl Iterator for ListNamespaces {
> Ok(ref entry) => {
> match entry.file_type() {
> Some(nix::dir::Type::Directory) => entry, // OK
> + None => match dir_entry_is_path(&entry) {
> + Ok(true) => entry,
> + Ok(false) => continue,
> + Err(err) => {
> + let mut base_path = self.base_path.to_owned();
> + if !self.ns.is_root() {
> + base_path.push(self.ns.path());
> + }
> + base_path.push("ns");
> + log::info!("error listing dirs in {:?}: {err}", base_path);
> + continue;
> + }
> + },
> _ => continue,
> }
> }
> --
> 2.30.2
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 1/2] adapt to changed callback signature of 'scandir'
2022-06-28 11:13 ` [pbs-devel] [PATCH proxmox-backup 1/2] adapt to changed callback signature of 'scandir' Dominik Csapak
@ 2022-06-28 11:47 ` Fabian Grünbichler
0 siblings, 0 replies; 9+ messages in thread
From: Fabian Grünbichler @ 2022-06-28 11:47 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
On June 28, 2022 1:13 pm, Dominik Csapak wrote:
> we now get an Option<FileStat> too
but don't use it *anywhere* - so why introduce a breaking change in
proxmox_sys for it? is there some other place where we use it?
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> pbs-datastore/src/backup_info.rs | 23 ++++++++++++++---------
> src/tools/disks/zfs.rs | 2 +-
> 2 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
> index 10320a35..21fe4602 100644
> --- a/pbs-datastore/src/backup_info.rs
> +++ b/pbs-datastore/src/backup_info.rs
> @@ -87,7 +87,7 @@ impl BackupGroup {
> libc::AT_FDCWD,
> &path,
> &BACKUP_DATE_REGEX,
> - |l2_fd, backup_time, file_type| {
> + |l2_fd, backup_time, file_type, _stat| {
> if file_type != nix::dir::Type::Directory {
> return Ok(());
> }
> @@ -127,7 +127,7 @@ impl BackupGroup {
> libc::AT_FDCWD,
> &path,
> &BACKUP_DATE_REGEX,
> - |l2_fd, backup_time, file_type| {
> + |l2_fd, backup_time, file_type, _stat| {
> if file_type != nix::dir::Type::Directory {
> return Ok(());
> }
> @@ -635,13 +635,18 @@ fn list_backup_files<P: ?Sized + nix::NixPath>(
> ) -> Result<Vec<String>, Error> {
> let mut files = vec![];
>
> - proxmox_sys::fs::scandir(dirfd, path, &BACKUP_FILE_REGEX, |_, filename, file_type| {
> - if file_type != nix::dir::Type::File {
> - return Ok(());
> - }
> - files.push(filename.to_owned());
> - Ok(())
> - })?;
> + proxmox_sys::fs::scandir(
> + dirfd,
> + path,
> + &BACKUP_FILE_REGEX,
> + |_, filename, file_type, _stat| {
> + if file_type != nix::dir::Type::File {
> + return Ok(());
> + }
> + files.push(filename.to_owned());
> + Ok(())
> + },
> + )?;
>
> Ok(files)
> }
> diff --git a/src/tools/disks/zfs.rs b/src/tools/disks/zfs.rs
> index 30a6cc0c..5f09c2cf 100644
> --- a/src/tools/disks/zfs.rs
> +++ b/src/tools/disks/zfs.rs
> @@ -178,7 +178,7 @@ pub(crate) fn update_zfs_objset_map(pool: &str) -> Result<(), Error> {
> libc::AT_FDCWD,
> &path,
> &OBJSET_REGEX,
> - |_l2_fd, filename, _type| {
> + |_l2_fd, filename, _type, _stat| {
> let (name, _) = parse_objset_stat(pool, filename)?;
> map.insert(name, (pool.to_string(), filename.to_string()));
> Ok(())
> --
> 2.30.2
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/2] partially fix #2915: 'stat' in case ReadDirEntry does not contain type
2022-06-28 11:13 ` [pbs-devel] [PATCH proxmox-backup 2/2] partially fix #2915: 'stat' in case ReadDirEntry does not contain type Dominik Csapak
2022-06-28 11:47 ` Fabian Grünbichler
@ 2022-06-28 11:49 ` Wolfgang Bumiller
2022-06-28 11:52 ` Wolfgang Bumiller
1 sibling, 1 reply; 9+ messages in thread
From: Wolfgang Bumiller @ 2022-06-28 11:49 UTC (permalink / raw)
To: Dominik Csapak; +Cc: pbs-devel
On Tue, Jun 28, 2022 at 01:13:18PM +0200, Dominik Csapak wrote:
> readdir/getdents may return 'DT_UNKNOWN' for the file type
> (which corresponds to 'None' in nix::dir::Entry), so stat the file and
> check the type
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> pbs-datastore/src/chunk_store.rs | 23 ++++++-------
> pbs-datastore/src/hierarchy.rs | 56 ++++++++++++++++++++++++++++++++
> 2 files changed, 66 insertions(+), 13 deletions(-)
>
> diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
> index c9714c1e..2c281b0c 100644
> --- a/pbs-datastore/src/chunk_store.rs
> +++ b/pbs-datastore/src/chunk_store.rs
> @@ -375,24 +375,21 @@ impl ChunkStore {
> ),
> };
>
> - let file_type = match entry.file_type() {
> - Some(file_type) => file_type,
> - None => bail!(
> - "unsupported file system type on chunk store '{}'",
> - self.name
> - ),
> - };
> - if file_type != nix::dir::Type::File {
> - continue;
> - }
> -
> - chunk_count += 1;
> -
> let filename = entry.file_name();
>
> let lock = self.mutex.lock();
>
> if let Ok(stat) = fstatat(dirfd, filename, nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW) {
> + if match entry.file_type() {
> + Some(file_type) => file_type != nix::dir::Type::File,
> + None => stat.st_mode & libc::S_IFREG == 0,
Should be `(stat.st_mode & libc::S_IFMT) != libc::S_IFREG`.
> + } {
> + drop(lock);
> + continue;
> + }
> +
> + chunk_count += 1;
> +
> if stat.st_atime < min_atime {
> //let age = now - stat.st_atime;
> //println!("UNLINK {} {:?}", age/(3600*24), filename);
> diff --git a/pbs-datastore/src/hierarchy.rs b/pbs-datastore/src/hierarchy.rs
> index d5007b07..a97a1d2a 100644
> --- a/pbs-datastore/src/hierarchy.rs
> +++ b/pbs-datastore/src/hierarchy.rs
> @@ -9,6 +9,16 @@ use pbs_api_types::{BackupNamespace, BackupType, BACKUP_DATE_REGEX, BACKUP_ID_RE
> use crate::backup_info::{BackupDir, BackupGroup};
> use crate::DataStore;
>
> +fn dir_entry_is_path(entry: &proxmox_sys::fs::ReadDirEntry) -> Result<bool, Error> {
> + let stat = nix::sys::stat::fstatat(
> + entry.parent_fd(),
> + entry.file_name(),
> + nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW,
> + )?;
> + let is_dir = stat.st_mode & libc::S_IFDIR > 0;
Should be `(stat.st_mode & libc::S_IFMT) != libc::S_IFDIR`.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/2] partially fix #2915: 'stat' in case ReadDirEntry does not contain type
2022-06-28 11:49 ` Wolfgang Bumiller
@ 2022-06-28 11:52 ` Wolfgang Bumiller
0 siblings, 0 replies; 9+ messages in thread
From: Wolfgang Bumiller @ 2022-06-28 11:52 UTC (permalink / raw)
To: Dominik Csapak; +Cc: pbs-devel
On Tue, Jun 28, 2022 at 01:49:23PM +0200, Wolfgang Bumiller wrote:
> On Tue, Jun 28, 2022 at 01:13:18PM +0200, Dominik Csapak wrote:
> > diff --git a/pbs-datastore/src/hierarchy.rs b/pbs-datastore/src/hierarchy.rs
> > index d5007b07..a97a1d2a 100644
> > --- a/pbs-datastore/src/hierarchy.rs
> > +++ b/pbs-datastore/src/hierarchy.rs
> > @@ -9,6 +9,16 @@ use pbs_api_types::{BackupNamespace, BackupType, BACKUP_DATE_REGEX, BACKUP_ID_RE
> > use crate::backup_info::{BackupDir, BackupGroup};
> > use crate::DataStore;
> >
> > +fn dir_entry_is_path(entry: &proxmox_sys::fs::ReadDirEntry) -> Result<bool, Error> {
> > + let stat = nix::sys::stat::fstatat(
> > + entry.parent_fd(),
> > + entry.file_name(),
> > + nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW,
> > + )?;
> > + let is_dir = stat.st_mode & libc::S_IFDIR > 0;
>
> Should be `(stat.st_mode & libc::S_IFMT) != libc::S_IFDIR`.
I meant `==` here ;-)
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-06-28 11:53 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-28 11:13 [pbs-devel] [PATCH proxmox/proxmox-backup] fix #2915: stat when necessary Dominik Csapak
2022-06-28 11:13 ` [pbs-devel] [PATCH proxmox 1/1] partially fix #2915: proxmox-sys: scandir: stat if file_type is unknown Dominik Csapak
2022-06-28 11:45 ` Wolfgang Bumiller
2022-06-28 11:13 ` [pbs-devel] [PATCH proxmox-backup 1/2] adapt to changed callback signature of 'scandir' Dominik Csapak
2022-06-28 11:47 ` Fabian Grünbichler
2022-06-28 11:13 ` [pbs-devel] [PATCH proxmox-backup 2/2] partially fix #2915: 'stat' in case ReadDirEntry does not contain type Dominik Csapak
2022-06-28 11:47 ` Fabian Grünbichler
2022-06-28 11:49 ` Wolfgang Bumiller
2022-06-28 11:52 ` Wolfgang Bumiller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox