Re: [pbs-devel] [PATCH v2 proxmox-backup 4/5] proxmox-backup-client: restore: add 'ignore-extract-device-errors' flag

[Max Carrara <m.carrara@proxmox.com>]

On 7/13/23 09:52, Christoph Heiss wrote:
> 
> On Wed, Jul 12, 2023 at 03:00:43PM +0200, Max Carrara wrote:
>> If this flag is provided, any errors that occur during the extraction
>> of a device node are silently ignored.
>>
>> Signed-off-by: Max Carrara <m.carrara@proxmox.com>
>> ---
>>  Changes v1 --> v2:
>>   * Remove unnecessary "future-proofing" that merges many error handlers into one
>>
>>  proxmox-backup-client/src/main.rs | 32 ++++++++++++++++++++++++++++++-
>>  1 file changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
>> index 1a6114b1..c7f90109 100644
>> --- a/proxmox-backup-client/src/main.rs
>> +++ b/proxmox-backup-client/src/main.rs
>>
>> [..]
>>
>> @@ -1245,6 +1252,7 @@ async fn restore(
>>      ignore_ownership: bool,
>>      ignore_permissions: bool,
>>      overwrite: bool,
>> +    ignore_extract_device_errors: bool,
>>  ) -> Result<Value, Error> {
>>      let repo = extract_repository_from_value(&param)?;
>>
>> @@ -1365,12 +1373,34 @@ async fn restore(
>>
>>          let mut reader = BufferedDynamicReader::new(index, chunk_reader);
>>
>> +        let on_error = if ignore_extract_device_errors {
>> +            let handler: PxarErrorHandler = Box::new(move |err: Error| {
>> +                use pbs_client::pxar::PxarExtractContext;
>> +
>> +                let ctx = err.downcast_ref::<PxarExtractContext>();
>> +
>> +                if ctx.is_none() {
>> +                    return Err(err);
>> +                }
>> +
>> +                if matches!(ctx.unwrap(), PxarExtractContext::ExtractDevice) {
>> +                    return Ok(());
>> +                }
>> +
>> +                Err(err)
>> +            });
>> +
>> +            Some(handler)
> Couldn't this be a lot simpler written as:
> 
> Some(Box::new(move |err: Error| {
>     use pbs_client::pxar::PxarExtractContext;
> 
>     match err.downcast_ref::<PxarExtractContext>() {
> 	Some(PxarExtractContext::ExtractDevice) => Ok(()),
> 	_ => Err(err),
>     }
> }))
> 
> (untested; just off the top of my head)
> 

Oh, true! I missed that one. I'll see what else turns up before
sending in another series though.

>> +        } else {
>> +            None
>> +        };
>> +
>>          let options = pbs_client::pxar::PxarExtractOptions {
>>              match_list: &[],
>>              extract_match_default: true,
>>              allow_existing_dirs,
>>              overwrite,
>> -            on_error: None,
>> +            on_error,
>>          };
>>
>>          let mut feature_flags = pbs_client::pxar::Flags::DEFAULT;
>> --
>> 2.39.2
>>
>>
>>
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>
>>