* [PATCH manager/storage 0/2] fix #3936: add validation for ZFS blocksize
@ 2026-05-12 13:48 David Riley
2026-05-12 13:48 ` [PATCH pve-manager 1/2] fix #3936: ui: storage: " David Riley
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: David Riley @ 2026-05-12 13:48 UTC (permalink / raw)
To: pve-devel; +Cc: David Riley
This patch series fixes #3936 [0] by implementing validation for the
ZFS blocksize parameter in both the backend API and the frontend UI.
Currently, providing an invalid blocksize (e.g., a non-power-of-two or
a value outside the supported range) does not result in an immediate
API error and therefore delays the failure to the volume creation.
This patch series ensures that invalid configurations are caught
early.
Validation [1][2]:
* Range: 512 bytes to 16 MiB.
* Format: Allow positive integers with optional 'k' or 'm' suffix.
* Constraint: Must be a power of two.
[0] https://bugzilla.proxmox.com/show_bug.cgi?id=3936
[1] https://openzfs.github.io/openzfs-docs/man/v2.4/7/zfsprops.7.html#volblocksize
[2] https://openzfs.github.io/openzfs-docs/man/v2.4/7/zfsprops.7.html#recordsize
pve-manager:
David Riley (1):
fix #3936: ui: storage: add validation for ZFS blocksize
www/manager6/Utils.js | 34 +++++++++++++++++++++++++++++
www/manager6/storage/ZFSEdit.js | 1 +
www/manager6/storage/ZFSPoolEdit.js | 1 +
3 files changed, 36 insertions(+)
pve-storage:
David Riley (1):
fix #3936: api: add zfs-blocksize format
src/PVE/Storage/Plugin.pm | 34 ++++++++++++++++++++++++++++++++
src/PVE/Storage/ZFSPoolPlugin.pm | 1 +
2 files changed, 35 insertions(+)
Summary over all repositories:
5 files changed, 71 insertions(+), 0 deletions(-)
--
Generated by murpp 0.11.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH pve-manager 1/2] fix #3936: ui: storage: add validation for ZFS blocksize 2026-05-12 13:48 [PATCH manager/storage 0/2] fix #3936: add validation for ZFS blocksize David Riley @ 2026-05-12 13:48 ` David Riley 2026-05-13 14:05 ` Fiona Ebner 2026-05-12 13:48 ` [PATCH pve-storage 2/2] fix #3936: api: add zfs-blocksize format David Riley 2026-05-15 8:00 ` superseded: [PATCH manager/storage 0/2] fix #3936: add validation for ZFS blocksize David Riley 2 siblings, 1 reply; 7+ messages in thread From: David Riley @ 2026-05-12 13:48 UTC (permalink / raw) To: pve-devel; +Cc: David Riley Add validation to the ZFS storage blocksize form field to prevent misconfigurations. Validation [0][1]: * Range: 512 bytes to 16 MiB * Format: Allow positive integers with optional 'k' or 'm' suffix. * Constraint: Must be a power of two. Link: https://bugzilla.proxmox.com/show_bug.cgi?id=3936 [0] https://openzfs.github.io/openzfs-docs/man/v2.4/7/zfsprops.7.html#volblocksize [1] https://openzfs.github.io/openzfs-docs/man/v2.4/7/zfsprops.7.html#recordsize Signed-off-by: David Riley <d.riley@proxmox.com> --- www/manager6/Utils.js | 34 +++++++++++++++++++++++++++++ www/manager6/storage/ZFSEdit.js | 1 + www/manager6/storage/ZFSPoolEdit.js | 1 + 3 files changed, 36 insertions(+) diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js index 01e80682..a8f684a0 100644 --- a/www/manager6/Utils.js +++ b/www/manager6/Utils.js @@ -274,6 +274,40 @@ Ext.define('PVE.Utils', { return '<i class="fa fa-' + iconCls + '"></i> ' + value; }, + validate_zfs_blocksize: function (value) { + if (!value || 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] ? match[2].toLowerCase() : ''; + + if (suffix === 'k') { + bytes *= 1024; + } else if (suffix === 'm') { + bytes *= 1024 * 1024; + } + + if (bytes < 512 || (bytes & (bytes - 1)) !== 0) { + return gettext( + 'Value must be a power of 2 and at least 512 (e.g., 4k, 8k, 16k, 32k).', + ); + } + + if (bytes > 16 * 1024 * 1024) { + return gettext('Value is too large (max 16 MiB).'); + } + + return true; + }, + render_pbs_fingerprint: (fp) => fp.substring(0, 23), render_backup_encryption: function (v, meta, record) { diff --git a/www/manager6/storage/ZFSEdit.js b/www/manager6/storage/ZFSEdit.js index e9d24642..ef18fa11 100644 --- a/www/manager6/storage/ZFSEdit.js +++ b/www/manager6/storage/ZFSEdit.js @@ -67,6 +67,7 @@ Ext.define('PVE.storage.ZFSInputPanel', { value: '4k', fieldLabel: gettext('Block Size'), allowBlank: false, + validator: PVE.Utils.validate_zfs_blocksize, }, { xtype: me.isCreate ? 'textfield' : 'displayfield', diff --git a/www/manager6/storage/ZFSPoolEdit.js b/www/manager6/storage/ZFSPoolEdit.js index 0066dfef..2d7b4498 100644 --- a/www/manager6/storage/ZFSPoolEdit.js +++ b/www/manager6/storage/ZFSPoolEdit.js @@ -106,6 +106,7 @@ Ext.define('PVE.storage.ZFSPoolInputPanel', { emptyText: '16k', fieldLabel: gettext('Block Size'), allowBlank: true, + validator: PVE.Utils.validate_zfs_blocksize, }, ], }); -- 2.47.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH pve-manager 1/2] fix #3936: ui: storage: add validation for ZFS blocksize 2026-05-12 13:48 ` [PATCH pve-manager 1/2] fix #3936: ui: storage: " David Riley @ 2026-05-13 14:05 ` Fiona Ebner 2026-05-15 6:57 ` David Riley 0 siblings, 1 reply; 7+ messages in thread From: Fiona Ebner @ 2026-05-13 14:05 UTC (permalink / raw) To: David Riley, pve-devel Am 12.05.26 um 3:47 PM schrieb David Riley: > Add validation to the ZFS storage blocksize form field to prevent > misconfigurations. > > Validation [0][1]: > * Range: 512 bytes to 16 MiB > * Format: Allow positive integers with optional 'k' or 'm' suffix. > * Constraint: Must be a power of two. > > Link: https://bugzilla.proxmox.com/show_bug.cgi?id=3936 > > [0] https://openzfs.github.io/openzfs-docs/man/v2.4/7/zfsprops.7.html#volblocksize > [1] https://openzfs.github.io/openzfs-docs/man/v2.4/7/zfsprops.7.html#recordsize > > Signed-off-by: David Riley <d.riley@proxmox.com> Some nits, but otherwise: Reviewed-by: Fiona Ebner <f.ebner@proxmox.com> > --- > www/manager6/Utils.js | 34 +++++++++++++++++++++++++++++ > www/manager6/storage/ZFSEdit.js | 1 + > www/manager6/storage/ZFSPoolEdit.js | 1 + > 3 files changed, 36 insertions(+) > > diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js > index 01e80682..a8f684a0 100644 > --- a/www/manager6/Utils.js > +++ b/www/manager6/Utils.js > @@ -274,6 +274,40 @@ Ext.define('PVE.Utils', { > return '<i class="fa fa-' + iconCls + '"></i> ' + value; > }, > > + validate_zfs_blocksize: function (value) { Style nit: new function names should be camelCase https://pve.proxmox.com/wiki/Javascript_Style_Guide#Casing > + if (!value || value === '') { Nit: the second half is redundant, because !'' is already true > + 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] ? match[2].toLowerCase() : ''; Nit: in this case, it could be: let suffix = match[2]?.toLowerCase(); > + > + if (suffix === 'k') { > + bytes *= 1024; > + } else if (suffix === 'm') { > + bytes *= 1024 * 1024; > + } > + > + if (bytes < 512 || (bytes & (bytes - 1)) !== 0) { > + return gettext( > + 'Value must be a power of 2 and at least 512 (e.g., 4k, 8k, 16k, 32k).', Not sure if it's worth splitting the check? The text could be "a power of 2 between 512 and 16m", then you'd implicitly cover the 'm' suffix as an example too. > + ); > + } > + > + if (bytes > 16 * 1024 * 1024) { > + return gettext('Value is too large (max 16 MiB).'); > + } > + > + return true; > + }, > + > render_pbs_fingerprint: (fp) => fp.substring(0, 23), > > render_backup_encryption: function (v, meta, record) { > diff --git a/www/manager6/storage/ZFSEdit.js b/www/manager6/storage/ZFSEdit.js > index e9d24642..ef18fa11 100644 > --- a/www/manager6/storage/ZFSEdit.js > +++ b/www/manager6/storage/ZFSEdit.js > @@ -67,6 +67,7 @@ Ext.define('PVE.storage.ZFSInputPanel', { > value: '4k', > fieldLabel: gettext('Block Size'), > allowBlank: false, > + validator: PVE.Utils.validate_zfs_blocksize, > }, > { > xtype: me.isCreate ? 'textfield' : 'displayfield', > diff --git a/www/manager6/storage/ZFSPoolEdit.js b/www/manager6/storage/ZFSPoolEdit.js > index 0066dfef..2d7b4498 100644 > --- a/www/manager6/storage/ZFSPoolEdit.js > +++ b/www/manager6/storage/ZFSPoolEdit.js > @@ -106,6 +106,7 @@ Ext.define('PVE.storage.ZFSPoolInputPanel', { > emptyText: '16k', > fieldLabel: gettext('Block Size'), > allowBlank: true, > + validator: PVE.Utils.validate_zfs_blocksize, > }, > ], > }); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH pve-manager 1/2] fix #3936: ui: storage: add validation for ZFS blocksize 2026-05-13 14:05 ` Fiona Ebner @ 2026-05-15 6:57 ` David Riley 0 siblings, 0 replies; 7+ messages in thread From: David Riley @ 2026-05-15 6:57 UTC (permalink / raw) To: Fiona Ebner, pve-devel Thanks for the review. I will send a v2 incorporating your feedback. On 5/13/26 4:05 PM, Fiona Ebner wrote: > Am 12.05.26 um 3:47 PM schrieb David Riley: >> Add validation to the ZFS storage blocksize form field to prevent >> misconfigurations. >> >> Validation [0][1]: >> * Range: 512 bytes to 16 MiB >> * Format: Allow positive integers with optional 'k' or 'm' suffix. >> * Constraint: Must be a power of two. >> >> Link: https://bugzilla.proxmox.com/show_bug.cgi?id=3936 >> >> [0] https://openzfs.github.io/openzfs-docs/man/v2.4/7/zfsprops.7.html#volblocksize >> [1] https://openzfs.github.io/openzfs-docs/man/v2.4/7/zfsprops.7.html#recordsize >> >> Signed-off-by: David Riley <d.riley@proxmox.com> > Some nits, but otherwise: > > Reviewed-by: Fiona Ebner <f.ebner@proxmox.com> > >> --- >> www/manager6/Utils.js | 34 +++++++++++++++++++++++++++++ >> www/manager6/storage/ZFSEdit.js | 1 + >> www/manager6/storage/ZFSPoolEdit.js | 1 + >> 3 files changed, 36 insertions(+) >> >> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js >> index 01e80682..a8f684a0 100644 >> --- a/www/manager6/Utils.js >> +++ b/www/manager6/Utils.js >> @@ -274,6 +274,40 @@ Ext.define('PVE.Utils', { >> return '<i class="fa fa-' + iconCls + '"></i> ' + value; >> }, >> >> + validate_zfs_blocksize: function (value) { > Style nit: new function names should be camelCase > > https://pve.proxmox.com/wiki/Javascript_Style_Guide#Casing ack. Will fix. > >> + if (!value || value === '') { > Nit: the second half is redundant, because !'' is already true ack. > >> + 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] ? match[2].toLowerCase() : ''; > Nit: in this case, it could be: > > let suffix = match[2]?.toLowerCase(); ack. > >> + >> + if (suffix === 'k') { >> + bytes *= 1024; >> + } else if (suffix === 'm') { >> + bytes *= 1024 * 1024; >> + } >> + >> + if (bytes < 512 || (bytes & (bytes - 1)) !== 0) { >> + return gettext( >> + 'Value must be a power of 2 and at least 512 (e.g., 4k, 8k, 16k, 32k).', > Not sure if it's worth splitting the check? The text could be "a power > of 2 between 512 and 16m", then you'd implicitly cover the 'm' suffix as > an example too. Agreed, I'll combine these into a single check for the v2. > >> + ); >> + } >> + >> + if (bytes > 16 * 1024 * 1024) { >> + return gettext('Value is too large (max 16 MiB).'); >> + } >> + >> + return true; >> + }, >> + >> render_pbs_fingerprint: (fp) => fp.substring(0, 23), >> >> render_backup_encryption: function (v, meta, record) { >> diff --git a/www/manager6/storage/ZFSEdit.js b/www/manager6/storage/ZFSEdit.js >> index e9d24642..ef18fa11 100644 >> --- a/www/manager6/storage/ZFSEdit.js >> +++ b/www/manager6/storage/ZFSEdit.js >> @@ -67,6 +67,7 @@ Ext.define('PVE.storage.ZFSInputPanel', { >> value: '4k', >> fieldLabel: gettext('Block Size'), >> allowBlank: false, >> + validator: PVE.Utils.validate_zfs_blocksize, >> }, >> { >> xtype: me.isCreate ? 'textfield' : 'displayfield', >> diff --git a/www/manager6/storage/ZFSPoolEdit.js b/www/manager6/storage/ZFSPoolEdit.js >> index 0066dfef..2d7b4498 100644 >> --- a/www/manager6/storage/ZFSPoolEdit.js >> +++ b/www/manager6/storage/ZFSPoolEdit.js >> @@ -106,6 +106,7 @@ Ext.define('PVE.storage.ZFSPoolInputPanel', { >> emptyText: '16k', >> fieldLabel: gettext('Block Size'), >> allowBlank: true, >> + validator: PVE.Utils.validate_zfs_blocksize, >> }, >> ], >> }); ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH pve-storage 2/2] fix #3936: api: add zfs-blocksize format 2026-05-12 13:48 [PATCH manager/storage 0/2] fix #3936: add validation for ZFS blocksize David Riley 2026-05-12 13:48 ` [PATCH pve-manager 1/2] fix #3936: ui: storage: " David Riley @ 2026-05-12 13:48 ` David Riley 2026-05-13 14:35 ` applied: " Fiona Ebner 2026-05-15 8:00 ` superseded: [PATCH manager/storage 0/2] fix #3936: add validation for ZFS blocksize David Riley 2 siblings, 1 reply; 7+ messages in thread From: David Riley @ 2026-05-12 13:48 UTC (permalink / raw) To: pve-devel; +Cc: David Riley Add a new JSONSchema format pve-storage-zfs-blocksize to validate the ZFS blocksize parameter. Validation [0][1]: * Range: 512 bytes to 16 MiB * Format: Allow positive integers with optional 'k' or 'm' suffix. * Constraint: Must be a power of two. Link: https://bugzilla.proxmox.com/show_bug.cgi?id=3936 [0] https://openzfs.github.io/openzfs-docs/man/v2.4/7/zfsprops.7.html#volblocksize [1] https://openzfs.github.io/openzfs-docs/man/v2.4/7/zfsprops.7.html#recordsize Signed-off-by: David Riley <d.riley@proxmox.com> --- src/PVE/Storage/Plugin.pm | 34 ++++++++++++++++++++++++++++++++ src/PVE/Storage/ZFSPoolPlugin.pm | 1 + 2 files changed, 35 insertions(+) diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index afd3141..486fab9 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -449,6 +449,40 @@ sub verify_dir_override { die "invalid override '$value'\n"; } +PVE::JSONSchema::register_format('pve-storage-zfs-blocksize', \&verify_zfs_blocksize); + +sub verify_zfs_blocksize { + my ($value, $noerr) = @_; + + return $value if !defined($value) || $value eq ''; + + if ($value =~ m/^([1-9][0-9]*)([km])?$/i) { + my ($num, $unit) = ($1, lc($2 // '')); + + if ($unit eq 'k') { + $num *= 1024; + } elsif ($unit eq 'm') { + $num *= 1024 * 1024; + } + + if ( + $num >= 512 + && $num <= 16 * 1024 * 1024 + && ($num & ($num - 1)) == 0 + ) { + return $value; + } + + return undef if $noerr; + die "value '$value' is not a valid ZFS blocksize. Must be a power of 2 " + . "between 512B and 16MiB (e.g., 4k, 8k, 16k, 64k, 1m).\n"; + } + + return undef if $noerr; + die "invalid ZFS blocksize format '$value'. Use a positive number " + . "(no leading zeros) with an optional 'k' or 'm' suffix.\n"; +} + sub private { return $defaultData; } diff --git a/src/PVE/Storage/ZFSPoolPlugin.pm b/src/PVE/Storage/ZFSPoolPlugin.pm index fcd7804..6a64401 100644 --- a/src/PVE/Storage/ZFSPoolPlugin.pm +++ b/src/PVE/Storage/ZFSPoolPlugin.pm @@ -31,6 +31,7 @@ sub properties { blocksize => { description => "block size", type => 'string', + format => 'pve-storage-zfs-blocksize', }, sparse => { description => "use sparse volumes", -- 2.47.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* applied: [PATCH pve-storage 2/2] fix #3936: api: add zfs-blocksize format 2026-05-12 13:48 ` [PATCH pve-storage 2/2] fix #3936: api: add zfs-blocksize format David Riley @ 2026-05-13 14:35 ` Fiona Ebner 0 siblings, 0 replies; 7+ messages in thread From: Fiona Ebner @ 2026-05-13 14:35 UTC (permalink / raw) To: pve-devel, David Riley On Tue, 12 May 2026 15:48:52 +0200, David Riley wrote: > Add a new JSONSchema format pve-storage-zfs-blocksize to validate > the ZFS blocksize parameter. > > Validation [0][1]: > * Range: 512 bytes to 16 MiB > * Format: Allow positive integers with optional 'k' or 'm' suffix. > * Constraint: Must be a power of two. Applied this one, thanks! Added a format_description for the blocksize property and mentioned ZFS in the description in a follow-up. Also moved the format defintion into the ZFS plugin module. [2/2] fix #3936: api: zfspool: validate blocksize property with format commit: ee66befd5008c1ab8c34c03569c96ff246af3fdb ^ permalink raw reply [flat|nested] 7+ messages in thread
* superseded: [PATCH manager/storage 0/2] fix #3936: add validation for ZFS blocksize 2026-05-12 13:48 [PATCH manager/storage 0/2] fix #3936: add validation for ZFS blocksize David Riley 2026-05-12 13:48 ` [PATCH pve-manager 1/2] fix #3936: ui: storage: " David Riley 2026-05-12 13:48 ` [PATCH pve-storage 2/2] fix #3936: api: add zfs-blocksize format David Riley @ 2026-05-15 8:00 ` David Riley 2 siblings, 0 replies; 7+ messages in thread From: David Riley @ 2026-05-15 8:00 UTC (permalink / raw) To: pve-devel superseded by: https://lore.proxmox.com/pve-devel/20260515075159.45099-1-d.riley@proxmox.com/ On 5/12/26 3:48 PM, David Riley wrote: > This patch series fixes #3936 [0] by implementing validation for the > ZFS blocksize parameter in both the backend API and the frontend UI. > > Currently, providing an invalid blocksize (e.g., a non-power-of-two or > a value outside the supported range) does not result in an immediate > API error and therefore delays the failure to the volume creation. > This patch series ensures that invalid configurations are caught > early. > > Validation [1][2]: > * Range: 512 bytes to 16 MiB. > * Format: Allow positive integers with optional 'k' or 'm' suffix. > * Constraint: Must be a power of two. > > [0] https://bugzilla.proxmox.com/show_bug.cgi?id=3936 > [1] https://openzfs.github.io/openzfs-docs/man/v2.4/7/zfsprops.7.html#volblocksize > [2] https://openzfs.github.io/openzfs-docs/man/v2.4/7/zfsprops.7.html#recordsize > > > pve-manager: > > David Riley (1): > fix #3936: ui: storage: add validation for ZFS blocksize > > www/manager6/Utils.js | 34 +++++++++++++++++++++++++++++ > www/manager6/storage/ZFSEdit.js | 1 + > www/manager6/storage/ZFSPoolEdit.js | 1 + > 3 files changed, 36 insertions(+) > > > pve-storage: > > David Riley (1): > fix #3936: api: add zfs-blocksize format > > src/PVE/Storage/Plugin.pm | 34 ++++++++++++++++++++++++++++++++ > src/PVE/Storage/ZFSPoolPlugin.pm | 1 + > 2 files changed, 35 insertions(+) > > > Summary over all repositories: > 5 files changed, 71 insertions(+), 0 deletions(-) > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-05-15 8:00 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-12 13:48 [PATCH manager/storage 0/2] fix #3936: add validation for ZFS blocksize David Riley 2026-05-12 13:48 ` [PATCH pve-manager 1/2] fix #3936: ui: storage: " David Riley 2026-05-13 14:05 ` Fiona Ebner 2026-05-15 6:57 ` David Riley 2026-05-12 13:48 ` [PATCH pve-storage 2/2] fix #3936: api: add zfs-blocksize format David Riley 2026-05-13 14:35 ` applied: " Fiona Ebner 2026-05-15 8:00 ` superseded: [PATCH manager/storage 0/2] fix #3936: add validation for ZFS blocksize David Riley
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.