all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: pve-devel@lists.proxmox.com, pbs-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH proxmox-backup v4 1/1] file-restore: make dynamic memory behaviour controllable
Date: Thu, 10 Nov 2022 11:36:29 +0100	[thread overview]
Message-ID: <20221110103634.2177856-2-d.csapak@proxmox.com> (raw)
In-Reply-To: <20221110103634.2177856-1-d.csapak@proxmox.com>

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





WARNING: multiple messages have this Message-ID
From: Dominik Csapak <d.csapak@proxmox.com>
To: pve-devel@lists.proxmox.com, pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup v4 1/1] file-restore: make dynamic memory behaviour controllable
Date: Thu, 10 Nov 2022 11:36:29 +0100	[thread overview]
Message-ID: <20221110103634.2177856-2-d.csapak@proxmox.com> (raw)
In-Reply-To: <20221110103634.2177856-1-d.csapak@proxmox.com>

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





  reply	other threads:[~2022-11-10 10:37 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-10 10:36 [pve-devel] [PATCH proxmox-backup/common/storage v4] improve file-restore timeout behaviour Dominik Csapak
2022-11-10 10:36 ` [pbs-devel] " Dominik Csapak
2022-11-10 10:36 ` Dominik Csapak [this message]
2022-11-10 10:36   ` [pbs-devel] [PATCH proxmox-backup v4 1/1] file-restore: make dynamic memory behaviour controllable Dominik Csapak
2022-11-15  8:29   ` [pve-devel] applied: " Thomas Lamprecht
2022-11-15  8:29     ` [pbs-devel] applied: " Thomas Lamprecht
2022-11-10 10:36 ` [pve-devel] [PATCH common v4 1/2] PBSClient: file_restore_list: add extraParams and use timeout Dominik Csapak
2022-11-10 10:36   ` [pbs-devel] " Dominik Csapak
2022-11-15 12:27   ` [pve-devel] applied: " Thomas Lamprecht
2022-11-15 12:27     ` [pbs-devel] applied: [pve-devel] " Thomas Lamprecht
2022-11-10 10:36 ` [pve-devel] [PATCH common v4 2/2] PBSClient: add optional 'dynamic-memory' parameter to file-restore commands Dominik Csapak
2022-11-10 10:36   ` [pbs-devel] " Dominik Csapak
2022-11-10 10:36 ` [pve-devel] [PATCH storage v4 1/3] api: FileRestore: decode and return proper error of file-restore listing Dominik Csapak
2022-11-10 10:36   ` [pbs-devel] " Dominik Csapak
2022-11-15 12:27   ` [pve-devel] applied: " Thomas Lamprecht
2022-11-15 12:27     ` [pbs-devel] applied: [pve-devel] " Thomas Lamprecht
2022-11-10 10:36 ` [pve-devel] [PATCH storage v4 2/3] api: FileRestore: make use of file-restores and guis timeout mechanism Dominik Csapak
2022-11-10 10:36   ` [pbs-devel] " Dominik Csapak
2022-11-15 12:28   ` [pve-devel] applied: " Thomas Lamprecht
2022-11-15 12:28     ` [pbs-devel] applied: [pve-devel] " Thomas Lamprecht
2022-11-10 10:36 ` [pve-devel] [PATCH storage v4 3/3] api: FileRestore: allow automatic memory increase for privileged users Dominik Csapak
2022-11-10 10:36   ` [pbs-devel] " Dominik Csapak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221110103634.2177856-2-d.csapak@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal