[pve-devel] [PATCH proxmox-backup/common/storage v4] improve file-restore timeout behaviour

[pve-devel] [PATCH proxmox-backup/common/storage v4] improve file-restore timeout behaviour

[Dominik Csapak]

this series improves the behaviour of the file-restore when some mount
operations take longer than the 30 second pveproxy timeout

and enables/disables the automatic increasing of memory depending
on the users privileges

depends (i'll add it to the seperate patches too)
pve-storage 1/3 fixes the currently broken error output and
does not depend on the rest of the series
pve-storage 2/3 depends on pve-common 1/2
pve-storage 3/3 depends on pve-common 2/2
pve-common 2/2 depends on proxmox-backup

changes from v3:
* don't hardcode the timeout parameter in pve-common, but make it a
  hash were we explicitely extract the timeout
* split the pve-storage commit into two, one for handling the error
  format and one for the timeout
* add patches to handle the dynamic memory behaviour

changes from v2:
* couple the error format to 'ouput-format' instead of 'json-error'
* remove 'error' property from 'error json object' (was redundant)
* always expect an error when we get an object and always treat
  it as ok when we get a list

changes from v1:
* rebased on master
* moved the json-error and timeout directly into pve-common (hardcoded)
  since there is only one usage of that function

proxmox-backup:

Dominik Csapak (1):
  file-restore: make dynamic memory behaviour controllable

 proxmox-file-restore/src/block_driver.rs      | 10 +++-
 proxmox-file-restore/src/block_driver_qemu.rs |  6 ++-
 proxmox-file-restore/src/main.rs              | 54 +++++++++++++++++--
 3 files changed, 61 insertions(+), 9 deletions(-)

pve-common:

Dominik Csapak (2):
  PBSClient: file_restore_list: add extraParams and use timeout
  PBSClient: add optional 'dynamic-memory' parameter to file-restore
    commands

 src/PVE/PBSClient.pm | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

pve-storage:

Dominik Csapak (3):
  api: FileRestore: decode and return proper error of file-restore
    listing
  api: FileRestore: make use of file-restores and guis timeout mechanism
  api: FileRestore: allow automatic memory increase for privileged users

 PVE/API2/Storage/FileRestore.pm | 48 +++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 8 deletions(-)

-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup/common/storage v4] improve file-restore timeout behaviour

[Dominik Csapak]

this series improves the behaviour of the file-restore when some mount
operations take longer than the 30 second pveproxy timeout

and enables/disables the automatic increasing of memory depending
on the users privileges

depends (i'll add it to the seperate patches too)
pve-storage 1/3 fixes the currently broken error output and
does not depend on the rest of the series
pve-storage 2/3 depends on pve-common 1/2
pve-storage 3/3 depends on pve-common 2/2
pve-common 2/2 depends on proxmox-backup

changes from v3:
* don't hardcode the timeout parameter in pve-common, but make it a
  hash were we explicitely extract the timeout
* split the pve-storage commit into two, one for handling the error
  format and one for the timeout
* add patches to handle the dynamic memory behaviour

changes from v2:
* couple the error format to 'ouput-format' instead of 'json-error'
* remove 'error' property from 'error json object' (was redundant)
* always expect an error when we get an object and always treat
  it as ok when we get a list

changes from v1:
* rebased on master
* moved the json-error and timeout directly into pve-common (hardcoded)
  since there is only one usage of that function

proxmox-backup:

Dominik Csapak (1):
  file-restore: make dynamic memory behaviour controllable

 proxmox-file-restore/src/block_driver.rs      | 10 +++-
 proxmox-file-restore/src/block_driver_qemu.rs |  6 ++-
 proxmox-file-restore/src/main.rs              | 54 +++++++++++++++++--
 3 files changed, 61 insertions(+), 9 deletions(-)

pve-common:

Dominik Csapak (2):
  PBSClient: file_restore_list: add extraParams and use timeout
  PBSClient: add optional 'dynamic-memory' parameter to file-restore
    commands

 src/PVE/PBSClient.pm | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

pve-storage:

Dominik Csapak (3):
  api: FileRestore: decode and return proper error of file-restore
    listing
  api: FileRestore: make use of file-restores and guis timeout mechanism
  api: FileRestore: allow automatic memory increase for privileged users

 PVE/API2/Storage/FileRestore.pm | 48 +++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 8 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH proxmox-backup v4 1/1] file-restore: make dynamic memory behaviour controllable

[Dominik Csapak]

by adding 'dynamic-memory' parameter that controls if we automatically
increase the memory of the guest vm or not

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
new in v4

not really happy with the 'clippy allow', but imho there is no obvious way to
bundle those parameters without restructuring the code much

 proxmox-file-restore/src/block_driver.rs      | 10 +++-
 proxmox-file-restore/src/block_driver_qemu.rs |  6 ++-
 proxmox-file-restore/src/main.rs              | 54 +++++++++++++++++--
 3 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/proxmox-file-restore/src/block_driver.rs b/proxmox-file-restore/src/block_driver.rs
index 01cc4e18..ad2da5fb 100644
--- a/proxmox-file-restore/src/block_driver.rs
+++ b/proxmox-file-restore/src/block_driver.rs
@@ -43,6 +43,7 @@ pub trait BlockRestoreDriver {
         details: SnapRestoreDetails,
         img_file: String,
         path: Vec<u8>,
+        dynamic_memory: bool,
     ) -> Async<Result<Vec<ArchiveEntry>, Error>>;
 
     /// pxar=true:
@@ -57,6 +58,7 @@ pub trait BlockRestoreDriver {
         path: Vec<u8>,
         format: Option<FileRestoreFormat>,
         zstd: bool,
+        dynamic_memory: bool,
     ) -> Async<Result<Box<dyn tokio::io::AsyncRead + Unpin + Send>, Error>>;
 
     /// Return status of all running/mapped images, result value is (id, extra data), where id must
@@ -92,9 +94,12 @@ pub async fn data_list(
     details: SnapRestoreDetails,
     img_file: String,
     path: Vec<u8>,
+    dynamic_memory: bool,
 ) -> Result<Vec<ArchiveEntry>, Error> {
     let driver = driver.unwrap_or(DEFAULT_DRIVER).resolve();
-    driver.data_list(details, img_file, path).await
+    driver
+        .data_list(details, img_file, path, dynamic_memory)
+        .await
 }
 
 pub async fn data_extract(
@@ -104,10 +109,11 @@ pub async fn data_extract(
     path: Vec<u8>,
     format: Option<FileRestoreFormat>,
     zstd: bool,
+    dynamic_memory: bool,
 ) -> Result<Box<dyn tokio::io::AsyncRead + Send + Unpin>, Error> {
     let driver = driver.unwrap_or(DEFAULT_DRIVER).resolve();
     driver
-        .data_extract(details, img_file, path, format, zstd)
+        .data_extract(details, img_file, path, format, zstd, dynamic_memory)
         .await
 }
 
diff --git a/proxmox-file-restore/src/block_driver_qemu.rs b/proxmox-file-restore/src/block_driver_qemu.rs
index c2cd8d49..ef0e88a5 100644
--- a/proxmox-file-restore/src/block_driver_qemu.rs
+++ b/proxmox-file-restore/src/block_driver_qemu.rs
@@ -218,13 +218,14 @@ impl BlockRestoreDriver for QemuBlockDriver {
         details: SnapRestoreDetails,
         img_file: String,
         mut path: Vec<u8>,
+        dynamic_memory: bool,
     ) -> Async<Result<Vec<ArchiveEntry>, Error>> {
         async move {
             let (cid, client) = ensure_running(&details).await?;
             if !path.is_empty() && path[0] != b'/' {
                 path.insert(0, b'/');
             }
-            if path_is_zfs(&path) {
+            if path_is_zfs(&path) && dynamic_memory {
                 if let Err(err) = set_dynamic_memory(cid, None).await {
                     log::error!("could not increase memory: {err}");
                 }
@@ -245,13 +246,14 @@ impl BlockRestoreDriver for QemuBlockDriver {
         mut path: Vec<u8>,
         format: Option<FileRestoreFormat>,
         zstd: bool,
+        dynamic_memory: bool,
     ) -> Async<Result<Box<dyn tokio::io::AsyncRead + Unpin + Send>, Error>> {
         async move {
             let (cid, client) = ensure_running(&details).await?;
             if !path.is_empty() && path[0] != b'/' {
                 path.insert(0, b'/');
             }
-            if path_is_zfs(&path) {
+            if path_is_zfs(&path) && dynamic_memory {
                 if let Err(err) = set_dynamic_memory(cid, None).await {
                     log::error!("could not increase memory: {err}");
                 }
diff --git a/proxmox-file-restore/src/main.rs b/proxmox-file-restore/src/main.rs
index 105dc079..4af49f52 100644
--- a/proxmox-file-restore/src/main.rs
+++ b/proxmox-file-restore/src/main.rs
@@ -96,6 +96,7 @@ fn keyfile_path(param: &Value) -> Option<String> {
     None
 }
 
+#[allow(clippy::too_many_arguments)]
 async fn list_files(
     repo: BackupRepository,
     namespace: BackupNamespace,
@@ -104,6 +105,7 @@ async fn list_files(
     crypt_config: Option<Arc<CryptConfig>>,
     keyfile: Option<String>,
     driver: Option<BlockDriverType>,
+    dynamic_memory: bool,
 ) -> Result<Vec<ArchiveEntry>, Error> {
     let client = connect(&repo)?;
     let client = BackupReader::start(
@@ -170,7 +172,7 @@ async fn list_files(
                 snapshot,
                 keyfile,
             };
-            data_list(driver, details, file, path).await
+            data_list(driver, details, file, path, dynamic_memory).await
         }
     }
 }
@@ -226,6 +228,12 @@ async fn list_files(
                 minimum: 1,
                 optional: true,
             },
+            "dynamic-memory": {
+                type: Boolean,
+                description: "If enabled, automatically increases memory for started vms in case of accessing a zpool inside.",
+                default: false,
+                optional: true,
+            },
         }
     },
     returns: {
@@ -243,6 +251,7 @@ async fn list(
     path: String,
     base64: bool,
     timeout: Option<u64>,
+    dynamic_memory: bool,
     param: Value,
 ) -> Result<(), Error> {
     let repo = extract_repository_from_value(&param)?;
@@ -272,7 +281,16 @@ async fn list(
     let result = if let Some(timeout) = timeout {
         match tokio::time::timeout(
             std::time::Duration::from_secs(timeout),
-            list_files(repo, ns, snapshot, path, crypt_config, keyfile, driver),
+            list_files(
+                repo,
+                ns,
+                snapshot,
+                path,
+                crypt_config,
+                keyfile,
+                driver,
+                dynamic_memory,
+            ),
         )
         .await
         {
@@ -280,7 +298,17 @@ async fn list(
             Err(_) => Err(http_err!(SERVICE_UNAVAILABLE, "list not finished in time")),
         }
     } else {
-        list_files(repo, ns, snapshot, path, crypt_config, keyfile, driver).await
+        list_files(
+            repo,
+            ns,
+            snapshot,
+            path,
+            crypt_config,
+            keyfile,
+            driver,
+            dynamic_memory,
+        )
+        .await
     };
 
     let output_format = get_output_format(&param);
@@ -387,6 +415,12 @@ async fn list(
                 type: BlockDriverType,
                 optional: true,
             },
+            "dynamic-memory": {
+                type: Boolean,
+                description: "If enabled, automatically increases memory for started vms in case of accessing a zpool inside.",
+                default: false,
+                optional: true,
+            },
         }
     }
 )]
@@ -400,6 +434,7 @@ async fn extract(
     target: Option<String>,
     format: Option<FileRestoreFormat>,
     zstd: bool,
+    dynamic_memory: bool,
     param: Value,
 ) -> Result<(), Error> {
     let repo = extract_repository_from_value(&param)?;
@@ -481,6 +516,7 @@ async fn extract(
                     path.clone(),
                     Some(FileRestoreFormat::Pxar),
                     false,
+                    dynamic_memory,
                 )
                 .await?;
                 let decoder = Decoder::from_tokio(reader).await?;
@@ -493,8 +529,16 @@ async fn extract(
                     format_err!("unable to remove temporary .pxarexclude-cli file - {}", e)
                 })?;
             } else {
-                let mut reader =
-                    data_extract(driver, details, file, path.clone(), format, zstd).await?;
+                let mut reader = data_extract(
+                    driver,
+                    details,
+                    file,
+                    path.clone(),
+                    format,
+                    zstd,
+                    dynamic_memory,
+                )
+                .await?;
                 tokio::io::copy(&mut reader, &mut tokio::io::stdout()).await?;
             }
         }
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v4 1/1] file-restore: make dynamic memory behaviour controllable

[Dominik Csapak]

by adding 'dynamic-memory' parameter that controls if we automatically
increase the memory of the guest vm or not

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
new in v4

not really happy with the 'clippy allow', but imho there is no obvious way to
bundle those parameters without restructuring the code much

 proxmox-file-restore/src/block_driver.rs      | 10 +++-
 proxmox-file-restore/src/block_driver_qemu.rs |  6 ++-
 proxmox-file-restore/src/main.rs              | 54 +++++++++++++++++--
 3 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/proxmox-file-restore/src/block_driver.rs b/proxmox-file-restore/src/block_driver.rs
index 01cc4e18..ad2da5fb 100644
--- a/proxmox-file-restore/src/block_driver.rs
+++ b/proxmox-file-restore/src/block_driver.rs
@@ -43,6 +43,7 @@ pub trait BlockRestoreDriver {
         details: SnapRestoreDetails,
         img_file: String,
         path: Vec<u8>,
+        dynamic_memory: bool,
     ) -> Async<Result<Vec<ArchiveEntry>, Error>>;
 
     /// pxar=true:
@@ -57,6 +58,7 @@ pub trait BlockRestoreDriver {
         path: Vec<u8>,
         format: Option<FileRestoreFormat>,
         zstd: bool,
+        dynamic_memory: bool,
     ) -> Async<Result<Box<dyn tokio::io::AsyncRead + Unpin + Send>, Error>>;
 
     /// Return status of all running/mapped images, result value is (id, extra data), where id must
@@ -92,9 +94,12 @@ pub async fn data_list(
     details: SnapRestoreDetails,
     img_file: String,
     path: Vec<u8>,
+    dynamic_memory: bool,
 ) -> Result<Vec<ArchiveEntry>, Error> {
     let driver = driver.unwrap_or(DEFAULT_DRIVER).resolve();
-    driver.data_list(details, img_file, path).await
+    driver
+        .data_list(details, img_file, path, dynamic_memory)
+        .await
 }
 
 pub async fn data_extract(
@@ -104,10 +109,11 @@ pub async fn data_extract(
     path: Vec<u8>,
     format: Option<FileRestoreFormat>,
     zstd: bool,
+    dynamic_memory: bool,
 ) -> Result<Box<dyn tokio::io::AsyncRead + Send + Unpin>, Error> {
     let driver = driver.unwrap_or(DEFAULT_DRIVER).resolve();
     driver
-        .data_extract(details, img_file, path, format, zstd)
+        .data_extract(details, img_file, path, format, zstd, dynamic_memory)
         .await
 }
 
diff --git a/proxmox-file-restore/src/block_driver_qemu.rs b/proxmox-file-restore/src/block_driver_qemu.rs
index c2cd8d49..ef0e88a5 100644
--- a/proxmox-file-restore/src/block_driver_qemu.rs
+++ b/proxmox-file-restore/src/block_driver_qemu.rs
@@ -218,13 +218,14 @@ impl BlockRestoreDriver for QemuBlockDriver {
         details: SnapRestoreDetails,
         img_file: String,
         mut path: Vec<u8>,
+        dynamic_memory: bool,
     ) -> Async<Result<Vec<ArchiveEntry>, Error>> {
         async move {
             let (cid, client) = ensure_running(&details).await?;
             if !path.is_empty() && path[0] != b'/' {
                 path.insert(0, b'/');
             }
-            if path_is_zfs(&path) {
+            if path_is_zfs(&path) && dynamic_memory {
                 if let Err(err) = set_dynamic_memory(cid, None).await {
                     log::error!("could not increase memory: {err}");
                 }
@@ -245,13 +246,14 @@ impl BlockRestoreDriver for QemuBlockDriver {
         mut path: Vec<u8>,
         format: Option<FileRestoreFormat>,
         zstd: bool,
+        dynamic_memory: bool,
     ) -> Async<Result<Box<dyn tokio::io::AsyncRead + Unpin + Send>, Error>> {
         async move {
             let (cid, client) = ensure_running(&details).await?;
             if !path.is_empty() && path[0] != b'/' {
                 path.insert(0, b'/');
             }
-            if path_is_zfs(&path) {
+            if path_is_zfs(&path) && dynamic_memory {
                 if let Err(err) = set_dynamic_memory(cid, None).await {
                     log::error!("could not increase memory: {err}");
                 }
diff --git a/proxmox-file-restore/src/main.rs b/proxmox-file-restore/src/main.rs
index 105dc079..4af49f52 100644
--- a/proxmox-file-restore/src/main.rs
+++ b/proxmox-file-restore/src/main.rs
@@ -96,6 +96,7 @@ fn keyfile_path(param: &Value) -> Option<String> {
     None
 }
 
+#[allow(clippy::too_many_arguments)]
 async fn list_files(
     repo: BackupRepository,
     namespace: BackupNamespace,
@@ -104,6 +105,7 @@ async fn list_files(
     crypt_config: Option<Arc<CryptConfig>>,
     keyfile: Option<String>,
     driver: Option<BlockDriverType>,
+    dynamic_memory: bool,
 ) -> Result<Vec<ArchiveEntry>, Error> {
     let client = connect(&repo)?;
     let client = BackupReader::start(
@@ -170,7 +172,7 @@ async fn list_files(
                 snapshot,
                 keyfile,
             };
-            data_list(driver, details, file, path).await
+            data_list(driver, details, file, path, dynamic_memory).await
         }
     }
 }
@@ -226,6 +228,12 @@ async fn list_files(
                 minimum: 1,
                 optional: true,
             },
+            "dynamic-memory": {
+                type: Boolean,
+                description: "If enabled, automatically increases memory for started vms in case of accessing a zpool inside.",
+                default: false,
+                optional: true,
+            },
         }
     },
     returns: {
@@ -243,6 +251,7 @@ async fn list(
     path: String,
     base64: bool,
     timeout: Option<u64>,
+    dynamic_memory: bool,
     param: Value,
 ) -> Result<(), Error> {
     let repo = extract_repository_from_value(&param)?;
@@ -272,7 +281,16 @@ async fn list(
     let result = if let Some(timeout) = timeout {
         match tokio::time::timeout(
             std::time::Duration::from_secs(timeout),
-            list_files(repo, ns, snapshot, path, crypt_config, keyfile, driver),
+            list_files(
+                repo,
+                ns,
+                snapshot,
+                path,
+                crypt_config,
+                keyfile,
+                driver,
+                dynamic_memory,
+            ),
         )
         .await
         {
@@ -280,7 +298,17 @@ async fn list(
             Err(_) => Err(http_err!(SERVICE_UNAVAILABLE, "list not finished in time")),
         }
     } else {
-        list_files(repo, ns, snapshot, path, crypt_config, keyfile, driver).await
+        list_files(
+            repo,
+            ns,
+            snapshot,
+            path,
+            crypt_config,
+            keyfile,
+            driver,
+            dynamic_memory,
+        )
+        .await
     };
 
     let output_format = get_output_format(&param);
@@ -387,6 +415,12 @@ async fn list(
                 type: BlockDriverType,
                 optional: true,
             },
+            "dynamic-memory": {
+                type: Boolean,
+                description: "If enabled, automatically increases memory for started vms in case of accessing a zpool inside.",
+                default: false,
+                optional: true,
+            },
         }
     }
 )]
@@ -400,6 +434,7 @@ async fn extract(
     target: Option<String>,
     format: Option<FileRestoreFormat>,
     zstd: bool,
+    dynamic_memory: bool,
     param: Value,
 ) -> Result<(), Error> {
     let repo = extract_repository_from_value(&param)?;
@@ -481,6 +516,7 @@ async fn extract(
                     path.clone(),
                     Some(FileRestoreFormat::Pxar),
                     false,
+                    dynamic_memory,
                 )
                 .await?;
                 let decoder = Decoder::from_tokio(reader).await?;
@@ -493,8 +529,16 @@ async fn extract(
                     format_err!("unable to remove temporary .pxarexclude-cli file - {}", e)
                 })?;
             } else {
-                let mut reader =
-                    data_extract(driver, details, file, path.clone(), format, zstd).await?;
+                let mut reader = data_extract(
+                    driver,
+                    details,
+                    file,
+                    path.clone(),
+                    format,
+                    zstd,
+                    dynamic_memory,
+                )
+                .await?;
                 tokio::io::copy(&mut reader, &mut tokio::io::stdout()).await?;
             }
         }
-- 
2.30.2

[pve-devel] applied: [pbs-devel] [PATCH proxmox-backup v4 1/1] file-restore: make dynamic memory behaviour controllable

[Thomas Lamprecht]

Am 10/11/2022 um 11:36 schrieb Dominik Csapak:
> by adding 'dynamic-memory' parameter that controls if we automatically
> increase the memory of the guest vm or not
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> new in v4
> 
> not really happy with the 'clippy allow', but imho there is no obvious way to
> bundle those parameters without restructuring the code much
> 
>  proxmox-file-restore/src/block_driver.rs      | 10 +++-
>  proxmox-file-restore/src/block_driver_qemu.rs |  6 ++-
>  proxmox-file-restore/src/main.rs              | 54 +++++++++++++++++--
>  3 files changed, 61 insertions(+), 9 deletions(-)
> 
>

applied, but superseeded looping thorugh a new param over quite a few function by
getting this out from the PBS_FILE_RESTORE_MEM_HOTPLUG_ALLOW  environment variable
being set to "true", thanks!

Note that I also made a few other followups to that code, e.g., introducing QMPSock
to avoid having QMP internals in a rather unrelated function, among other things.

[pbs-devel] applied: [PATCH proxmox-backup v4 1/1] file-restore: make dynamic memory behaviour controllable

[Thomas Lamprecht]

Am 10/11/2022 um 11:36 schrieb Dominik Csapak:
> by adding 'dynamic-memory' parameter that controls if we automatically
> increase the memory of the guest vm or not
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> new in v4
> 
> not really happy with the 'clippy allow', but imho there is no obvious way to
> bundle those parameters without restructuring the code much
> 
>  proxmox-file-restore/src/block_driver.rs      | 10 +++-
>  proxmox-file-restore/src/block_driver_qemu.rs |  6 ++-
>  proxmox-file-restore/src/main.rs              | 54 +++++++++++++++++--
>  3 files changed, 61 insertions(+), 9 deletions(-)
> 
>

applied, but superseeded looping thorugh a new param over quite a few function by
getting this out from the PBS_FILE_RESTORE_MEM_HOTPLUG_ALLOW  environment variable
being set to "true", thanks!

Note that I also made a few other followups to that code, e.g., introducing QMPSock
to avoid having QMP internals in a rather unrelated function, among other things.

[pve-devel] [PATCH common v4 1/2] PBSClient: file_restore_list: add extraParams and use timeout

[Dominik Csapak]

under some conditions, like when calling it in the api where we have
a 30s pveproxy limit, we want to make use of the '--timeout' parameter
of the file-restore binary, but we may want to call it in the future
where we don't want add timeout.

To achieve that, add an extendable 'extra_params' hash parameter to
'file_restore_list' and use the timeout from there.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v3:
* don't hardcode timeout, but use an extra_params hash
 src/PVE/PBSClient.pm | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index d7dd6e1..ec05a1c 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -375,14 +375,19 @@ sub status {
 };
 
 sub file_restore_list {
-    my ($self, $snapshot, $filepath, $base64) = @_;
+    my ($self, $snapshot, $filepath, $base64, $extra_params) = @_;
 
     (my $namespace, $snapshot) = split_namespaced_parameter($self, $snapshot);
+    my $cmd = [ $snapshot, $filepath, "--base64", $base64 ? 1 : 0];
+
+    if (my $timeout = $extra_params->{timeout}) {
+	push $cmd->@*, '--timeout', $timeout;
+    }
 
     return run_client_cmd(
 	$self,
 	"list",
-	[ $snapshot, $filepath, "--base64", $base64 ? 1 : 0 ],
+	$cmd,
 	0,
 	"proxmox-file-restore",
 	$namespace,
-- 
2.30.2

[pbs-devel] [PATCH common v4 1/2] PBSClient: file_restore_list: add extraParams and use timeout

[Dominik Csapak]

under some conditions, like when calling it in the api where we have
a 30s pveproxy limit, we want to make use of the '--timeout' parameter
of the file-restore binary, but we may want to call it in the future
where we don't want add timeout.

To achieve that, add an extendable 'extra_params' hash parameter to
'file_restore_list' and use the timeout from there.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v3:
* don't hardcode timeout, but use an extra_params hash
 src/PVE/PBSClient.pm | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index d7dd6e1..ec05a1c 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -375,14 +375,19 @@ sub status {
 };
 
 sub file_restore_list {
-    my ($self, $snapshot, $filepath, $base64) = @_;
+    my ($self, $snapshot, $filepath, $base64, $extra_params) = @_;
 
     (my $namespace, $snapshot) = split_namespaced_parameter($self, $snapshot);
+    my $cmd = [ $snapshot, $filepath, "--base64", $base64 ? 1 : 0];
+
+    if (my $timeout = $extra_params->{timeout}) {
+	push $cmd->@*, '--timeout', $timeout;
+    }
 
     return run_client_cmd(
 	$self,
 	"list",
-	[ $snapshot, $filepath, "--base64", $base64 ? 1 : 0 ],
+	$cmd,
 	0,
 	"proxmox-file-restore",
 	$namespace,
-- 
2.30.2

[pve-devel] applied: [PATCH common v4 1/2] PBSClient: file_restore_list: add extraParams and use timeout

[Thomas Lamprecht]

Am 10/11/2022 um 11:36 schrieb Dominik Csapak:
> under some conditions, like when calling it in the api where we have
> a 30s pveproxy limit, we want to make use of the '--timeout' parameter
> of the file-restore binary, but we may want to call it in the future
> where we don't want add timeout.
> 
> To achieve that, add an extendable 'extra_params' hash parameter to
> 'file_restore_list' and use the timeout from there.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v3:
> * don't hardcode timeout, but use an extra_params hash
>  src/PVE/PBSClient.pm | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
>

applied, thanks!

[pbs-devel] applied: [pve-devel] [PATCH common v4 1/2] PBSClient: file_restore_list: add extraParams and use timeout

[Thomas Lamprecht]

Am 10/11/2022 um 11:36 schrieb Dominik Csapak:
> under some conditions, like when calling it in the api where we have
> a 30s pveproxy limit, we want to make use of the '--timeout' parameter
> of the file-restore binary, but we may want to call it in the future
> where we don't want add timeout.
> 
> To achieve that, add an extendable 'extra_params' hash parameter to
> 'file_restore_list' and use the timeout from there.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v3:
> * don't hardcode timeout, but use an extra_params hash
>  src/PVE/PBSClient.pm | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
>

applied, thanks!

[pve-devel] [PATCH common v4 2/2] PBSClient: add optional 'dynamic-memory' parameter to file-restore commands

[Dominik Csapak]

so that we can make use of the dynamic memory feature of file-restore
(it automatically increases the memory of the file-restore vm when
we detect a zfs path)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
new in v4
depends on the proxmox-backup patch for file-restore
 src/PVE/PBSClient.pm | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index ec05a1c..13b28c1 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -383,6 +383,9 @@ sub file_restore_list {
     if (my $timeout = $extra_params->{timeout}) {
 	push $cmd->@*, '--timeout', $timeout;
     }
+    if (my $dynamic_memory = $extra_params->{'dynamic-memory'}) {
+	push $cmd->@*, '--dynamic-memory', $dynamic_memory;
+    }
 
     return run_client_cmd(
 	$self,
@@ -416,10 +419,16 @@ sub file_restore_extract_prepare {
 
 # this blocks while data is transfered, call this from a background worker
 sub file_restore_extract {
-    my ($self, $output_file, $snapshot, $filepath, $base64) = @_;
+    my ($self, $output_file, $snapshot, $filepath, $base64, $extra_params) = @_;
 
     (my $namespace, $snapshot) = split_namespaced_parameter($self, $snapshot);
 
+    my $cmd = [ $snapshot, $filepath, "-", "--base64", $base64 ? 1 : 0 ];
+
+    if (my $dynamic_memory = $extra_params->{'dynamic-memory'}) {
+	push $cmd->@*, '--dynamic-memory', $dynamic_memory;
+    }
+
     my $ret = eval {
 	local $SIG{ALRM} = sub { die "got timeout\n" };
 	alarm(30);
@@ -433,7 +442,7 @@ sub file_restore_extract {
 	return run_raw_client_cmd(
 	    $self,
             "extract",
-	    [ $snapshot, $filepath, "-", "--base64", $base64 ? 1 : 0 ],
+	    $cmd,
 	    binary => "proxmox-file-restore",
 	    namespace => $namespace,
 	    errfunc => $errfunc,
-- 
2.30.2

[pbs-devel] [PATCH common v4 2/2] PBSClient: add optional 'dynamic-memory' parameter to file-restore commands

[Dominik Csapak]

so that we can make use of the dynamic memory feature of file-restore
(it automatically increases the memory of the file-restore vm when
we detect a zfs path)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
new in v4
depends on the proxmox-backup patch for file-restore
 src/PVE/PBSClient.pm | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index ec05a1c..13b28c1 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -383,6 +383,9 @@ sub file_restore_list {
     if (my $timeout = $extra_params->{timeout}) {
 	push $cmd->@*, '--timeout', $timeout;
     }
+    if (my $dynamic_memory = $extra_params->{'dynamic-memory'}) {
+	push $cmd->@*, '--dynamic-memory', $dynamic_memory;
+    }
 
     return run_client_cmd(
 	$self,
@@ -416,10 +419,16 @@ sub file_restore_extract_prepare {
 
 # this blocks while data is transfered, call this from a background worker
 sub file_restore_extract {
-    my ($self, $output_file, $snapshot, $filepath, $base64) = @_;
+    my ($self, $output_file, $snapshot, $filepath, $base64, $extra_params) = @_;
 
     (my $namespace, $snapshot) = split_namespaced_parameter($self, $snapshot);
 
+    my $cmd = [ $snapshot, $filepath, "-", "--base64", $base64 ? 1 : 0 ];
+
+    if (my $dynamic_memory = $extra_params->{'dynamic-memory'}) {
+	push $cmd->@*, '--dynamic-memory', $dynamic_memory;
+    }
+
     my $ret = eval {
 	local $SIG{ALRM} = sub { die "got timeout\n" };
 	alarm(30);
@@ -433,7 +442,7 @@ sub file_restore_extract {
 	return run_raw_client_cmd(
 	    $self,
             "extract",
-	    [ $snapshot, $filepath, "-", "--base64", $base64 ? 1 : 0 ],
+	    $cmd,
 	    binary => "proxmox-file-restore",
 	    namespace => $namespace,
 	    errfunc => $errfunc,
-- 
2.30.2

[pve-devel] [PATCH storage v4 1/3] api: FileRestore: decode and return proper error of file-restore listing

[Dominik Csapak]

since commit
ba690c40 ("file-restore: remove 'json-error' parameter from list_files")

in proxmox-backup, the file-restore binary will return the error as json
when called with '--output-format json' (which we do in PVE::PBSClient)

here, we assume that 'file-restore' will fail in that case, and we try
to use the return value as an array ref which fails, and the user never
sees the real error message.

To fix that, check the ref type of the return value and act accordingly

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
no changes from v3
does not depend on rest of series
fixes error output

 PVE/API2/Storage/FileRestore.pm | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/Storage/FileRestore.pm b/PVE/API2/Storage/FileRestore.pm
index ccc56e5..4033136 100644
--- a/PVE/API2/Storage/FileRestore.pm
+++ b/PVE/API2/Storage/FileRestore.pm
@@ -121,13 +121,24 @@ __PACKAGE__->register_method ({
 	my $client = PVE::PBSClient->new($scfg, $storeid);
 	my $ret = $client->file_restore_list($snap, $path, $base64);
 
-	# 'leaf' is a proper JSON boolean, map to perl-y bool
-	# TODO: make PBSClient decode all bools always as 1/0?
-	foreach my $item (@$ret) {
-	    $item->{leaf} = $item->{leaf} ? 1 : 0;
+	if (ref($ret) eq "HASH") {
+	    my $msg = $ret->{message};
+	    if (my $code = $ret->{code}) {
+		die PVE::Exception->new("$msg\n", code => $code);
+	    } else {
+		die "$msg\n";
+	    }
+	} elsif (ref($ret) eq "ARRAY") {
+	    # 'leaf' is a proper JSON boolean, map to perl-y bool
+	    # TODO: make PBSClient decode all bools always as 1/0?
+	    foreach my $item (@$ret) {
+		$item->{leaf} = $item->{leaf} ? 1 : 0;
+	    }
+
+	    return $ret;
 	}
 
-	return $ret;
+	die "invalid proxmox-file-restore output";
     }});
 
 __PACKAGE__->register_method ({
-- 
2.30.2

[pbs-devel] [PATCH storage v4 1/3] api: FileRestore: decode and return proper error of file-restore listing

[Dominik Csapak]

since commit
ba690c40 ("file-restore: remove 'json-error' parameter from list_files")

in proxmox-backup, the file-restore binary will return the error as json
when called with '--output-format json' (which we do in PVE::PBSClient)

here, we assume that 'file-restore' will fail in that case, and we try
to use the return value as an array ref which fails, and the user never
sees the real error message.

To fix that, check the ref type of the return value and act accordingly

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
no changes from v3
does not depend on rest of series
fixes error output

 PVE/API2/Storage/FileRestore.pm | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/Storage/FileRestore.pm b/PVE/API2/Storage/FileRestore.pm
index ccc56e5..4033136 100644
--- a/PVE/API2/Storage/FileRestore.pm
+++ b/PVE/API2/Storage/FileRestore.pm
@@ -121,13 +121,24 @@ __PACKAGE__->register_method ({
 	my $client = PVE::PBSClient->new($scfg, $storeid);
 	my $ret = $client->file_restore_list($snap, $path, $base64);
 
-	# 'leaf' is a proper JSON boolean, map to perl-y bool
-	# TODO: make PBSClient decode all bools always as 1/0?
-	foreach my $item (@$ret) {
-	    $item->{leaf} = $item->{leaf} ? 1 : 0;
+	if (ref($ret) eq "HASH") {
+	    my $msg = $ret->{message};
+	    if (my $code = $ret->{code}) {
+		die PVE::Exception->new("$msg\n", code => $code);
+	    } else {
+		die "$msg\n";
+	    }
+	} elsif (ref($ret) eq "ARRAY") {
+	    # 'leaf' is a proper JSON boolean, map to perl-y bool
+	    # TODO: make PBSClient decode all bools always as 1/0?
+	    foreach my $item (@$ret) {
+		$item->{leaf} = $item->{leaf} ? 1 : 0;
+	    }
+
+	    return $ret;
 	}
 
-	return $ret;
+	die "invalid proxmox-file-restore output";
     }});
 
 __PACKAGE__->register_method ({
-- 
2.30.2

[pve-devel] applied: [PATCH storage v4 1/3] api: FileRestore: decode and return proper error of file-restore listing

[Thomas Lamprecht]

Am 10/11/2022 um 11:36 schrieb Dominik Csapak:
> since commit
> ba690c40 ("file-restore: remove 'json-error' parameter from list_files")
> 
> in proxmox-backup, the file-restore binary will return the error as json
> when called with '--output-format json' (which we do in PVE::PBSClient)
> 
> here, we assume that 'file-restore' will fail in that case, and we try
> to use the return value as an array ref which fails, and the user never
> sees the real error message.
> 
> To fix that, check the ref type of the return value and act accordingly
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> no changes from v3
> does not depend on rest of series
> fixes error output
> 
>  PVE/API2/Storage/FileRestore.pm | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
>

applied, thanks!

[pbs-devel] applied: [pve-devel] [PATCH storage v4 1/3] api: FileRestore: decode and return proper error of file-restore listing

[Thomas Lamprecht]

Am 10/11/2022 um 11:36 schrieb Dominik Csapak:
> since commit
> ba690c40 ("file-restore: remove 'json-error' parameter from list_files")
> 
> in proxmox-backup, the file-restore binary will return the error as json
> when called with '--output-format json' (which we do in PVE::PBSClient)
> 
> here, we assume that 'file-restore' will fail in that case, and we try
> to use the return value as an array ref which fails, and the user never
> sees the real error message.
> 
> To fix that, check the ref type of the return value and act accordingly
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> no changes from v3
> does not depend on rest of series
> fixes error output
> 
>  PVE/API2/Storage/FileRestore.pm | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
>

applied, thanks!

[pve-devel] [PATCH storage v4 2/3] api: FileRestore: make use of file-restores and guis timeout mechanism

[Dominik Csapak]

file-restore has a 'timeout' parameter and if that is exceeded, returns
an error with the http code 503 Service Unavailable

When the web ui encounters such an error, it retries the listing a few
times before giving up.

To make use of these, add the 'timeout' parameter to the new
'extraParams' of 'file_restore_list'.

25 seconds are chosen because it's under pveproxy 30s limit, with a bit
of overhead to spare for the rest of the api call, like json decoding,
forking, access control checks, etc.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
new in v4
depends on pve-common 1/2

 PVE/API2/Storage/FileRestore.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/API2/Storage/FileRestore.pm b/PVE/API2/Storage/FileRestore.pm
index 4033136..764ebfb 100644
--- a/PVE/API2/Storage/FileRestore.pm
+++ b/PVE/API2/Storage/FileRestore.pm
@@ -119,7 +119,7 @@ __PACKAGE__->register_method ({
 	my (undef, $snap) = PVE::Storage::parse_volname($cfg, $volid);
 
 	my $client = PVE::PBSClient->new($scfg, $storeid);
-	my $ret = $client->file_restore_list($snap, $path, $base64);
+	my $ret = $client->file_restore_list($snap, $path, $base64, { timeout => 25 });
 
 	if (ref($ret) eq "HASH") {
 	    my $msg = $ret->{message};
-- 
2.30.2

[pbs-devel] [PATCH storage v4 2/3] api: FileRestore: make use of file-restores and guis timeout mechanism

[Dominik Csapak]

file-restore has a 'timeout' parameter and if that is exceeded, returns
an error with the http code 503 Service Unavailable

When the web ui encounters such an error, it retries the listing a few
times before giving up.

To make use of these, add the 'timeout' parameter to the new
'extraParams' of 'file_restore_list'.

25 seconds are chosen because it's under pveproxy 30s limit, with a bit
of overhead to spare for the rest of the api call, like json decoding,
forking, access control checks, etc.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
new in v4
depends on pve-common 1/2

 PVE/API2/Storage/FileRestore.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/API2/Storage/FileRestore.pm b/PVE/API2/Storage/FileRestore.pm
index 4033136..764ebfb 100644
--- a/PVE/API2/Storage/FileRestore.pm
+++ b/PVE/API2/Storage/FileRestore.pm
@@ -119,7 +119,7 @@ __PACKAGE__->register_method ({
 	my (undef, $snap) = PVE::Storage::parse_volname($cfg, $volid);
 
 	my $client = PVE::PBSClient->new($scfg, $storeid);
-	my $ret = $client->file_restore_list($snap, $path, $base64);
+	my $ret = $client->file_restore_list($snap, $path, $base64, { timeout => 25 });
 
 	if (ref($ret) eq "HASH") {
 	    my $msg = $ret->{message};
-- 
2.30.2

[pve-devel] applied: [PATCH storage v4 2/3] api: FileRestore: make use of file-restores and guis timeout mechanism

[Thomas Lamprecht]

Am 10/11/2022 um 11:36 schrieb Dominik Csapak:
> file-restore has a 'timeout' parameter and if that is exceeded, returns
> an error with the http code 503 Service Unavailable
> 
> When the web ui encounters such an error, it retries the listing a few
> times before giving up.
> 
> To make use of these, add the 'timeout' parameter to the new
> 'extraParams' of 'file_restore_list'.
> 
> 25 seconds are chosen because it's under pveproxy 30s limit, with a bit
> of overhead to spare for the rest of the api call, like json decoding,
> forking, access control checks, etc.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> new in v4
> depends on pve-common 1/2

in a soft way though (no "compile" breakage if not set)

> 
>  PVE/API2/Storage/FileRestore.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!

[pbs-devel] applied: [pve-devel] [PATCH storage v4 2/3] api: FileRestore: make use of file-restores and guis timeout mechanism

[Thomas Lamprecht]

Am 10/11/2022 um 11:36 schrieb Dominik Csapak:
> file-restore has a 'timeout' parameter and if that is exceeded, returns
> an error with the http code 503 Service Unavailable
> 
> When the web ui encounters such an error, it retries the listing a few
> times before giving up.
> 
> To make use of these, add the 'timeout' parameter to the new
> 'extraParams' of 'file_restore_list'.
> 
> 25 seconds are chosen because it's under pveproxy 30s limit, with a bit
> of overhead to spare for the rest of the api call, like json decoding,
> forking, access control checks, etc.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> new in v4
> depends on pve-common 1/2

in a soft way though (no "compile" breakage if not set)

> 
>  PVE/API2/Storage/FileRestore.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!

[pve-devel] [PATCH storage v4 3/3] api: FileRestore: allow automatic memory increase for privileged users

[Dominik Csapak]

if the user has the appropriate rights (details in the comments of
'check_allow_dynamic_memory') enable the dynamic memory behaviour
of the file-restore binary

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
new in v4:
depends on pve-common 2/2

 PVE/API2/Storage/FileRestore.pm | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Storage/FileRestore.pm b/PVE/API2/Storage/FileRestore.pm
index 764ebfb..4b50eed 100644
--- a/PVE/API2/Storage/FileRestore.pm
+++ b/PVE/API2/Storage/FileRestore.pm
@@ -33,6 +33,19 @@ my $parse_volname_or_id = sub {
     return $volid;
 };
 
+# returns one if file-restore vms memory increase should be allowed
+# user needs either
+# 'VM.Allocate' on '/vms'  (can create any number of vms)
+# or 'Sys.Modify' on '/' (can modify the system to a state where it no longer functions)
+my sub check_allow_dynamic_memory {
+    my ($rpcenv, $user) = @_;
+
+    return 1 if $rpcenv->check($user, '/vms', ['VM.Allocate'], 1);
+    return 1 if $rpcenv->check($user, '/', ['Sys.Modify'], 1);
+
+    return 0;
+}
+
 __PACKAGE__->register_method ({
     name => 'list',
     path => 'list',
@@ -119,7 +132,11 @@ __PACKAGE__->register_method ({
 	my (undef, $snap) = PVE::Storage::parse_volname($cfg, $volid);
 
 	my $client = PVE::PBSClient->new($scfg, $storeid);
-	my $ret = $client->file_restore_list($snap, $path, $base64, { timeout => 25 });
+	my $extract_params = {
+	    timeout => 25,
+	    'dynamic-memory' => check_allow_dynamic_memory($rpcenv, $user),
+	};
+	my $ret = $client->file_restore_list($snap, $path, $base64, $extract_params);
 
 	if (ref($ret) eq "HASH") {
 	    my $msg = $ret->{message};
@@ -196,10 +213,14 @@ __PACKAGE__->register_method ({
 	my $client = PVE::PBSClient->new($scfg, $storeid);
 	my $fifo = $client->file_restore_extract_prepare();
 
+	my $extra_params = {
+	    'dynamic-memory' => check_allow_dynamic_memory($rpcenv, $user),
+	};
+
 	$rpcenv->fork_worker('pbs-download', undef, $user, sub {
 	    my $name = decode_base64($path);
 	    print "Starting download of file: $name\n";
-	    $client->file_restore_extract($fifo, $snap, $path, 1);
+	    $client->file_restore_extract($fifo, $snap, $path, 1, $extra_params);
 	});
 
 	my $ret = {
-- 
2.30.2

[pbs-devel] [PATCH storage v4 3/3] api: FileRestore: allow automatic memory increase for privileged users

[Dominik Csapak]

if the user has the appropriate rights (details in the comments of
'check_allow_dynamic_memory') enable the dynamic memory behaviour
of the file-restore binary

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
new in v4:
depends on pve-common 2/2

 PVE/API2/Storage/FileRestore.pm | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Storage/FileRestore.pm b/PVE/API2/Storage/FileRestore.pm
index 764ebfb..4b50eed 100644
--- a/PVE/API2/Storage/FileRestore.pm
+++ b/PVE/API2/Storage/FileRestore.pm
@@ -33,6 +33,19 @@ my $parse_volname_or_id = sub {
     return $volid;
 };
 
+# returns one if file-restore vms memory increase should be allowed
+# user needs either
+# 'VM.Allocate' on '/vms'  (can create any number of vms)
+# or 'Sys.Modify' on '/' (can modify the system to a state where it no longer functions)
+my sub check_allow_dynamic_memory {
+    my ($rpcenv, $user) = @_;
+
+    return 1 if $rpcenv->check($user, '/vms', ['VM.Allocate'], 1);
+    return 1 if $rpcenv->check($user, '/', ['Sys.Modify'], 1);
+
+    return 0;
+}
+
 __PACKAGE__->register_method ({
     name => 'list',
     path => 'list',
@@ -119,7 +132,11 @@ __PACKAGE__->register_method ({
 	my (undef, $snap) = PVE::Storage::parse_volname($cfg, $volid);
 
 	my $client = PVE::PBSClient->new($scfg, $storeid);
-	my $ret = $client->file_restore_list($snap, $path, $base64, { timeout => 25 });
+	my $extract_params = {
+	    timeout => 25,
+	    'dynamic-memory' => check_allow_dynamic_memory($rpcenv, $user),
+	};
+	my $ret = $client->file_restore_list($snap, $path, $base64, $extract_params);
 
 	if (ref($ret) eq "HASH") {
 	    my $msg = $ret->{message};
@@ -196,10 +213,14 @@ __PACKAGE__->register_method ({
 	my $client = PVE::PBSClient->new($scfg, $storeid);
 	my $fifo = $client->file_restore_extract_prepare();
 
+	my $extra_params = {
+	    'dynamic-memory' => check_allow_dynamic_memory($rpcenv, $user),
+	};
+
 	$rpcenv->fork_worker('pbs-download', undef, $user, sub {
 	    my $name = decode_base64($path);
 	    print "Starting download of file: $name\n";
-	    $client->file_restore_extract($fifo, $snap, $path, 1);
+	    $client->file_restore_extract($fifo, $snap, $path, 1, $extra_params);
 	});
 
 	my $ret = {
-- 
2.30.2