[pbs-devel] [PATCH backup v2 1/2] use truncate whenever we create files

[pbs-devel] [PATCH backup v2 1/2] use truncate whenever we create files

[Maximiliano Sandoval]

Fixes the suspicious_open_options clippy lint, for example:

```
warning: file opened with `create`, but `truncate` behavior not defined
    --> src/api2/tape/restore.rs:1713:18
     |
1713 |                 .create(true)
     |                  ^^^^^^^^^^^^- help: add: `.truncate(true)`
     |
     = help: if you intend to overwrite an existing file entirely, call `.truncate(true)`
     = help: if you instead know that you may want to keep some parts of the old file, call `.truncate(false)`
     = help: alternatively, use `.append(true)` to append to the file instead of overwriting it
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_open_options
```

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
Differences from v1:
 - Do not add truncate/create when create_new is used

 pbs-client/src/pxar/extract.rs | 1 +
 src/api2/tape/restore.rs       | 1 +
 src/server/sync.rs             | 1 +
 3 files changed, 3 insertions(+)

diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
index 64754006..1f017d81 100644
--- a/pbs-client/src/pxar/extract.rs
+++ b/pbs-client/src/pxar/extract.rs
@@ -135,6 +135,7 @@ where
             if let Some(entry) = prelude {
                 let mut prelude_file = OpenOptions::new()
                     .create(true)
+                    .truncate(true)
                     .write(true)
                     .open(path)
                     .with_context(|| format!("error creating prelude file '{path:?}'"))?;
diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
index 93a6053b..2cc1baab 100644
--- a/src/api2/tape/restore.rs
+++ b/src/api2/tape/restore.rs
@@ -1711,6 +1711,7 @@ fn try_restore_snapshot_archive<R: pxar::decoder::SeqRead>(
             let mut tmpfile = std::fs::OpenOptions::new()
                 .write(true)
                 .create(true)
+                .truncate(true)
                 .read(true)
                 .open(&tmp_path)
                 .map_err(|err| format_err!("restore {:?} failed - {}", tmp_path, err))?;
diff --git a/src/server/sync.rs b/src/server/sync.rs
index 5e3fbdcd..4dd46c5a 100644
--- a/src/server/sync.rs
+++ b/src/server/sync.rs
@@ -159,6 +159,7 @@ impl SyncSourceReader for RemoteSourceReader {
         let tmpfile = std::fs::OpenOptions::new()
             .write(true)
             .create(true)
+            .truncate(true)
             .read(true)
             .open(&tmp_path)?;
 
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH backup v2 2/2] remove create & truncate when create_new is used

[Maximiliano Sandoval]

As per its documentation [1]:

> If .create_new(true) is set, .create() and .truncate() are ignored.

This gets rid of the "file opened with `create`, but `truncate`
behavior not defined " clippy warnings.

[1] https://doc.rust-lang.org/std/fs/struct.OpenOptions.html#method.create_new

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-datastore/src/datastore.rs    | 2 +-
 proxmox-backup-client/src/main.rs | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index fd3990d6..75c0c16a 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -749,9 +749,9 @@ impl DataStore {
 
         let mut open_options = std::fs::OpenOptions::new();
         open_options.write(true);
-        open_options.truncate(true);
 
         if force {
+            open_options.truncate(true);
             open_options.create(true);
         } else {
             open_options.create_new(true);
diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index 632a2917..589a097b 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -1598,7 +1598,6 @@ async fn restore(
         if let Some(target) = target {
             let mut writer = std::fs::OpenOptions::new()
                 .write(true)
-                .create(true)
                 .create_new(true)
                 .open(target)
                 .map_err(|err| {
@@ -1724,7 +1723,6 @@ async fn restore(
         let mut writer = if let Some(target) = target {
             std::fs::OpenOptions::new()
                 .write(true)
-                .create(true)
                 .create_new(true)
                 .open(target)
                 .map_err(|err| format_err!("unable to create target file {:?} - {}", target, err))?
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] applied: [PATCH backup v2 2/2] remove create & truncate when create_new is used

[Fabian Grünbichler]

applied this one, although we might want more fine-grained control for
the last hunk to allow directly extracting to a block device or raw
image file?

On January 27, 2025 10:24 am, Maximiliano Sandoval wrote:
> As per its documentation [1]:
> 
>> If .create_new(true) is set, .create() and .truncate() are ignored.
> 
> This gets rid of the "file opened with `create`, but `truncate`
> behavior not defined " clippy warnings.
> 
> [1] https://doc.rust-lang.org/std/fs/struct.OpenOptions.html#method.create_new
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>  pbs-datastore/src/datastore.rs    | 2 +-
>  proxmox-backup-client/src/main.rs | 2 --
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
> index fd3990d6..75c0c16a 100644
> --- a/pbs-datastore/src/datastore.rs
> +++ b/pbs-datastore/src/datastore.rs
> @@ -749,9 +749,9 @@ impl DataStore {
>  
>          let mut open_options = std::fs::OpenOptions::new();
>          open_options.write(true);
> -        open_options.truncate(true);
>  
>          if force {
> +            open_options.truncate(true);
>              open_options.create(true);
>          } else {
>              open_options.create_new(true);
> diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
> index 632a2917..589a097b 100644
> --- a/proxmox-backup-client/src/main.rs
> +++ b/proxmox-backup-client/src/main.rs
> @@ -1598,7 +1598,6 @@ async fn restore(
>          if let Some(target) = target {
>              let mut writer = std::fs::OpenOptions::new()
>                  .write(true)
> -                .create(true)
>                  .create_new(true)
>                  .open(target)
>                  .map_err(|err| {
> @@ -1724,7 +1723,6 @@ async fn restore(
>          let mut writer = if let Some(target) = target {
>              std::fs::OpenOptions::new()
>                  .write(true)
> -                .create(true)
>                  .create_new(true)
>                  .open(target)
>                  .map_err(|err| format_err!("unable to create target file {:?} - {}", target, err))?
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH backup v2 1/2] use truncate whenever we create files

[Fabian Grünbichler]

On January 27, 2025 10:24 am, Maximiliano Sandoval wrote:
> Fixes the suspicious_open_options clippy lint, for example:
> 
> ```
> warning: file opened with `create`, but `truncate` behavior not defined
>     --> src/api2/tape/restore.rs:1713:18
>      |
> 1713 |                 .create(true)
>      |                  ^^^^^^^^^^^^- help: add: `.truncate(true)`
>      |
>      = help: if you intend to overwrite an existing file entirely, call `.truncate(true)`
>      = help: if you instead know that you may want to keep some parts of the old file, call `.truncate(false)`
>      = help: alternatively, use `.append(true)` to append to the file instead of overwriting it
>      = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_open_options
> ```
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
> Differences from v1:
>  - Do not add truncate/create when create_new is used
> 
>  pbs-client/src/pxar/extract.rs | 1 +
>  src/api2/tape/restore.rs       | 1 +
>  src/server/sync.rs             | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
> index 64754006..1f017d81 100644
> --- a/pbs-client/src/pxar/extract.rs
> +++ b/pbs-client/src/pxar/extract.rs
> @@ -135,6 +135,7 @@ where
>              if let Some(entry) = prelude {
>                  let mut prelude_file = OpenOptions::new()
>                      .create(true)
> +                    .truncate(true)

this is not correct, this is a (special) file that is restored, so it
should probably be handled like regular files are - if overwrite is set,
truncate, else, create_new ..

>                      .write(true)
>                      .open(path)
>                      .with_context(|| format!("error creating prelude file '{path:?}'"))?;
> diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
> index 93a6053b..2cc1baab 100644
> --- a/src/api2/tape/restore.rs
> +++ b/src/api2/tape/restore.rs
> @@ -1711,6 +1711,7 @@ fn try_restore_snapshot_archive<R: pxar::decoder::SeqRead>(
>              let mut tmpfile = std::fs::OpenOptions::new()
>                  .write(true)
>                  .create(true)
> +                .truncate(true)
>                  .read(true)
>                  .open(&tmp_path)
>                  .map_err(|err| format_err!("restore {:?} failed - {}", tmp_path, err))?;
> diff --git a/src/server/sync.rs b/src/server/sync.rs
> index 5e3fbdcd..4dd46c5a 100644
> --- a/src/server/sync.rs
> +++ b/src/server/sync.rs
> @@ -159,6 +159,7 @@ impl SyncSourceReader for RemoteSourceReader {
>          let tmpfile = std::fs::OpenOptions::new()
>              .write(true)
>              .create(true)
> +            .truncate(true)
>              .read(true)
>              .open(&tmp_path)?;
>  
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel