Re: [pbs-devel] [PATCH proxmox-backup] gc: attach context to index reader errors and ignore NotFound

[Dietmar Maurer <dietmar@proxmox.com>]

I would prefer to return std::io::Error instead:

impl FixedIndexReader {
-    pub fn open(path: &Path) -> Result<Self, Error> {
+    pub fn open(path: &Path) -> Result<Self, std::io::Error> {

Wolfgang wrote some macros for that;

proxmox::io_bail!() and proxmox::io_format_err!()


> On 09/08/2020 11:18 AM Stefan Reiter <s.reiter@proxmox.com> wrote:
> 
>  
> Ignore NotFound errors during phase 1, this just means that a snapshot
> was forgotten or pruned between scanning for .fidx/.didx files and
> actually opening the index to touch the chunks.
> 
> ignore_notfound has to be a real function, since generics are not
> supported for closures.
> 
> The open methods for dynamic and fixed indices are switched from the
> usual format_err! to err.context() to allow checking for the root error
> (and thus the io::ErrorKind) further up the call chain.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
>  src/backup/datastore.rs     | 28 ++++++++++++++++++++++++----
>  src/backup/dynamic_index.rs |  5 ++++-
>  src/backup/fixed_index.rs   |  7 +++++--
>  3 files changed, 33 insertions(+), 7 deletions(-)
> 
> diff --git a/src/backup/datastore.rs b/src/backup/datastore.rs
> index ebe47487..1f455d64 100644
> --- a/src/backup/datastore.rs
> +++ b/src/backup/datastore.rs
> @@ -426,6 +426,20 @@ impl DataStore {
>          Ok(())
>      }
>  
> +    fn ignore_notfound<T>(res: Result<T, Error>) -> Result<Option<T>, Error> {
> +        match res {
> +            Ok(t) => Ok(Some(t)),
> +            Err(err) => {
> +                if let Some(ioerr) = err.downcast_ref::<std::io::Error>() {
> +                    if ioerr.kind() == std::io::ErrorKind::NotFound {
> +                        return Ok(None);
> +                    }
> +                }
> +                Err(err)
> +            }
> +        }
> +    }
> +
>      fn mark_used_chunks(&self, status: &mut GarbageCollectionStatus, worker: &WorkerTask) -> Result<(), Error> {
>  
>          let image_list = self.list_images()?;
> @@ -443,11 +457,17 @@ impl DataStore {
>  
>              if let Ok(archive_type) = archive_type(&path) {
>                  if archive_type == ArchiveType::FixedIndex {
> -                    let index = self.open_fixed_reader(&path)?;
> -                    self.index_mark_used_chunks(index, &path, status, worker)?;
> +                    if let Some(index) = Self::ignore_notfound(self.open_fixed_reader(&path))? {
> +                        self.index_mark_used_chunks(index, &path, status, worker)?;
> +                    } else {
> +                        worker.warn(format!("warning: could no longer find fixed index '{:?}'", &path));
> +                    }
>                  } else if archive_type == ArchiveType::DynamicIndex {
> -                    let index = self.open_dynamic_reader(&path)?;
> -                    self.index_mark_used_chunks(index, &path, status, worker)?;
> +                    if let Some(index) = Self::ignore_notfound(self.open_dynamic_reader(&path))? {
> +                        self.index_mark_used_chunks(index, &path, status, worker)?;
> +                    } else {
> +                        worker.warn(format!("warning: could no longer find dynamic index '{:?}'", &path));
> +                    }
>                  }
>              }
>              done += 1;
> diff --git a/src/backup/dynamic_index.rs b/src/backup/dynamic_index.rs
> index f70aa44f..a7ce0f24 100644
> --- a/src/backup/dynamic_index.rs
> +++ b/src/backup/dynamic_index.rs
> @@ -86,7 +86,10 @@ impl DynamicIndexReader {
>          File::open(path)
>              .map_err(Error::from)
>              .and_then(Self::new)
> -            .map_err(|err| format_err!("Unable to open dynamic index {:?} - {}", path, err))
> +            .map_err(|err| {
> +                let msg = format!("Unable to open dynamic index {:?} - {}", path, err);
> +                err.context(msg)
> +            })
>      }
>  
>      pub fn new(mut file: std::fs::File) -> Result<Self, Error> {
> diff --git a/src/backup/fixed_index.rs b/src/backup/fixed_index.rs
> index 5d6cc1ff..bf864173 100644
> --- a/src/backup/fixed_index.rs
> +++ b/src/backup/fixed_index.rs
> @@ -1,4 +1,4 @@
> -use anyhow::{bail, format_err, Error};
> +use anyhow::{bail, Error};
>  use std::io::{Seek, SeekFrom};
>  
>  use super::chunk_stat::*;
> @@ -62,7 +62,10 @@ impl FixedIndexReader {
>          File::open(path)
>              .map_err(Error::from)
>              .and_then(|file| Self::new(file))
> -            .map_err(|err| format_err!("Unable to open fixed index {:?} - {}", path, err))
> +            .map_err(|err| {
> +                let msg = format!("Unable to open fixed index {:?} - {}", path, err);
> +                err.context(msg)
> +            })
>      }
>  
>      pub fn new(mut file: std::fs::File) -> Result<Self, Error> {
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel