public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] applied: [PATCH proxmox-backup 1/2] api2/tape/restore: restore_chunk_archive: only ignore tape related errors
@ 2021-04-14  8:42 Dietmar Maurer
  0 siblings, 0 replies; only message in thread
From: Dietmar Maurer @ 2021-04-14  8:42 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dominik Csapak

I don't think this fixes any bugs, but the code is cleaner and more obvious, so
I applied it anyways...

> On 04/13/2021 12:58 PM Dominik Csapak <d.csapak@proxmox.com> wrote:
> 
>  
> when we get an error from the tape, we possibly want to ignore it,
> i.e. when the file was incomplete, but we still want to error
> out if the error came from e.g, the datastore, so we have to move
> the error checking code to the 'next_chunk' call
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/api2/tape/restore.rs | 77 ++++++++++++++++++++--------------------
>  1 file changed, 38 insertions(+), 39 deletions(-)
> 
> diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
> index afce3449..9f79f06f 100644
> --- a/src/api2/tape/restore.rs
> +++ b/src/api2/tape/restore.rs
> @@ -597,54 +597,53 @@ fn restore_chunk_archive<'a>(
>  
>      let mut decoder = ChunkArchiveDecoder::new(reader);
>  
> -    let result: Result<_, Error> = proxmox::try_block!({
> -        while let Some((digest, blob)) = decoder.next_chunk()? {
> -
> -            worker.check_abort()?;
> -
> -            if let Some(datastore) = datastore {
> -                let chunk_exists = datastore.cond_touch_chunk(&digest, false)?;
> -                if !chunk_exists {
> -                    blob.verify_crc()?;
> +    loop {
> +        let (digest, blob) = match decoder.next_chunk() {
> +            Ok(Some((digest, blob))) => (digest, blob),
> +            Ok(None) => break,
> +            Err(err) => {
> +                let reader = decoder.reader();
> +
> +                // check if this stream is marked incomplete
> +                if let Ok(true) = reader.is_incomplete() {
> +                    return Ok(Some(chunks));
> +                }
>  
> -                    if blob.crypt_mode()? == CryptMode::None {
> -                        blob.decode(None, Some(&digest))?; // verify digest
> -                    }
> -                    if verbose {
> -                        task_log!(worker, "Insert chunk: {}", proxmox::tools::digest_to_hex(&digest));
> -                    }
> -                    datastore.insert_chunk(&blob, &digest)?;
> -                } else if verbose {
> -                    task_log!(worker, "Found existing chunk: {}", proxmox::tools::digest_to_hex(&digest));
> +                // check if this is an aborted stream without end marker
> +                if let Ok(false) = reader.has_end_marker() {
> +                    worker.log("missing stream end marker".to_string());
> +                    return Ok(None);
>                  }
> -            } else if verbose {
> -                task_log!(worker, "Found chunk: {}", proxmox::tools::digest_to_hex(&digest));
> +
> +                // else the archive is corrupt
> +                return Err(err);
>              }
> -            chunks.push(digest);
> -        }
> -        Ok(())
> -    });
> +        };
>  
> -    match result {
> -        Ok(()) => Ok(Some(chunks)),
> -        Err(err) => {
> -            let reader = decoder.reader();
> +        worker.check_abort()?;
>  
> -            // check if this stream is marked incomplete
> -            if let Ok(true) = reader.is_incomplete() {
> -                return Ok(Some(chunks));
> -            }
> +        if let Some(datastore) = datastore {
> +            let chunk_exists = datastore.cond_touch_chunk(&digest, false)?;
> +            if !chunk_exists {
> +                blob.verify_crc()?;
>  
> -            // check if this is an aborted stream without end marker
> -            if let Ok(false) = reader.has_end_marker() {
> -                worker.log("missing stream end marker".to_string());
> -                return Ok(None);
> +                if blob.crypt_mode()? == CryptMode::None {
> +                    blob.decode(None, Some(&digest))?; // verify digest
> +                }
> +                if verbose {
> +                    task_log!(worker, "Insert chunk: {}", proxmox::tools::digest_to_hex(&digest));
> +                }
> +                datastore.insert_chunk(&blob, &digest)?;
> +            } else if verbose {
> +                task_log!(worker, "Found existing chunk: {}", proxmox::tools::digest_to_hex(&digest));
>              }
> -
> -            // else the archive is corrupt
> -            Err(err)
> +        } else if verbose {
> +            task_log!(worker, "Found chunk: {}", proxmox::tools::digest_to_hex(&digest));
>          }
> +        chunks.push(digest);
>      }
> +
> +    Ok(Some(chunks))
>  }
>  
>  fn restore_snapshot_archive<'a>(
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel




^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-04-14  8:42 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-14  8:42 [pbs-devel] applied: [PATCH proxmox-backup 1/2] api2/tape/restore: restore_chunk_archive: only ignore tape related errors Dietmar Maurer

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