[PATCH proxmox{-backup,} 0/3] partially fix #6933: add option to specify zfs recordsize

[PATCH proxmox{-backup,} 0/3] partially fix #6933: add option to specify zfs recordsize

[Shan Shaji]

The option to specify the `recordsize` was previously available neither
in the UI nor in the CLI. Users had to configure it manually using the
ZFS CLI. This series adds an option for specifying the `recordsize`
through both the UI and `proxmox-backup-manager` when creating storage.

Since adding support for editing this property at the dataset level
would require more changes, I am sending the creation related
changes first. Support for editing the property will be send
as a separate series.

I am also trying to benchmark with different `recordsize` and `volblocksize`
values on my host with an NVMe SSD. However, testing with multiple values
(1m,2m adnd 4m) are taking longer, and I am bit unsure how meaningful the
results would be because the test environment uses ZFS on top
of ZFS. Therefore, I am sending this series for an initial review.
The changes themselves are fairly straightforward.

References:
- https://openzfs.github.io/openzfs-docs/Performance%20and%20Tuning/Workload%20Tuning.html#dataset-recordsize

proxmox-backup:

Shan Shaji (2):
  partially fix #6933: ui: storage: add option to specify zfs recordsize
  partially fix #6933: bin: add option to specify ZFS recordsize

 src/api2/node/disks/zfs.rs             | 14 +++++++++++--
 src/bin/proxmox_backup_manager/disk.rs |  8 ++++++--
 www/Utils.js                           | 28 ++++++++++++++++++++++++++
 www/window/ZFSCreate.js                |  8 ++++++++
 4 files changed, 54 insertions(+), 4 deletions(-)


proxmox:

Shan Shaji (1):
  pbs-api-types: zfs: define schema for recordsize

 pbs-api-types/src/zfs.rs | 5 +++++
 1 file changed, 5 insertions(+)


Summary over all repositories:
  5 files changed, 59 insertions(+), 4 deletions(-)

-- 
Generated by murpp 0.10.0

[PATCH proxmox-backup 1/3] partially fix #6933: ui: storage: add option to specify zfs recordsize

[Shan Shaji]

Earlier, to change the recordsize of an zfs storage dataset the users
had to use the `zfs` cli. Improved this by adding an option to specify
the recordsize while creating the storage. By default the recordsize is
set at 128KB.

Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
---
 src/api2/node/disks/zfs.rs | 14 ++++++++++++--
 www/Utils.js               | 28 ++++++++++++++++++++++++++++
 www/window/ZFSCreate.js    |  8 ++++++++
 3 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/src/api2/node/disks/zfs.rs b/src/api2/node/disks/zfs.rs
index 7a62e3f70..43d0ce259 100644
--- a/src/api2/node/disks/zfs.rs
+++ b/src/api2/node/disks/zfs.rs
@@ -7,8 +7,8 @@ use proxmox_schema::api;
 
 use pbs_api_types::{
     DATASTORE_SCHEMA, DISK_ARRAY_SCHEMA, DISK_LIST_SCHEMA, DataStoreConfig, NODE_SCHEMA,
-    PRIV_SYS_AUDIT, PRIV_SYS_MODIFY, UPID_SCHEMA, ZFS_ASHIFT_SCHEMA, ZPOOL_NAME_SCHEMA,
-    ZfsCompressionType, ZfsRaidLevel, ZpoolListItem,
+    PRIV_SYS_AUDIT, PRIV_SYS_MODIFY, UPID_SCHEMA, ZFS_ASHIFT_SCHEMA, ZFS_RECORD_SIZE_SCHEMA,
+    ZPOOL_NAME_SCHEMA, ZfsCompressionType, ZfsRaidLevel, ZpoolListItem,
 };
 
 use crate::tools::disks::{
@@ -138,6 +138,10 @@ pub fn zpool_details(name: String) -> Result<Value, Error> {
                 type: bool,
                 optional: true,
             },
+            recordsize: {
+                schema: ZFS_RECORD_SIZE_SCHEMA,
+                optional: true,
+            }
         },
     },
     returns: {
@@ -155,6 +159,7 @@ pub fn create_zpool(
     compression: Option<String>,
     ashift: Option<usize>,
     add_datastore: Option<bool>,
+    recordsize: Option<String>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<String, Error> {
     let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
@@ -292,6 +297,11 @@ pub fn create_zpool(
             if let Some(compression) = compression {
                 command.arg(format!("compression={compression}"));
             }
+
+            if let Some(recordsize) = recordsize {
+                command.arg(format!("recordsize={recordsize}"));
+            }
+
             command.args(["relatime=on", &name]);
             info!("# {command:?}");
             match proxmox_sys::command::run_command(command, None) {
diff --git a/www/Utils.js b/www/Utils.js
index d6bfd459e..1c96bd0b3 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -395,6 +395,34 @@ Ext.define('PBS.Utils', {
         return cls;
     },
 
+    validateZfsRecordSize: function (value) {
+        if (!value) {
+            return true;
+        }
+
+        let match = value.match(/^([1-9][0-9]*)([km])?$/i);
+        if (!match) {
+            return gettext(
+                'Invalid format. Use numbers with optional k or m suffix (e.g., 16k).',
+            );
+        }
+
+        let bytes = parseInt(match[1], 10);
+        let suffix = match[2]?.toLowerCase();
+
+        if (suffix === 'k') {
+            bytes *= 1024;
+        } else if (suffix === 'm') {
+            bytes *= 1024 * 1024;
+        }
+
+        if (bytes < 512 || (bytes & (bytes - 1)) !== 0 || bytes > 16 * 1024 * 1024) {
+            return gettext('Value must be a power of 2 between 512 and 16m');
+        }
+
+        return true;
+    },
+
     constructor: function () {
         var me = this;
 
diff --git a/www/window/ZFSCreate.js b/www/window/ZFSCreate.js
index 9a32bd5f3..7dcffc1b8 100644
--- a/www/window/ZFSCreate.js
+++ b/www/window/ZFSCreate.js
@@ -72,6 +72,14 @@ Ext.define('PBS.window.CreateZFS', {
                     value: '12',
                     name: 'ashift',
                 },
+                {
+                    xtype: 'proxmoxtextfield',
+                    name: 'recordsize',
+                    fieldLabel: gettext('Record Size'),
+                    allowBlank: true,
+                    validator: PBS.Utils.validateZfsRecordSize,
+                    emptyText: '128k'
+                },
             ],
             columnB: [
                 {
-- 
2.47.3

[PATCH proxmox-backup 2/3] partially fix #6933: bin: add option to specify ZFS recordsize

[Shan Shaji]

Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
---
 src/bin/proxmox_backup_manager/disk.rs | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/bin/proxmox_backup_manager/disk.rs b/src/bin/proxmox_backup_manager/disk.rs
index 258c5dfbd..91f738783 100644
--- a/src/bin/proxmox_backup_manager/disk.rs
+++ b/src/bin/proxmox_backup_manager/disk.rs
@@ -6,8 +6,7 @@ use proxmox_schema::api;
 use std::io::IsTerminal;
 
 use pbs_api_types::{
-    BLOCKDEVICE_DISK_AND_PARTITION_NAME_SCHEMA, BLOCKDEVICE_NAME_SCHEMA, DATASTORE_SCHEMA,
-    DISK_LIST_SCHEMA, ZFS_ASHIFT_SCHEMA, ZfsCompressionType, ZfsRaidLevel,
+    BLOCKDEVICE_DISK_AND_PARTITION_NAME_SCHEMA, BLOCKDEVICE_NAME_SCHEMA, DATASTORE_SCHEMA, DISK_LIST_SCHEMA, ZFS_ASHIFT_SCHEMA, ZFS_RECORD_SIZE_SCHEMA, ZfsCompressionType, ZfsRaidLevel
 };
 use proxmox_backup::tools::disks::{
     FileSystemType, SmartAttribute, complete_disk_name, complete_partition_name,
@@ -204,6 +203,11 @@ async fn wipe_disk(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<
                 type: bool,
                 optional: true,
             },
+            recordsize: {
+                schema: ZFS_RECORD_SIZE_SCHEMA,
+                optional: true,
+            }
+
        },
    },
 )]
-- 
2.47.3

[PATCH proxmox 3/3] pbs-api-types: zfs: define schema for recordsize

[Shan Shaji]

Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
---
 pbs-api-types/src/zfs.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/pbs-api-types/src/zfs.rs b/pbs-api-types/src/zfs.rs
index 2cfbd403..3cfb9f8f 100644
--- a/pbs-api-types/src/zfs.rs
+++ b/pbs-api-types/src/zfs.rs
@@ -6,6 +6,7 @@ use proxmox_schema::*;
 
 const_regex! {
     pub ZPOOL_NAME_REGEX = r"^[a-zA-Z][a-z0-9A-Z\-_.:]+$";
+    ZFS_RECORD_SIZE_REGEX = r"^(?i)([1-9][0-9]*)([km])?$";
 }
 
 pub const ZFS_ASHIFT_SCHEMA: Schema = IntegerSchema::new("Pool sector size exponent.")
@@ -18,6 +19,10 @@ pub const ZPOOL_NAME_SCHEMA: Schema = StringSchema::new("ZFS Pool Name")
     .format(&ApiStringFormat::Pattern(&ZPOOL_NAME_REGEX))
     .schema();
 
+pub const ZFS_RECORD_SIZE_SCHEMA: Schema = StringSchema::new("ZFS Recordsize. Use numbers with optional k or m suffix (e.g., 128k).")
+    .format(&ApiStringFormat::Pattern(&ZFS_RECORD_SIZE_REGEX))
+    .schema();
+
 #[api(default: "On")]
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "lowercase")]
-- 
2.47.3