public inbox for pve-devel@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





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

Thread overview: 11+ 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 ` Dominik Csapak [this message]
2022-11-15  8:29   ` [pve-devel] applied: [pbs-devel] [PATCH proxmox-backup v4 1/1] file-restore: make dynamic memory behaviour controllable 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-15 12:27   ` [pve-devel] applied: " 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 ` [pve-devel] [PATCH storage v4 1/3] api: FileRestore: decode and return proper error of file-restore listing Dominik Csapak
2022-11-15 12:27   ` [pve-devel] applied: " 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-15 12:28   ` [pve-devel] applied: " 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

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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal