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

Wolfgang Bumiller w.bumiller at proxmox.com
Wed Aug 17 09:31:55 CEST 2022


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 at 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





More information about the pbs-devel mailing list