[pbs-devel] [PATCH v2 proxmox-backup 0/2] Introduce bitflags for overwrite

[pbs-devel] [PATCH v2 proxmox-backup 0/2] Introduce bitflags for overwrite

[Christian Ebner]

When restoring a pxar archive to a target containing preexisting data,
the `overwrite` flag allows to force recreation of file payloads.
However, the current implementation does not allow for symlinks or
hardlinks to be recreated.

These patches introduce the logic to overwrite symlinks and hardlinks
for archive entries, if a directory entry with matching name is already
present on the filesystem. An existing entry is unlinked and the
symlink/hardlink created.

In order to allow a more fine grained control over the overwrite
behaviour, the current overwrite flag is refactored to set all bits
of a newly introduced bitflag. Further, optional parameters for
individual bits controlling the overwriting of files, symlinks and
hardlinks are created.

Christian Ebner (2):
  fix: #4761: unlink existing entries for hard/symlinks when overwrite
  fix: #4761: introduce overwrite bitflags for fine grained overwrites

 pbs-client/src/catalog_shell.rs   |  9 +++-
 pbs-client/src/pxar/extract.rs    | 82 ++++++++++++++++++++++++++-----
 pbs-client/src/pxar/mod.rs        |  2 +-
 proxmox-backup-client/src/main.rs | 28 ++++++++++-
 pxar-bin/src/main.rs              | 32 +++++++++++-
 5 files changed, 135 insertions(+), 18 deletions(-)

-- 
2.39.2

[pbs-devel] [PATCH v2 proxmox-backup 1/2] fix: #4761: unlink existing entries for hard/symlinks when overwrite

[Christian Ebner]

Creating symlinks or hardlinks might fail if a directory entry with the
same name is already present on the filesystem during restore.

When the overwrite flag is given, on failure unlink the existing entry
(except directories) and retry hard/symlink creation.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since v1:
* rebased to current master

 pbs-client/src/pxar/extract.rs | 39 +++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
index 4dbaf52d..c1e7b417 100644
--- a/pbs-client/src/pxar/extract.rs
+++ b/pbs-client/src/pxar/extract.rs
@@ -547,7 +547,21 @@ impl Extractor {
         link: &OsStr,
     ) -> Result<(), Error> {
         let parent = self.parent_fd()?;
-        nix::unistd::symlinkat(link, Some(parent), file_name)?;
+
+        match nix::unistd::symlinkat(link, Some(parent), file_name) {
+            Ok(()) => {}
+            Err(nix::errno::Errno::EEXIST) => {
+                if !self.overwrite {
+                    return Err(nix::errno::Errno::EEXIST.into());
+                }
+                // Never unlink directories
+                let flag = nix::unistd::UnlinkatFlags::NoRemoveDir;
+                nix::unistd::unlinkat(Some(parent), file_name, flag)?;
+                nix::unistd::symlinkat(link, Some(parent), file_name)?;
+            }
+            Err(err) => return Err(err.into())
+        }
+
         metadata::apply_at(
             self.feature_flags,
             metadata,
@@ -564,13 +578,32 @@ impl Extractor {
         let parent = self.parent_fd()?;
         let root = self.dir_stack.root_dir_fd()?;
         let target = CString::new(link.as_bytes())?;
-        nix::unistd::linkat(
+
+        match nix::unistd::linkat(
             Some(root.as_raw_fd()),
             target.as_c_str(),
             Some(parent),
             file_name,
             nix::unistd::LinkatFlags::NoSymlinkFollow,
-        )?;
+        ) {
+            Ok(()) => {}
+            Err(nix::errno::Errno::EEXIST) => {
+                if !self.overwrite {
+                    return Err(nix::errno::Errno::EEXIST.into());
+                }
+                // Never unlink directories
+                let flag = nix::unistd::UnlinkatFlags::NoRemoveDir;
+                nix::unistd::unlinkat(Some(parent), file_name, flag)?;
+                nix::unistd::linkat(
+                    Some(root.as_raw_fd()),
+                    target.as_c_str(),
+                    Some(parent),
+                    file_name,
+                    nix::unistd::LinkatFlags::NoSymlinkFollow,
+                )?;
+            }
+            Err(err) => return Err(err.into())
+        }
 
         Ok(())
     }
-- 
2.39.2

[pbs-devel] [PATCH v2 proxmox-backup 2/2] fix: #4761: introduce overwrite bitflags for fine grained overwrites

[Christian Ebner]

Adds OverwriteFlags for granular control of which entry types should
overwrite entries present on the filesystem during a restore.

The original overwrite flag is refactored in order to cover all of the
other cases.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since v1:
* rebased and refactored to current master

 pbs-client/src/catalog_shell.rs   |  9 ++++--
 pbs-client/src/pxar/extract.rs    | 47 +++++++++++++++++++++++--------
 pbs-client/src/pxar/mod.rs        |  2 +-
 proxmox-backup-client/src/main.rs | 28 +++++++++++++++++-
 pxar-bin/src/main.rs              | 32 +++++++++++++++++++--
 5 files changed, 101 insertions(+), 17 deletions(-)

diff --git a/pbs-client/src/catalog_shell.rs b/pbs-client/src/catalog_shell.rs
index 98af5699..b0be50f1 100644
--- a/pbs-client/src/catalog_shell.rs
+++ b/pbs-client/src/catalog_shell.rs
@@ -987,8 +987,13 @@ impl Shell {
             .metadata()
             .clone();
 
-        let extractor =
-            crate::pxar::extract::Extractor::new(rootdir, root_meta, true, false, Flags::DEFAULT);
+        let extractor = crate::pxar::extract::Extractor::new(
+            rootdir,
+            root_meta,
+            true,
+            crate::pxar::extract::OverwriteFlags::NONE,
+            Flags::DEFAULT
+        );
 
         let mut extractor = ExtractorState::new(
             &mut self.catalog,
diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
index c1e7b417..6faf0b54 100644
--- a/pbs-client/src/pxar/extract.rs
+++ b/pbs-client/src/pxar/extract.rs
@@ -9,6 +9,7 @@ use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex};
 
 use anyhow::{bail, format_err, Context, Error};
+use bitflags::bitflags;
 use nix::dir::Dir;
 use nix::fcntl::OFlag;
 use nix::sys::stat::Mode;
@@ -33,10 +34,34 @@ pub struct PxarExtractOptions<'a> {
     pub match_list: &'a [MatchEntry],
     pub extract_match_default: bool,
     pub allow_existing_dirs: bool,
-    pub overwrite: bool,
+    pub overwrite_flags: OverwriteFlags,
     pub on_error: Option<ErrorHandler>,
 }
 
+
+bitflags! {
+    pub struct OverwriteFlags: u8 {
+        /// Disable all
+        const NONE = 0x0;
+        /// Overwrite existing entries file content 
+        const FILE = 0x1;
+        /// Overwrite existing entry with symlink
+        const SYMLINK = 0x2;
+        /// Overwrite existing entry with hardlink
+        const HARDLINK = 0x4;
+        /// Enable all
+        const ALL = OverwriteFlags::FILE.bits()
+            | OverwriteFlags::SYMLINK.bits()
+            | OverwriteFlags::HARDLINK.bits();
+    }
+}
+
+impl Default for OverwriteFlags {
+    fn default() -> Self {
+        OverwriteFlags::NONE
+    }
+}
+
 pub type ErrorHandler = Box<dyn FnMut(Error) -> Result<(), Error> + Send>;
 
 pub fn extract_archive<T, F>(
@@ -141,7 +166,7 @@ where
             dir,
             root.metadata().clone(),
             options.allow_existing_dirs,
-            options.overwrite,
+            options.overwrite_flags,
             feature_flags,
         );
 
@@ -345,7 +370,7 @@ where
                         metadata,
                         *size,
                         &mut contents,
-                        self.extractor.overwrite,
+                        self.extractor.overwrite_flags.contains(OverwriteFlags::FILE),
                     )
                 } else {
                     Err(format_err!(
@@ -438,7 +463,7 @@ impl std::fmt::Display for PxarExtractContext {
 pub struct Extractor {
     feature_flags: Flags,
     allow_existing_dirs: bool,
-    overwrite: bool,
+    overwrite_flags: OverwriteFlags,
     dir_stack: PxarDirStack,
 
     /// For better error output we need to track the current path in the Extractor state.
@@ -455,13 +480,13 @@ impl Extractor {
         root_dir: Dir,
         metadata: Metadata,
         allow_existing_dirs: bool,
-        overwrite: bool,
+        overwrite_flags: OverwriteFlags,
         feature_flags: Flags,
     ) -> Self {
         Self {
             dir_stack: PxarDirStack::new(root_dir, metadata),
             allow_existing_dirs,
-            overwrite,
+            overwrite_flags,
             feature_flags,
             current_path: Arc::new(Mutex::new(OsString::new())),
             on_error: Box::new(Err),
@@ -551,7 +576,7 @@ impl Extractor {
         match nix::unistd::symlinkat(link, Some(parent), file_name) {
             Ok(()) => {}
             Err(nix::errno::Errno::EEXIST) => {
-                if !self.overwrite {
+                if !self.overwrite_flags.contains(OverwriteFlags::SYMLINK) {
                     return Err(nix::errno::Errno::EEXIST.into());
                 }
                 // Never unlink directories
@@ -588,7 +613,7 @@ impl Extractor {
         ) {
             Ok(()) => {}
             Err(nix::errno::Errno::EEXIST) => {
-                if !self.overwrite {
+                if !self.overwrite_flags.contains(OverwriteFlags::HARDLINK) {
                     return Err(nix::errno::Errno::EEXIST.into());
                 }
                 // Never unlink directories
@@ -1065,7 +1090,7 @@ where
     )
     .with_context(|| format!("unable to open target directory {:?}", destination.as_ref()))?;
 
-    Ok(Extractor::new(dir, metadata, false, false, Flags::DEFAULT))
+    Ok(Extractor::new(dir, metadata, false, OverwriteFlags::NONE, Flags::DEFAULT))
 }
 
 pub async fn extract_sub_dir<T, DEST, PATH>(
@@ -1199,7 +1224,7 @@ where
                         .contents()
                         .await
                         .context("found regular file entry without contents in archive")?,
-                    extractor.overwrite,
+                    extractor.overwrite_flags.contains(OverwriteFlags::FILE),
                 )
                 .await?
         }
@@ -1247,7 +1272,7 @@ where
                             &mut decoder
                                 .contents()
                                 .context("found regular file entry without contents in archive")?,
-                            extractor.overwrite,
+                            extractor.overwrite_flags.contains(OverwriteFlags::FILE),
                         )
                         .await?
                 }
diff --git a/pbs-client/src/pxar/mod.rs b/pbs-client/src/pxar/mod.rs
index b042717d..03610700 100644
--- a/pbs-client/src/pxar/mod.rs
+++ b/pbs-client/src/pxar/mod.rs
@@ -59,7 +59,7 @@ pub use flags::Flags;
 pub use create::{create_archive, PxarCreateOptions};
 pub use extract::{
     create_tar, create_zip, extract_archive, extract_sub_dir, extract_sub_dir_seq, ErrorHandler,
-    PxarExtractContext, PxarExtractOptions,
+    PxarExtractContext, PxarExtractOptions, OverwriteFlags,
 };
 
 /// The format requires to build sorted directory lookup tables in
diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index d9e7b899..db94b65d 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -1234,6 +1234,21 @@ We do not extract '.pxar' archives when writing to standard output.
                 optional: true,
                 default: false,
             },
+            "overwrite-files": {
+                description: "overwrite already existing files",
+                optional: true,
+                default: false,
+            },
+            "overwrite-symlinks": {
+                description: "overwrite already existing entries by archives symlink",
+                optional: true,
+                default: false,
+            },
+            "overwrite-hardlinks": {
+                description: "overwrite already existing entries by archives hardlink",
+                optional: true,
+                default: false,
+            },
             "ignore-extract-device-errors": {
                 type: Boolean,
                 description: "ignore errors that occur during device node extraction",
@@ -1252,6 +1267,9 @@ async fn restore(
     ignore_ownership: bool,
     ignore_permissions: bool,
     overwrite: bool,
+    overwrite_files: bool,
+    overwrite_symlinks: bool,
+    overwrite_hardlinks: bool,
     ignore_extract_device_errors: bool,
 ) -> Result<Value, Error> {
     let repo = extract_repository_from_value(&param)?;
@@ -1388,11 +1406,19 @@ async fn restore(
             None
         };
 
+        let mut overwrite_flags = pbs_client::pxar::OverwriteFlags::NONE;
+        overwrite_flags.set(pbs_client::pxar::OverwriteFlags::FILE, overwrite_files);
+        overwrite_flags.set(pbs_client::pxar::OverwriteFlags::SYMLINK, overwrite_symlinks);
+        overwrite_flags.set(pbs_client::pxar::OverwriteFlags::HARDLINK, overwrite_hardlinks);
+        if overwrite {
+            overwrite_flags.insert(pbs_client::pxar::OverwriteFlags::ALL);
+        }
+
         let options = pbs_client::pxar::PxarExtractOptions {
             match_list: &[],
             extract_match_default: true,
             allow_existing_dirs,
-            overwrite,
+            overwrite_flags,
             on_error,
         };
 
diff --git a/pxar-bin/src/main.rs b/pxar-bin/src/main.rs
index 90887321..fcfa1435 100644
--- a/pxar-bin/src/main.rs
+++ b/pxar-bin/src/main.rs
@@ -12,7 +12,9 @@ use futures::select;
 use tokio::signal::unix::{signal, SignalKind};
 
 use pathpatterns::{MatchEntry, MatchType, PatternFlag};
-use pbs_client::pxar::{format_single_line_entry, Flags, PxarExtractOptions, ENCODER_MAX_ENTRIES};
+use pbs_client::pxar::{
+    format_single_line_entry, Flags, PxarExtractOptions, OverwriteFlags, ENCODER_MAX_ENTRIES,
+};
 
 use proxmox_router::cli::*;
 use proxmox_schema::api;
@@ -74,10 +76,25 @@ fn extract_archive_from_reader<R: std::io::Read>(
                 default: false,
             },
             "overwrite": {
+                description: "overwrite already existing files, symlinks and hardlinks",
+                optional: true,
+                default: false,
+            },
+            "overwrite-files": {
                 description: "overwrite already existing files",
                 optional: true,
                 default: false,
             },
+            "overwrite-symlinks": {
+                description: "overwrite already existing entries by archives symlink",
+                optional: true,
+                default: false,
+            },
+            "overwrite-hardlinks": {
+                description: "overwrite already existing entries by archives hardlink",
+                optional: true,
+                default: false,
+            },
             "files-from": {
                 description: "File containing match pattern for files to restore.",
                 optional: true,
@@ -116,6 +133,9 @@ fn extract_archive(
     no_acls: bool,
     allow_existing_dirs: bool,
     overwrite: bool,
+    overwrite_files: bool,
+    overwrite_symlinks: bool,
+    overwrite_hardlinks: bool,
     files_from: Option<String>,
     no_device_nodes: bool,
     no_fifos: bool,
@@ -141,6 +161,14 @@ fn extract_archive(
     if no_sockets {
         feature_flags.remove(Flags::WITH_SOCKETS);
     }
+    
+    let mut overwrite_flags = OverwriteFlags::NONE;
+    overwrite_flags.set(OverwriteFlags::FILE, overwrite_files);
+    overwrite_flags.set(OverwriteFlags::SYMLINK, overwrite_symlinks);
+    overwrite_flags.set(OverwriteFlags::HARDLINK, overwrite_hardlinks);
+    if overwrite {
+        overwrite_flags.insert(OverwriteFlags::ALL);
+    }
 
     let pattern = pattern.unwrap_or_default();
     let target = target.as_ref().map_or_else(|| ".", String::as_str);
@@ -183,7 +211,7 @@ fn extract_archive(
     let options = PxarExtractOptions {
         match_list: &match_list,
         allow_existing_dirs,
-        overwrite,
+        overwrite_flags,
         extract_match_default,
         on_error,
     };
-- 
2.39.2

Re: [pbs-devel] [PATCH v2 proxmox-backup 1/2] fix: #4761: unlink existing entries for hard/symlinks when overwrite

[Wolfgang Bumiller]

On Tue, Aug 01, 2023 at 12:34:11PM +0200, Christian Ebner wrote:
> Creating symlinks or hardlinks might fail if a directory entry with the
> same name is already present on the filesystem during restore.
> 
> When the overwrite flag is given, on failure unlink the existing entry
> (except directories) and retry hard/symlink creation.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes since v1:
> * rebased to current master
> 
>  pbs-client/src/pxar/extract.rs | 39 +++++++++++++++++++++++++++++++---
>  1 file changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
> index 4dbaf52d..c1e7b417 100644
> --- a/pbs-client/src/pxar/extract.rs
> +++ b/pbs-client/src/pxar/extract.rs
> @@ -547,7 +547,21 @@ impl Extractor {
>          link: &OsStr,
>      ) -> Result<(), Error> {
>          let parent = self.parent_fd()?;
> -        nix::unistd::symlinkat(link, Some(parent), file_name)?;
> +
> +        match nix::unistd::symlinkat(link, Some(parent), file_name) {
> +            Ok(()) => {}
> +            Err(nix::errno::Errno::EEXIST) => {
> +                if !self.overwrite {
> +                    return Err(nix::errno::Errno::EEXIST.into());
> +                }

The above could be shortened to a pattern gaurd:

               Err(nix::errno::Errno::EEXIST) if self.overwrite => {

then we don't need the extra `return Err()` since we just run into the
final `Err(err)` case.

> +                // Never unlink directories
> +                let flag = nix::unistd::UnlinkatFlags::NoRemoveDir;
> +                nix::unistd::unlinkat(Some(parent), file_name, flag)?;
> +                nix::unistd::symlinkat(link, Some(parent), file_name)?;
> +            }
> +            Err(err) => return Err(err.into())
> +        }
> +
>          metadata::apply_at(
>              self.feature_flags,
>              metadata,
> @@ -564,13 +578,32 @@ impl Extractor {
>          let parent = self.parent_fd()?;
>          let root = self.dir_stack.root_dir_fd()?;
>          let target = CString::new(link.as_bytes())?;
> -        nix::unistd::linkat(
> +
> +        match nix::unistd::linkat(
>              Some(root.as_raw_fd()),
>              target.as_c_str(),
>              Some(parent),
>              file_name,
>              nix::unistd::LinkatFlags::NoSymlinkFollow,
> -        )?;

We can avoid some duplication by moving the `linkat` call into a
closure:
        let dolink = || {
            nix::unistd::linkat(
                Some(root.as_raw_fd()),
                target.as_c_str(),
                Some(parent),
                file_name,
                nix::unistd::LinkatFlags::NoSymlinkFollow,
            )
        };
        match dolink() {
            ...
            nix::unistd::unlinkat(Some(parent), file_name, flag)?;
            dolink()?;
        }
        ...

> +        ) {
> +            Ok(()) => {}
> +            Err(nix::errno::Errno::EEXIST) => {
> +                if !self.overwrite {
> +                    return Err(nix::errno::Errno::EEXIST.into());
> +                }
> +                // Never unlink directories
> +                let flag = nix::unistd::UnlinkatFlags::NoRemoveDir;
> +                nix::unistd::unlinkat(Some(parent), file_name, flag)?;
> +                nix::unistd::linkat(
> +                    Some(root.as_raw_fd()),
> +                    target.as_c_str(),
> +                    Some(parent),
> +                    file_name,
> +                    nix::unistd::LinkatFlags::NoSymlinkFollow,
> +                )?;
> +            }
> +            Err(err) => return Err(err.into())
> +        }
>  
>          Ok(())
>      }
> -- 
> 2.39.2

Re: [pbs-devel] [PATCH v2 proxmox-backup 2/2] fix: #4761: introduce overwrite bitflags for fine grained overwrites

[Wolfgang Bumiller]

This has some trailing whitespace and needs a `rustfmt`.

Also, see inline comments:

On Tue, Aug 01, 2023 at 12:34:12PM +0200, Christian Ebner wrote:
> Adds OverwriteFlags for granular control of which entry types should
> overwrite entries present on the filesystem during a restore.
> 
> The original overwrite flag is refactored in order to cover all of the
> other cases.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes since v1:
> * rebased and refactored to current master
> 
>  pbs-client/src/catalog_shell.rs   |  9 ++++--
>  pbs-client/src/pxar/extract.rs    | 47 +++++++++++++++++++++++--------
>  pbs-client/src/pxar/mod.rs        |  2 +-
>  proxmox-backup-client/src/main.rs | 28 +++++++++++++++++-
>  pxar-bin/src/main.rs              | 32 +++++++++++++++++++--
>  5 files changed, 101 insertions(+), 17 deletions(-)
> 
(...)
> diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
> index c1e7b417..6faf0b54 100644
> --- a/pbs-client/src/pxar/extract.rs
> +++ b/pbs-client/src/pxar/extract.rs
> @@ -9,6 +9,7 @@ use std::path::{Path, PathBuf};
>  use std::sync::{Arc, Mutex};
>  
>  use anyhow::{bail, format_err, Context, Error};
> +use bitflags::bitflags;
>  use nix::dir::Dir;
>  use nix::fcntl::OFlag;
>  use nix::sys::stat::Mode;
> @@ -33,10 +34,34 @@ pub struct PxarExtractOptions<'a> {
>      pub match_list: &'a [MatchEntry],
>      pub extract_match_default: bool,
>      pub allow_existing_dirs: bool,
> -    pub overwrite: bool,
> +    pub overwrite_flags: OverwriteFlags,
>      pub on_error: Option<ErrorHandler>,
>  }
>  
> +
> +bitflags! {
> +    pub struct OverwriteFlags: u8 {
> +        /// Disable all
> +        const NONE = 0x0;

bitflags provides a `OverwriteFlags::empty()`, so I'd drop the `NONE`
variant

> +        /// Overwrite existing entries file content 
> +        const FILE = 0x1;
> +        /// Overwrite existing entry with symlink
> +        const SYMLINK = 0x2;
> +        /// Overwrite existing entry with hardlink
> +        const HARDLINK = 0x4;
> +        /// Enable all
> +        const ALL = OverwriteFlags::FILE.bits()
> +            | OverwriteFlags::SYMLINK.bits()
> +            | OverwriteFlags::HARDLINK.bits();

and bitflags also provides a `OverwriteFlags::all()`, so I'd drop `ALL`
as well ;-)

> +    }
> +}
> +
> +impl Default for OverwriteFlags {
> +    fn default() -> Self {
> +        OverwriteFlags::NONE

since this is `0`, you can just `derive(Default)` instead

> +    }
> +}
> +
>  pub type ErrorHandler = Box<dyn FnMut(Error) -> Result<(), Error> + Send>;
>  
>  pub fn extract_archive<T, F>(
> @@ -141,7 +166,7 @@ where
(...)

Otherwise LGTM

Re: [pbs-devel] [PATCH v2 proxmox-backup 1/2] fix: #4761: unlink existing entries for hard/symlinks when overwrite

[Wolfgang Bumiller]

On Fri, Aug 04, 2023 at 02:20:40PM +0200, Wolfgang Bumiller wrote:
> On Tue, Aug 01, 2023 at 12:34:11PM +0200, Christian Ebner wrote:
> > Creating symlinks or hardlinks might fail if a directory entry with the
> > same name is already present on the filesystem during restore.
> > 
> > When the overwrite flag is given, on failure unlink the existing entry
> > (except directories) and retry hard/symlink creation.
> > 
> > Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> > ---
> > changes since v1:
> > * rebased to current master
> > 
> >  pbs-client/src/pxar/extract.rs | 39 +++++++++++++++++++++++++++++++---
> >  1 file changed, 36 insertions(+), 3 deletions(-)
> > 
> > diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
> > index 4dbaf52d..c1e7b417 100644
> > --- a/pbs-client/src/pxar/extract.rs
> > +++ b/pbs-client/src/pxar/extract.rs
> > @@ -547,7 +547,21 @@ impl Extractor {
> >          link: &OsStr,
> >      ) -> Result<(), Error> {
> >          let parent = self.parent_fd()?;
> > -        nix::unistd::symlinkat(link, Some(parent), file_name)?;
> > +
> > +        match nix::unistd::symlinkat(link, Some(parent), file_name) {
> > +            Ok(()) => {}
> > +            Err(nix::errno::Errno::EEXIST) => {
> > +                if !self.overwrite {
> > +                    return Err(nix::errno::Errno::EEXIST.into());
> > +                }
> 
> The above could be shortened to a pattern gaurd:
> 
>                Err(nix::errno::Errno::EEXIST) if self.overwrite => {
> 
> then we don't need the extra `return Err()` since we just run into the
> final `Err(err)` case.

As a side note, you could have used
    Err(err @ nix::errno::Errno::EEXIST)
and then returned `Err(err.into())` ;-)