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

[pbs-devel] [PATCH 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 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>
---
 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 f6c1991f..14f9b43d 100644
--- a/pbs-client/src/pxar/extract.rs
+++ b/pbs-client/src/pxar/extract.rs
@@ -326,7 +326,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,
@@ -343,13 +357,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 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>
---
 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 14f9b43d..60029dbd 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, 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>(
@@ -80,7 +105,7 @@ where
         dir,
         root.metadata().clone(),
         options.allow_existing_dirs,
-        options.overwrite,
+        options.overwrite_flags,
         feature_flags,
     );
 
@@ -199,7 +224,7 @@ where
                 &mut decoder.contents().ok_or_else(|| {
                     format_err!("found regular file entry without contents in archive")
                 })?,
-                extractor.overwrite,
+                extractor.overwrite_flags.contains(OverwriteFlags::FILE),
             ),
             (false, _) => Ok(()), // skip this
         }
@@ -217,7 +242,7 @@ where
 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.
@@ -234,13 +259,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),
@@ -330,7 +355,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
@@ -367,7 +392,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
@@ -860,7 +885,7 @@ where
         )
     })?;
 
-    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>(
@@ -993,7 +1018,7 @@ where
                     &mut file.contents().await.map_err(|_| {
                         format_err!("found regular file entry without contents in archive")
                     })?,
-                    extractor.overwrite,
+                    extractor.overwrite_flags.contains(OverwriteFlags::FILE),
                 )
                 .await?
         }
@@ -1041,7 +1066,7 @@ where
                             &mut decoder.contents().ok_or_else(|| {
                                 format_err!("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 a158101d..81c535c7 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,
-    PxarExtractOptions,
+    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 55198108..b56f8c78 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -1232,6 +1232,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,
+            },
         }
     }
 )]
@@ -1244,6 +1259,9 @@ async fn restore(
     ignore_ownership: bool,
     ignore_permissions: bool,
     overwrite: bool,
+    overwrite_files: bool,
+    overwrite_symlinks: bool,
+    overwrite_hardlinks: bool,
 ) -> Result<Value, Error> {
     let repo = extract_repository_from_value(&param)?;
 
@@ -1364,11 +1382,19 @@ async fn restore(
 
         let mut reader = BufferedDynamicReader::new(index, chunk_reader);
 
+        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: None,
         };
 
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 proxmox-backup 0/2] Introduce bitflags for overwrite

[Christian Ebner]

Please ignore this one, I sent a rebased and refactored v2.

Cheers,
Chris

> On 07.06.2023 15:30 CEST Christian Ebner <c.ebner@proxmox.com> wrote:
> 
>  
> 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