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

[pbs-devel] [PATCH v3 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    | 85 ++++++++++++++++++++++++-------
 pbs-client/src/pxar/mod.rs        |  2 +-
 proxmox-backup-client/src/main.rs | 34 ++++++++++++-
 pxar-bin/src/main.rs              | 32 +++++++++++-
 5 files changed, 139 insertions(+), 23 deletions(-)

-- 
2.39.2

[pbs-devel] [PATCH v3 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

changes since v2:
* Capture `err` in match pattern
* Deduplicate code by introducing `dolink` closure

 pbs-client/src/pxar/extract.rs | 46 ++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
index 4dbaf52d..9a95faa0 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(err @ nix::errno::Errno::EEXIST) => {
+                if !self.overwrite {
+                    return Err(err.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,29 @@ 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(
-            Some(root.as_raw_fd()),
-            target.as_c_str(),
-            Some(parent),
-            file_name,
-            nix::unistd::LinkatFlags::NoSymlinkFollow,
-        )?;
+        let dolink = || {
+            nix::unistd::linkat(
+                Some(root.as_raw_fd()),
+                target.as_c_str(),
+                Some(parent),
+                file_name,
+                nix::unistd::LinkatFlags::NoSymlinkFollow,
+            )
+        };
+
+        match dolink() {
+            Ok(()) => {}
+            Err(err @ nix::errno::Errno::EEXIST) => {
+                if !self.overwrite {
+                    return Err(err.into());
+                }
+                // Never unlink directories
+                let flag = nix::unistd::UnlinkatFlags::NoRemoveDir;
+                nix::unistd::unlinkat(Some(parent), file_name, flag)?;
+                dolink()?;
+            }
+            Err(err) => return Err(err.into())
+        }
 
         Ok(())
     }
-- 
2.39.2

[pbs-devel] [PATCH v3 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 to current master

changes since v2:
* use `all()` and `empty()` instead of defining ALL and NONE variants
  for bitflags.
* Derive default instead of explicit impl.
* Fix formating issues by running rustfmt 

 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 | 34 +++++++++++++++++++++-
 pxar-bin/src/main.rs              | 32 +++++++++++++++++++--
 5 files changed, 105 insertions(+), 19 deletions(-)

diff --git a/pbs-client/src/catalog_shell.rs b/pbs-client/src/catalog_shell.rs
index 98af5699..b8aaf8cb 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::empty(),
+            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 9a95faa0..37399f17 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,22 @@ 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! {
+    #[derive(Default)]
+    pub struct OverwriteFlags: u8 {
+        /// Overwrite existing entries file content
+        const FILE = 0x1;
+        /// Overwrite existing entry with symlink
+        const SYMLINK = 0x2;
+        /// Overwrite existing entry with hardlink
+        const HARDLINK = 0x4;
+    }
+}
+
 pub type ErrorHandler = Box<dyn FnMut(Error) -> Result<(), Error> + Send>;
 
 pub fn extract_archive<T, F>(
@@ -141,7 +154,7 @@ where
             dir,
             root.metadata().clone(),
             options.allow_existing_dirs,
-            options.overwrite,
+            options.overwrite_flags,
             feature_flags,
         );
 
@@ -345,7 +358,9 @@ where
                         metadata,
                         *size,
                         &mut contents,
-                        self.extractor.overwrite,
+                        self.extractor
+                            .overwrite_flags
+                            .contains(OverwriteFlags::FILE),
                     )
                 } else {
                     Err(format_err!(
@@ -438,7 +453,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 +470,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 +566,7 @@ impl Extractor {
         match nix::unistd::symlinkat(link, Some(parent), file_name) {
             Ok(()) => {}
             Err(err @ nix::errno::Errno::EEXIST) => {
-                if !self.overwrite {
+                if !self.overwrite_flags.contains(OverwriteFlags::SYMLINK) {
                     return Err(err.into());
                 }
                 // Never unlink directories
@@ -559,7 +574,7 @@ impl Extractor {
                 nix::unistd::unlinkat(Some(parent), file_name, flag)?;
                 nix::unistd::symlinkat(link, Some(parent), file_name)?;
             }
-            Err(err) => return Err(err.into())
+            Err(err) => return Err(err.into()),
         }
 
         metadata::apply_at(
@@ -591,7 +606,7 @@ impl Extractor {
         match dolink() {
             Ok(()) => {}
             Err(err @ nix::errno::Errno::EEXIST) => {
-                if !self.overwrite {
+                if !self.overwrite_flags.contains(OverwriteFlags::HARDLINK) {
                     return Err(err.into());
                 }
                 // Never unlink directories
@@ -599,7 +614,7 @@ impl Extractor {
                 nix::unistd::unlinkat(Some(parent), file_name, flag)?;
                 dolink()?;
             }
-            Err(err) => return Err(err.into())
+            Err(err) => return Err(err.into()),
         }
 
         Ok(())
@@ -1062,7 +1077,13 @@ 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::empty(),
+        Flags::DEFAULT,
+    ))
 }
 
 pub async fn extract_sub_dir<T, DEST, PATH>(
@@ -1196,7 +1217,7 @@ where
                         .contents()
                         .await
                         .context("found regular file entry without contents in archive")?,
-                    extractor.overwrite,
+                    extractor.overwrite_flags.contains(OverwriteFlags::FILE),
                 )
                 .await?
         }
@@ -1244,7 +1265,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..14674b9b 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,
+    OverwriteFlags, PxarExtractContext, PxarExtractOptions,
 };
 
 /// 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..1a13291a 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,25 @@ async fn restore(
             None
         };
 
+        let mut overwrite_flags = pbs_client::pxar::OverwriteFlags::empty();
+        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..bc044035 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, OverwriteFlags, PxarExtractOptions, 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,
@@ -142,6 +162,14 @@ fn extract_archive(
         feature_flags.remove(Flags::WITH_SOCKETS);
     }
 
+    let mut overwrite_flags = OverwriteFlags::empty();
+    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

[pbs-devel] applied: [PATCH v3 proxmox-backup 0/2] Introduce bitflags for overwrite

[Wolfgang Bumiller]

applied both patches, thanks