public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [RFC proxmox-backup] backup/backup_info: ignore vanished backup dirs during listing
Date: Thu, 28 Jan 2021 08:34:30 +0100	[thread overview]
Message-ID: <1611818626.6jkzjorqup.astroid@nora.none> (raw)
In-Reply-To: <20210127134314.22530-1-d.csapak@proxmox.com>

when looking at the call sites of list_backup_files, this seems wrong - 
e.g. when listing a BackupGroup's backups, such a no-longer-existing 
snapshot is now returned but has no files. when creating a new 
BackupInfo from a BackupDir, or listing a BackupInfo's files, it returns 
Ok but contains no files.

the decision whether to ignore such an error must be made at the call 
sites of the call sites above, since in most cases I'd want to ignore 
the snapshot and not just the file listing..

list_snapshot_files already throws an error if it can't read the 
manifest, list_snapshots and list_groups seem to include it with no 
files?

On January 27, 2021 2:43 pm, Dominik Csapak wrote:
> this had the effect that that a snapshot listing errored out when called
> during a prune action
> 
> we now ignore such errors and simply return no files
> 
> also add some context for the error if one happens
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> maybe we also want to filter such snapshots out one level higher?
> so that we do not show 'empty' snapshots
> 
>  src/backup/backup_info.rs | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/src/backup/backup_info.rs b/src/backup/backup_info.rs
> index 5ff1a6f8..dba3fea5 100644
> --- a/src/backup/backup_info.rs
> +++ b/src/backup/backup_info.rs
> @@ -1,4 +1,7 @@
> -use crate::tools;
> +use crate::tools::{
> +    self,
> +    fs::FileIterOps,
> +};
>  
>  use anyhow::{bail, format_err, Error};
>  use std::os::unix::io::RawFd;
> @@ -339,11 +342,24 @@ impl BackupInfo {
>  fn list_backup_files<P: ?Sized + nix::NixPath>(dirfd: RawFd, path: &P) -> Result<Vec<String>, Error> {
>      let mut files = vec![];
>  
> -    tools::scandir(dirfd, path, &BACKUP_FILE_REGEX, |_, filename, file_type| {
> -        if file_type != nix::dir::Type::File { return Ok(()); }
> -        files.push(filename.to_owned());
> -        Ok(())
> -    })?;
> +    match tools::fs::read_subdir(dirfd, path) {
> +        Ok(iter) => {
> +            for entry in iter.filter_file_name_regex(&BACKUP_FILE_REGEX) {
> +                let entry = entry?;
> +                let file_type = entry.file_type().ok_or_else(|| format_err!("unable to detect file type"))?;
> +                if file_type == nix::dir::Type::File {
> +                    // ok because we filtered with BACKUP_FILE_REGEX
> +                    let filename = unsafe { entry.file_name_utf8_unchecked() };
> +                    files.push(filename.to_owned());
> +                }
> +            }
> +        },
> +        Err(nix::Error::Sys(errno)) if errno == nix::errno::Errno::ENOENT => {
> +            // ignore vanished backup dirs
> +            eprintln!("backup dir vanished during listing");
> +        },
> +        Err(err) => bail!("error listing backup files: {}", err),
> +    }
>  
>      Ok(files)
>  }
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 




  reply	other threads:[~2021-01-28  7:34 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-27 13:43 Dominik Csapak
2021-01-28  7:34 ` Fabian Grünbichler [this message]
2021-01-28 12:26   ` Dominik Csapak
2021-01-29  9:01     ` Fabian Grünbichler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1611818626.6jkzjorqup.astroid@nora.none \
    --to=f.gruenbichler@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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