[pbs-devel] [PATCH proxmox-backup 0/3] pbs-client feature #3923

[pbs-devel] [PATCH proxmox-backup 0/3] pbs-client feature #3923

[Markus Frank]

These patches are for ignore-acl, ignore-xattr, ignore-ownership &
overwrite-existing-files parameter in proxmox-backup-client.

Markus Frank (3):
  overwrite-existing-files for pxar
  skip xattr/acl/ownership options
  added ignore-acl/xattr/ownership & overwrite parameter to
    proxmox-backup-client

 pbs-client/src/catalog_shell.rs   |  4 +--
 pbs-client/src/pxar/extract.rs    | 24 ++++++++++++++--
 pbs-client/src/pxar/flags.rs      |  1 +
 pbs-client/src/pxar/metadata.rs   | 48 ++++++++++++++++++++-----------
 proxmox-backup-client/src/main.rs | 40 +++++++++++++++++++++++++-
 pxar-bin/src/main.rs              |  7 +++++
 6 files changed, 101 insertions(+), 23 deletions(-)

-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 1/3] added overwrite-existing-files as

[Markus Frank]

If true, O_EXCL is not set and therefore overwrites the files and does not
error out.

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 pbs-client/src/catalog_shell.rs |  4 ++--
 pbs-client/src/pxar/extract.rs  | 24 +++++++++++++++++++++---
 pxar-bin/src/main.rs            |  7 +++++++
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/pbs-client/src/catalog_shell.rs b/pbs-client/src/catalog_shell.rs
index b11901ed..25e27b37 100644
--- a/pbs-client/src/catalog_shell.rs
+++ b/pbs-client/src/catalog_shell.rs
@@ -984,7 +984,7 @@ impl Shell {
             .clone();

         let extractor =
-            crate::pxar::extract::Extractor::new(rootdir, root_meta, true, Flags::DEFAULT);
+            crate::pxar::extract::Extractor::new(rootdir, root_meta, true, false, Flags::DEFAULT);

         let mut extractor = ExtractorState::new(
             &mut self.catalog,
@@ -1172,7 +1172,7 @@ impl<'a> ExtractorState<'a> {
                 let file_name = CString::new(entry.file_name().as_bytes())?;
                 let mut contents = entry.contents().await?;
                 self.extractor
-                    .async_extract_file(&file_name, entry.metadata(), *size, &mut contents)
+                    .async_extract_file(&file_name, entry.metadata(), *size, &mut contents, false)
                     .await
             }
             _ => {
diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
index 161d2cef..3b9151aa 100644
--- a/pbs-client/src/pxar/extract.rs
+++ b/pbs-client/src/pxar/extract.rs
@@ -34,6 +34,7 @@ pub struct PxarExtractOptions<'a> {
     pub match_list: &'a [MatchEntry],
     pub extract_match_default: bool,
     pub allow_existing_dirs: bool,
+    pub overwrite_existing_files: bool,
     pub on_error: Option<ErrorHandler>,
 }

@@ -80,6 +81,7 @@ where
         dir,
         root.metadata().clone(),
         options.allow_existing_dirs,
+        options.overwrite_existing_files,
         feature_flags,
     );

@@ -198,6 +200,7 @@ where
                 &mut decoder.contents().ok_or_else(|| {
                     format_err!("found regular file entry without contents in archive")
                 })?,
+                extractor.overwrite_existing_files,
             ),
             (false, _) => Ok(()), // skip this
         }
@@ -215,6 +218,7 @@ where
 pub struct Extractor {
     feature_flags: Flags,
     allow_existing_dirs: bool,
+    overwrite_existing_files: bool,
     dir_stack: PxarDirStack,

     /// For better error output we need to track the current path in the Extractor state.
@@ -231,11 +235,13 @@ impl Extractor {
         root_dir: Dir,
         metadata: Metadata,
         allow_existing_dirs: bool,
+        overwrite_existing_files: bool,
         feature_flags: Flags,
     ) -> Self {
         Self {
             dir_stack: PxarDirStack::new(root_dir, metadata),
             allow_existing_dirs,
+            overwrite_existing_files,
             feature_flags,
             current_path: Arc::new(Mutex::new(OsString::new())),
             on_error: Box::new(Err),
@@ -392,14 +398,19 @@ impl Extractor {
         metadata: &Metadata,
         size: u64,
         contents: &mut dyn io::Read,
+        overwrite_existing_files: bool,
     ) -> Result<(), Error> {
         let parent = self.parent_fd()?;
+        let mut oflags = OFlag::O_CREAT | OFlag::O_WRONLY | OFlag::O_CLOEXEC;
+        if !overwrite_existing_files {
+            oflags = oflags | OFlag::O_EXCL;
+        }
         let mut file = unsafe {
             std::fs::File::from_raw_fd(
                 nix::fcntl::openat(
                     parent,
                     file_name,
-                    OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_WRONLY | OFlag::O_CLOEXEC,
+                    oflags,
                     Mode::from_bits(0o600).unwrap(),
                 )
                 .map_err(|err| format_err!("failed to create file {:?}: {}", file_name, err))?,
@@ -448,14 +459,19 @@ impl Extractor {
         metadata: &Metadata,
         size: u64,
         contents: &mut T,
+        overwrite_existing_files: bool,
     ) -> Result<(), Error> {
         let parent = self.parent_fd()?;
+        let mut oflags = OFlag::O_CREAT | OFlag::O_WRONLY | OFlag::O_CLOEXEC;
+        if !overwrite_existing_files {
+            oflags = oflags | OFlag::O_EXCL;
+        }
         let mut file = tokio::fs::File::from_std(unsafe {
             std::fs::File::from_raw_fd(
                 nix::fcntl::openat(
                     parent,
                     file_name,
-                    OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_WRONLY | OFlag::O_CLOEXEC,
+                    oflags,
                     Mode::from_bits(0o600).unwrap(),
                 )
                 .map_err(|err| format_err!("failed to create file {:?}: {}", file_name, err))?,
@@ -818,7 +834,7 @@ where
         )
     })?;

-    Ok(Extractor::new(dir, metadata, false, Flags::DEFAULT))
+    Ok(Extractor::new(dir, metadata, false, false, Flags::DEFAULT))
 }

 pub async fn extract_sub_dir<T, DEST, PATH>(
@@ -951,6 +967,7 @@ where
                     &mut file.contents().await.map_err(|_| {
                         format_err!("found regular file entry without contents in archive")
                     })?,
+                    extractor.overwrite_existing_files,
                 )
                 .await?
         }
@@ -998,6 +1015,7 @@ where
                             &mut decoder.contents().ok_or_else(|| {
                                 format_err!("found regular file entry without contents in archive")
                             })?,
+                            extractor.overwrite_existing_files,
                         )
                         .await?
                 }
diff --git a/pxar-bin/src/main.rs b/pxar-bin/src/main.rs
index 3714eb03..4b49df51 100644
--- a/pxar-bin/src/main.rs
+++ b/pxar-bin/src/main.rs
@@ -75,6 +75,11 @@ fn extract_archive_from_reader<R: std::io::Read>(
                 optional: true,
                 default: false,
             },
+            "overwrite_existing_files": {
+                description: "overwrite existing files",
+                optional: true,
+                default: false,
+            },
             "files-from": {
                 description: "File containing match pattern for files to restore.",
                 optional: true,
@@ -112,6 +117,7 @@ fn extract_archive(
     no_fcaps: bool,
     no_acls: bool,
     allow_existing_dirs: bool,
+    overwrite_existing_files: bool,
     files_from: Option<String>,
     no_device_nodes: bool,
     no_fifos: bool,
@@ -179,6 +185,7 @@ fn extract_archive(
     let options = PxarExtractOptions {
         match_list: &match_list,
         allow_existing_dirs,
+        overwrite_existing_files,
         extract_match_default,
         on_error,
     };
--
2.30.2

[pbs-devel] [PATCH proxmox-backup 2/3] skip xattr/acl/ownership options

[Markus Frank]

added cases to skip xattr/acl/ownership if the flags are not set.
Also added WITH_PERMISSIONS to Default-Flags, because otherwise it
would be needed to activly set it and most filesystems that support
XATTR and ACL also support POSIX-Permissions.

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 pbs-client/src/pxar/flags.rs    |  1 +
 pbs-client/src/pxar/metadata.rs | 48 +++++++++++++++++++++------------
 2 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/pbs-client/src/pxar/flags.rs b/pbs-client/src/pxar/flags.rs
index d46c8af3..938d0c57 100644
--- a/pbs-client/src/pxar/flags.rs
+++ b/pbs-client/src/pxar/flags.rs
@@ -135,6 +135,7 @@ bitflags! {
             Flags::WITH_FLAG_PROJINHERIT.bits() |
             Flags::WITH_SUBVOLUME.bits() |
             Flags::WITH_SUBVOLUME_RO.bits() |
+            Flags::WITH_PERMISSIONS.bits() |
             Flags::WITH_XATTRS.bits() |
             Flags::WITH_ACL.bits() |
             Flags::WITH_SELINUX.bits() |
diff --git a/pbs-client/src/pxar/metadata.rs b/pbs-client/src/pxar/metadata.rs
index 22bc5f9d..3195fb03 100644
--- a/pbs-client/src/pxar/metadata.rs
+++ b/pbs-client/src/pxar/metadata.rs
@@ -100,27 +100,17 @@ pub fn apply(
     on_error: &mut (dyn FnMut(Error) -> Result<(), Error> + Send),
 ) -> Result<(), Error> {
     let c_proc_path = CString::new(format!("/proc/self/fd/{}", fd)).unwrap();
+    apply_ownership(flags, c_proc_path.as_ptr(), metadata, &mut *on_error)?;
 
-    unsafe {
-        // UID and GID first, as this fails if we lose access anyway.
-        c_result!(libc::chown(
-            c_proc_path.as_ptr(),
-            metadata.stat.uid,
-            metadata.stat.gid
-        ))
-        .map(drop)
-        .or_else(allow_notsupp)
-        .map_err(|err| format_err!("failed to set ownership: {}", err))
-        .or_else(&mut *on_error)?;
-    }
-
-    let mut skip_xattrs = false;
+    let mut skip_xattrs = !flags.contains(Flags::WITH_XATTRS);
     apply_xattrs(flags, c_proc_path.as_ptr(), metadata, &mut skip_xattrs)
         .or_else(&mut *on_error)?;
     add_fcaps(flags, c_proc_path.as_ptr(), metadata, &mut skip_xattrs).or_else(&mut *on_error)?;
-    apply_acls(flags, &c_proc_path, metadata, path_info)
-        .map_err(|err| format_err!("failed to apply acls: {}", err))
-        .or_else(&mut *on_error)?;
+    if flags.contains(Flags::WITH_ACL) {
+        apply_acls(flags, &c_proc_path, metadata, path_info)
+            .map_err(|err| format_err!("failed to apply acls: {}", err))
+            .or_else(&mut *on_error)?;
+    }
     apply_quota_project_id(flags, fd, metadata).or_else(&mut *on_error)?;
 
     // Finally mode and time. We may lose access with mode, but the changing the mode also
@@ -162,6 +152,30 @@ pub fn apply(
     Ok(())
 }
 
+pub fn apply_ownership(
+    flags: Flags,
+    c_proc_path: *const libc::c_char,
+    metadata: &Metadata,
+    on_error: &mut (dyn FnMut(Error) -> Result<(), Error> + Send),
+) -> Result<(), Error> {
+    if !flags.contains(Flags::WITH_PERMISSIONS) {
+        return Ok(());
+    }
+    unsafe {
+        // UID and GID first, as this fails if we lose access anyway.
+        c_result!(libc::chown(
+            c_proc_path,
+            metadata.stat.uid,
+            metadata.stat.gid
+        ))
+        .map(drop)
+        .or_else(allow_notsupp)
+        .map_err(|err| format_err!("failed to set ownership: {}", err))
+        .or_else(&mut *on_error)?;
+    }
+    Ok(())
+}
+
 fn add_fcaps(
     flags: Flags,
     c_proc_path: *const libc::c_char,
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup 3/3] added ignore-acl/xattr/ownership & overwrite parameter to proxmox-backup-client

[Markus Frank]

If ignore-acl/ignore-xattr/ignore-ownership is set, the corresponding flag gets
removed.

overwrite-existing-files is saved as an PxarExtractOption like allow-existing-dirs.

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 proxmox-backup-client/src/main.rs | 40 ++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index 4bb9aa5e..20c29e26 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -1201,6 +1201,26 @@ We do not extract '.pxar' archives when writing to standard output.
                 type: CryptMode,
                 optional: true,
             },
+            "ignore-ownership": {
+                type: Boolean,
+                description: "ignore owner settings",
+                optional: true,
+            },
+            "ignore-acl": {
+                type: Boolean,
+                description: "ignore acl errors",
+                optional: true,
+            },
+            "ignore-xattr": {
+                type: Boolean,
+                description: "ignore xattr errors",
+                optional: true,
+            },
+            "overwrite-existing-files": {
+                type: Boolean,
+                description: "overwrite existing files",
+                optional: true,
+            },
         }
     }
 )]
@@ -1210,6 +1230,11 @@ async fn restore(param: Value) -> Result<Value, Error> {
 
     let allow_existing_dirs = param["allow-existing-dirs"].as_bool().unwrap_or(false);
 
+    let ignore_acls = param["ignore-acl"].as_bool().unwrap_or(false);
+    let ignore_xattrs = param["ignore-xattr"].as_bool().unwrap_or(false);
+    let ignore_ownership = param["ignore-ownership"].as_bool().unwrap_or(false);
+    let overwrite_existing_files = param["overwrite-existing-files"].as_bool().unwrap_or(false);
+
     let archive_name = json::required_string_param(&param, "archive-name")?;
 
     let rate = match param["rate"].as_str() {
@@ -1331,14 +1356,27 @@ async fn restore(param: Value) -> Result<Value, Error> {
             match_list: &[],
             extract_match_default: true,
             allow_existing_dirs,
+            overwrite_existing_files,
             on_error: None,
         };
 
+        let mut feature_flags = pbs_client::pxar::Flags::DEFAULT;
+
+        if ignore_ownership {
+            feature_flags.remove(pbs_client::pxar::Flags::WITH_PERMISSIONS);
+        }
+        if ignore_acls {
+            feature_flags.remove(pbs_client::pxar::Flags::WITH_ACL);
+        }
+        if ignore_xattrs {
+            feature_flags.remove(pbs_client::pxar::Flags::WITH_XATTRS);
+        }
+
         if let Some(target) = target {
             pbs_client::pxar::extract_archive(
                 pxar::decoder::Decoder::from_std(reader)?,
                 Path::new(target),
-                pbs_client::pxar::Flags::DEFAULT,
+                feature_flags,
                 |path| {
                     log::debug!("{:?}", path);
                 },
-- 
2.30.2

Re: [pbs-devel] [PATCH proxmox-backup 1/3] added overwrite-existing-files as

[Markus Frank]

I am sorry. Subject/Commit-Message should be:

added overwrite-existing-files to PxarExtractOptions

On 8/16/22 11:19, Markus Frank wrote:
> If true, O_EXCL is not set and therefore overwrites the files and does not
> error out.
> 
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
>   pbs-client/src/catalog_shell.rs |  4 ++--
>   pbs-client/src/pxar/extract.rs  | 24 +++++++++++++++++++++---
>   pxar-bin/src/main.rs            |  7 +++++++
>   3 files changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/pbs-client/src/catalog_shell.rs b/pbs-client/src/catalog_shell.rs
> index b11901ed..25e27b37 100644
> --- a/pbs-client/src/catalog_shell.rs
> +++ b/pbs-client/src/catalog_shell.rs
> @@ -984,7 +984,7 @@ impl Shell {
>               .clone();
> 
>           let extractor =
> -            crate::pxar::extract::Extractor::new(rootdir, root_meta, true, Flags::DEFAULT);
> +            crate::pxar::extract::Extractor::new(rootdir, root_meta, true, false, Flags::DEFAULT);
> 
>           let mut extractor = ExtractorState::new(
>               &mut self.catalog,
> @@ -1172,7 +1172,7 @@ impl<'a> ExtractorState<'a> {
>                   let file_name = CString::new(entry.file_name().as_bytes())?;
>                   let mut contents = entry.contents().await?;
>                   self.extractor
> -                    .async_extract_file(&file_name, entry.metadata(), *size, &mut contents)
> +                    .async_extract_file(&file_name, entry.metadata(), *size, &mut contents, false)
>                       .await
>               }
>               _ => {
> diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
> index 161d2cef..3b9151aa 100644
> --- a/pbs-client/src/pxar/extract.rs
> +++ b/pbs-client/src/pxar/extract.rs
> @@ -34,6 +34,7 @@ pub struct PxarExtractOptions<'a> {
>       pub match_list: &'a [MatchEntry],
>       pub extract_match_default: bool,
>       pub allow_existing_dirs: bool,
> +    pub overwrite_existing_files: bool,
>       pub on_error: Option<ErrorHandler>,
>   }
> 
> @@ -80,6 +81,7 @@ where
>           dir,
>           root.metadata().clone(),
>           options.allow_existing_dirs,
> +        options.overwrite_existing_files,
>           feature_flags,
>       );
> 
> @@ -198,6 +200,7 @@ where
>                   &mut decoder.contents().ok_or_else(|| {
>                       format_err!("found regular file entry without contents in archive")
>                   })?,
> +                extractor.overwrite_existing_files,
>               ),
>               (false, _) => Ok(()), // skip this
>           }
> @@ -215,6 +218,7 @@ where
>   pub struct Extractor {
>       feature_flags: Flags,
>       allow_existing_dirs: bool,
> +    overwrite_existing_files: bool,
>       dir_stack: PxarDirStack,
> 
>       /// For better error output we need to track the current path in the Extractor state.
> @@ -231,11 +235,13 @@ impl Extractor {
>           root_dir: Dir,
>           metadata: Metadata,
>           allow_existing_dirs: bool,
> +        overwrite_existing_files: bool,
>           feature_flags: Flags,
>       ) -> Self {
>           Self {
>               dir_stack: PxarDirStack::new(root_dir, metadata),
>               allow_existing_dirs,
> +            overwrite_existing_files,
>               feature_flags,
>               current_path: Arc::new(Mutex::new(OsString::new())),
>               on_error: Box::new(Err),
> @@ -392,14 +398,19 @@ impl Extractor {
>           metadata: &Metadata,
>           size: u64,
>           contents: &mut dyn io::Read,
> +        overwrite_existing_files: bool,
>       ) -> Result<(), Error> {
>           let parent = self.parent_fd()?;
> +        let mut oflags = OFlag::O_CREAT | OFlag::O_WRONLY | OFlag::O_CLOEXEC;
> +        if !overwrite_existing_files {
> +            oflags = oflags | OFlag::O_EXCL;
> +        }
>           let mut file = unsafe {
>               std::fs::File::from_raw_fd(
>                   nix::fcntl::openat(
>                       parent,
>                       file_name,
> -                    OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_WRONLY | OFlag::O_CLOEXEC,
> +                    oflags,
>                       Mode::from_bits(0o600).unwrap(),
>                   )
>                   .map_err(|err| format_err!("failed to create file {:?}: {}", file_name, err))?,
> @@ -448,14 +459,19 @@ impl Extractor {
>           metadata: &Metadata,
>           size: u64,
>           contents: &mut T,
> +        overwrite_existing_files: bool,
>       ) -> Result<(), Error> {
>           let parent = self.parent_fd()?;
> +        let mut oflags = OFlag::O_CREAT | OFlag::O_WRONLY | OFlag::O_CLOEXEC;
> +        if !overwrite_existing_files {
> +            oflags = oflags | OFlag::O_EXCL;
> +        }
>           let mut file = tokio::fs::File::from_std(unsafe {
>               std::fs::File::from_raw_fd(
>                   nix::fcntl::openat(
>                       parent,
>                       file_name,
> -                    OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_WRONLY | OFlag::O_CLOEXEC,
> +                    oflags,
>                       Mode::from_bits(0o600).unwrap(),
>                   )
>                   .map_err(|err| format_err!("failed to create file {:?}: {}", file_name, err))?,
> @@ -818,7 +834,7 @@ where
>           )
>       })?;
> 
> -    Ok(Extractor::new(dir, metadata, false, Flags::DEFAULT))
> +    Ok(Extractor::new(dir, metadata, false, false, Flags::DEFAULT))
>   }
> 
>   pub async fn extract_sub_dir<T, DEST, PATH>(
> @@ -951,6 +967,7 @@ where
>                       &mut file.contents().await.map_err(|_| {
>                           format_err!("found regular file entry without contents in archive")
>                       })?,
> +                    extractor.overwrite_existing_files,
>                   )
>                   .await?
>           }
> @@ -998,6 +1015,7 @@ where
>                               &mut decoder.contents().ok_or_else(|| {
>                                   format_err!("found regular file entry without contents in archive")
>                               })?,
> +                            extractor.overwrite_existing_files,
>                           )
>                           .await?
>                   }
> diff --git a/pxar-bin/src/main.rs b/pxar-bin/src/main.rs
> index 3714eb03..4b49df51 100644
> --- a/pxar-bin/src/main.rs
> +++ b/pxar-bin/src/main.rs
> @@ -75,6 +75,11 @@ fn extract_archive_from_reader<R: std::io::Read>(
>                   optional: true,
>                   default: false,
>               },
> +            "overwrite_existing_files": {
> +                description: "overwrite existing files",
> +                optional: true,
> +                default: false,
> +            },
>               "files-from": {
>                   description: "File containing match pattern for files to restore.",
>                   optional: true,
> @@ -112,6 +117,7 @@ fn extract_archive(
>       no_fcaps: bool,
>       no_acls: bool,
>       allow_existing_dirs: bool,
> +    overwrite_existing_files: bool,
>       files_from: Option<String>,
>       no_device_nodes: bool,
>       no_fifos: bool,
> @@ -179,6 +185,7 @@ fn extract_archive(
>       let options = PxarExtractOptions {
>           match_list: &match_list,
>           allow_existing_dirs,
> +        overwrite_existing_files,
>           extract_match_default,
>           on_error,
>       };
> --
> 2.30.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 

Re: [pbs-devel] [PATCH proxmox-backup 1/3] added overwrite-existing-files as

[Wolfgang Bumiller]

On Tue, Aug 16, 2022 at 11:19:27AM +0200, Markus Frank wrote:
> If true, O_EXCL is not set and therefore overwrites the files and does not
> error out.
> 
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
>  pbs-client/src/catalog_shell.rs |  4 ++--
>  pbs-client/src/pxar/extract.rs  | 24 +++++++++++++++++++++---
>  pxar-bin/src/main.rs            |  7 +++++++
>  3 files changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/pbs-client/src/catalog_shell.rs b/pbs-client/src/catalog_shell.rs
> index b11901ed..25e27b37 100644
> --- a/pbs-client/src/catalog_shell.rs
> +++ b/pbs-client/src/catalog_shell.rs
> @@ -984,7 +984,7 @@ impl Shell {
>              .clone();
> 
>          let extractor =
> -            crate::pxar::extract::Extractor::new(rootdir, root_meta, true, Flags::DEFAULT);
> +            crate::pxar::extract::Extractor::new(rootdir, root_meta, true, false, Flags::DEFAULT);
> 
>          let mut extractor = ExtractorState::new(
>              &mut self.catalog,
> @@ -1172,7 +1172,7 @@ impl<'a> ExtractorState<'a> {
>                  let file_name = CString::new(entry.file_name().as_bytes())?;
>                  let mut contents = entry.contents().await?;
>                  self.extractor
> -                    .async_extract_file(&file_name, entry.metadata(), *size, &mut contents)
> +                    .async_extract_file(&file_name, entry.metadata(), *size, &mut contents, false)
>                      .await
>              }
>              _ => {
> diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
> index 161d2cef..3b9151aa 100644
> --- a/pbs-client/src/pxar/extract.rs
> +++ b/pbs-client/src/pxar/extract.rs
> @@ -34,6 +34,7 @@ pub struct PxarExtractOptions<'a> {
>      pub match_list: &'a [MatchEntry],
>      pub extract_match_default: bool,
>      pub allow_existing_dirs: bool,
> +    pub overwrite_existing_files: bool,
>      pub on_error: Option<ErrorHandler>,
>  }
> 
> @@ -80,6 +81,7 @@ where
>          dir,
>          root.metadata().clone(),
>          options.allow_existing_dirs,
> +        options.overwrite_existing_files,
>          feature_flags,
>      );
> 
> @@ -198,6 +200,7 @@ where
>                  &mut decoder.contents().ok_or_else(|| {
>                      format_err!("found regular file entry without contents in archive")
>                  })?,
> +                extractor.overwrite_existing_files,
>              ),
>              (false, _) => Ok(()), // skip this
>          }
> @@ -215,6 +218,7 @@ where
>  pub struct Extractor {
>      feature_flags: Flags,
>      allow_existing_dirs: bool,
> +    overwrite_existing_files: bool,
>      dir_stack: PxarDirStack,
> 
>      /// For better error output we need to track the current path in the Extractor state.
> @@ -231,11 +235,13 @@ impl Extractor {
>          root_dir: Dir,
>          metadata: Metadata,
>          allow_existing_dirs: bool,
> +        overwrite_existing_files: bool,
>          feature_flags: Flags,
>      ) -> Self {
>          Self {
>              dir_stack: PxarDirStack::new(root_dir, metadata),
>              allow_existing_dirs,
> +            overwrite_existing_files,
>              feature_flags,
>              current_path: Arc::new(Mutex::new(OsString::new())),
>              on_error: Box::new(Err),
> @@ -392,14 +398,19 @@ impl Extractor {
>          metadata: &Metadata,
>          size: u64,
>          contents: &mut dyn io::Read,
> +        overwrite_existing_files: bool,
>      ) -> Result<(), Error> {
>          let parent = self.parent_fd()?;
> +        let mut oflags = OFlag::O_CREAT | OFlag::O_WRONLY | OFlag::O_CLOEXEC;

When replacing existing files we should include `O_TRUNC`, too,
otherwise if the new file is smaller and doesn't end in a hole (where we
already fixup the size with an explicit truncate() call) it'll have
parts of the previous contents left over at the end.

> +        if !overwrite_existing_files {
> +            oflags = oflags | OFlag::O_EXCL;
> +        }
>          let mut file = unsafe {
>              std::fs::File::from_raw_fd(
>                  nix::fcntl::openat(
>                      parent,
>                      file_name,
> -                    OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_WRONLY | OFlag::O_CLOEXEC,
> +                    oflags,
>                      Mode::from_bits(0o600).unwrap(),
>                  )
>                  .map_err(|err| format_err!("failed to create file {:?}: {}", file_name, err))?,
> @@ -448,14 +459,19 @@ impl Extractor {
>          metadata: &Metadata,
>          size: u64,
>          contents: &mut T,
> +        overwrite_existing_files: bool,
>      ) -> Result<(), Error> {
>          let parent = self.parent_fd()?;
> +        let mut oflags = OFlag::O_CREAT | OFlag::O_WRONLY | OFlag::O_CLOEXEC;
> +        if !overwrite_existing_files {
> +            oflags = oflags | OFlag::O_EXCL;
> +        }

same here

>          let mut file = tokio::fs::File::from_std(unsafe {
>              std::fs::File::from_raw_fd(
>                  nix::fcntl::openat(
>                      parent,
>                      file_name,
> -                    OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_WRONLY | OFlag::O_CLOEXEC,
> +                    oflags,
>                      Mode::from_bits(0o600).unwrap(),
>                  )
>                  .map_err(|err| format_err!("failed to create file {:?}: {}", file_name, err))?,
> @@ -818,7 +834,7 @@ where
>          )
>      })?;
> 
> -    Ok(Extractor::new(dir, metadata, false, Flags::DEFAULT))
> +    Ok(Extractor::new(dir, metadata, false, false, Flags::DEFAULT))
>  }
> 
>  pub async fn extract_sub_dir<T, DEST, PATH>(
> @@ -951,6 +967,7 @@ where
>                      &mut file.contents().await.map_err(|_| {
>                          format_err!("found regular file entry without contents in archive")
>                      })?,
> +                    extractor.overwrite_existing_files,
>                  )
>                  .await?
>          }
> @@ -998,6 +1015,7 @@ where
>                              &mut decoder.contents().ok_or_else(|| {
>                                  format_err!("found regular file entry without contents in archive")
>                              })?,
> +                            extractor.overwrite_existing_files,
>                          )
>                          .await?
>                  }
> diff --git a/pxar-bin/src/main.rs b/pxar-bin/src/main.rs
> index 3714eb03..4b49df51 100644
> --- a/pxar-bin/src/main.rs
> +++ b/pxar-bin/src/main.rs
> @@ -75,6 +75,11 @@ fn extract_archive_from_reader<R: std::io::Read>(
>                  optional: true,
>                  default: false,
>              },
> +            "overwrite_existing_files": {
> +                description: "overwrite existing files",
> +                optional: true,
> +                default: false,
> +            },
>              "files-from": {
>                  description: "File containing match pattern for files to restore.",
>                  optional: true,
> @@ -112,6 +117,7 @@ fn extract_archive(
>      no_fcaps: bool,
>      no_acls: bool,
>      allow_existing_dirs: bool,
> +    overwrite_existing_files: bool,
>      files_from: Option<String>,
>      no_device_nodes: bool,
>      no_fifos: bool,
> @@ -179,6 +185,7 @@ fn extract_archive(
>      let options = PxarExtractOptions {
>          match_list: &match_list,
>          allow_existing_dirs,
> +        overwrite_existing_files,
>          extract_match_default,
>          on_error,
>      };
> --
> 2.30.2

Re: [pbs-devel] [PATCH proxmox-backup 2/3] skip xattr/acl/ownership options

[Wolfgang Bumiller]

On Tue, Aug 16, 2022 at 11:19:28AM +0200, Markus Frank wrote:
> added cases to skip xattr/acl/ownership if the flags are not set.
> Also added WITH_PERMISSIONS to Default-Flags, because otherwise it
> would be needed to activly set it and most filesystems that support
> XATTR and ACL also support POSIX-Permissions.
> 
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
>  pbs-client/src/pxar/flags.rs    |  1 +
>  pbs-client/src/pxar/metadata.rs | 48 +++++++++++++++++++++------------
>  2 files changed, 32 insertions(+), 17 deletions(-)
> 
> diff --git a/pbs-client/src/pxar/flags.rs b/pbs-client/src/pxar/flags.rs
> index d46c8af3..938d0c57 100644
> --- a/pbs-client/src/pxar/flags.rs
> +++ b/pbs-client/src/pxar/flags.rs
> @@ -135,6 +135,7 @@ bitflags! {
>              Flags::WITH_FLAG_PROJINHERIT.bits() |
>              Flags::WITH_SUBVOLUME.bits() |
>              Flags::WITH_SUBVOLUME_RO.bits() |
> +            Flags::WITH_PERMISSIONS.bits() |
>              Flags::WITH_XATTRS.bits() |
>              Flags::WITH_ACL.bits() |
>              Flags::WITH_SELINUX.bits() |
> diff --git a/pbs-client/src/pxar/metadata.rs b/pbs-client/src/pxar/metadata.rs
> index 22bc5f9d..3195fb03 100644
> --- a/pbs-client/src/pxar/metadata.rs
> +++ b/pbs-client/src/pxar/metadata.rs
> @@ -100,27 +100,17 @@ pub fn apply(
>      on_error: &mut (dyn FnMut(Error) -> Result<(), Error> + Send),
>  ) -> Result<(), Error> {
>      let c_proc_path = CString::new(format!("/proc/self/fd/{}", fd)).unwrap();
> +    apply_ownership(flags, c_proc_path.as_ptr(), metadata, &mut *on_error)?;
>  
> -    unsafe {
> -        // UID and GID first, as this fails if we lose access anyway.
> -        c_result!(libc::chown(
> -            c_proc_path.as_ptr(),
> -            metadata.stat.uid,
> -            metadata.stat.gid
> -        ))
> -        .map(drop)
> -        .or_else(allow_notsupp)
> -        .map_err(|err| format_err!("failed to set ownership: {}", err))
> -        .or_else(&mut *on_error)?;
> -    }
> -
> -    let mut skip_xattrs = false;
> +    let mut skip_xattrs = !flags.contains(Flags::WITH_XATTRS);
>      apply_xattrs(flags, c_proc_path.as_ptr(), metadata, &mut skip_xattrs)
>          .or_else(&mut *on_error)?;
>      add_fcaps(flags, c_proc_path.as_ptr(), metadata, &mut skip_xattrs).or_else(&mut *on_error)?;
> -    apply_acls(flags, &c_proc_path, metadata, path_info)
> -        .map_err(|err| format_err!("failed to apply acls: {}", err))
> -        .or_else(&mut *on_error)?;
> +    if flags.contains(Flags::WITH_ACL) {
> +        apply_acls(flags, &c_proc_path, metadata, path_info)
> +            .map_err(|err| format_err!("failed to apply acls: {}", err))
> +            .or_else(&mut *on_error)?;
> +    }
>      apply_quota_project_id(flags, fd, metadata).or_else(&mut *on_error)?;
>  
>      // Finally mode and time. We may lose access with mode, but the changing the mode also
> @@ -162,6 +152,30 @@ pub fn apply(
>      Ok(())
>  }
>  
> +pub fn apply_ownership(
> +    flags: Flags,
> +    c_proc_path: *const libc::c_char,
> +    metadata: &Metadata,
> +    on_error: &mut (dyn FnMut(Error) -> Result<(), Error> + Send),
> +) -> Result<(), Error> {
> +    if !flags.contains(Flags::WITH_PERMISSIONS) {
> +        return Ok(());
> +    }

While we're at it, I think we should rename `WITH_PERMISSIONS` to
`WITH_OWNER`, as it doesn't affect permissions ;-)
We could mask the chmod call in metadata::apply with this instead and
have both flags separately?

But I'd like to get rid fo this confusion in the code :-)

> +    unsafe {
> +        // UID and GID first, as this fails if we lose access anyway.
> +        c_result!(libc::chown(
> +            c_proc_path,
> +            metadata.stat.uid,
> +            metadata.stat.gid
> +        ))
> +        .map(drop)
> +        .or_else(allow_notsupp)
> +        .map_err(|err| format_err!("failed to set ownership: {}", err))
> +        .or_else(&mut *on_error)?;
> +    }
> +    Ok(())
> +}
> +
>  fn add_fcaps(
>      flags: Flags,
>      c_proc_path: *const libc::c_char,
> -- 
> 2.30.2