[pve-devel] [PATCH-SERIES v3 storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images

[pve-devel] [PATCH-SERIES v3 storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images

[Lorenz Stechauner]

changes to v2:
* cleaned up commit message for pve-storage
* changed implementation to put the preallocation selector on the column with fewer items

changes to v1:
* adjusted preallocation api description
* moved sub preallocation_cmd_option above `# Storage implementation`
* updated PreallocationSelector to work with `default`
* reworked placement of Prealloc.Selector in Base.js


pve-storage:
Lorenz Stechauner (1):
  fix #3580: plugins: make preallocation mode selectable for qcow2 and
    raw images

 PVE/Storage/BTRFSPlugin.pm     |  1 +
 PVE/Storage/CIFSPlugin.pm      |  1 +
 PVE/Storage/DirPlugin.pm       |  1 +
 PVE/Storage/GlusterfsPlugin.pm |  4 ++-
 PVE/Storage/NFSPlugin.pm       |  1 +
 PVE/Storage/Plugin.pm          | 48 +++++++++++++++++++++++++++++++++-
 6 files changed, 54 insertions(+), 2 deletions(-)


pve-manager
Lorenz Stechauner (2):
  ui: add PreallocationSelector
  fix 3850: ui: storage: using PreallocationSelector for file based
    storage types

 www/manager6/Makefile                      |  1 +
 www/manager6/controller/StorageEdit.js     |  6 ++++++
 www/manager6/form/PreallocationSelector.js | 11 +++++++++++
 www/manager6/storage/Base.js               | 20 ++++++++++++++++++++
 www/manager6/storage/NFSEdit.js            |  2 +-
 5 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 www/manager6/form/PreallocationSelector.js

-- 
2.30.2

[pve-devel] [PATCH v3 storage 1/1] fix #3580: plugins: make preallocation mode selectable for qcow2 and raw images

[Lorenz Stechauner]

the plugins for file based storages
 * BTRFS
 * CIFS
 * Dir
 * Glusterfs
 * NFS
now allow the option 'preallocation'.

'preallocation' can have four values:
 * default
 * off
 * metadata
 * falloc
 * full
see man pages for `qemu-img` for what these mean exactly. [0]

the defualt value was chosen to be
 * qcow2: metadata (as previously)
 * raw: off

when using 'metadata' as preallocation mode, for raw images 'off'
is used.

[0] https://qemu.readthedocs.io/en/latest/system/images.html#disk-image-file-formats

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 PVE/Storage/BTRFSPlugin.pm     |  1 +
 PVE/Storage/CIFSPlugin.pm      |  1 +
 PVE/Storage/DirPlugin.pm       |  1 +
 PVE/Storage/GlusterfsPlugin.pm |  4 ++-
 PVE/Storage/NFSPlugin.pm       |  1 +
 PVE/Storage/Plugin.pm          | 48 +++++++++++++++++++++++++++++++++-
 6 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm
index 1407f44..63d307d 100644
--- a/PVE/Storage/BTRFSPlugin.pm
+++ b/PVE/Storage/BTRFSPlugin.pm
@@ -73,6 +73,7 @@ sub options {
 	is_mountpoint => { optional => 1 },
 	nocow => { optional => 1 },
 	mkdir => { optional => 1 },
+	preallocation => { optional => 1 },
 	# TODO: The new variant of mkdir with  `populate` vs `create`...
     };
 }
diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
index c5f3894..3a7e638 100644
--- a/PVE/Storage/CIFSPlugin.pm
+++ b/PVE/Storage/CIFSPlugin.pm
@@ -142,6 +142,7 @@ sub options {
 	smbversion => { optional => 1},
 	mkdir => { optional => 1 },
 	bwlimit => { optional => 1 },
+	preallocation => { optional => 1 },
     };
 }
 
diff --git a/PVE/Storage/DirPlugin.pm b/PVE/Storage/DirPlugin.pm
index 2267f11..3eeec98 100644
--- a/PVE/Storage/DirPlugin.pm
+++ b/PVE/Storage/DirPlugin.pm
@@ -59,6 +59,7 @@ sub options {
 	mkdir => { optional => 1 },
 	is_mountpoint => { optional => 1 },
 	bwlimit => { optional => 1 },
+	preallocation => { optional => 1 },
    };
 }
 
diff --git a/PVE/Storage/GlusterfsPlugin.pm b/PVE/Storage/GlusterfsPlugin.pm
index ea4df82..d8d2b88 100644
--- a/PVE/Storage/GlusterfsPlugin.pm
+++ b/PVE/Storage/GlusterfsPlugin.pm
@@ -137,6 +137,7 @@ sub options {
 	format => { optional => 1 },
 	mkdir => { optional => 1 },
 	bwlimit => { optional => 1 },
+	preallocation => { optional => 1 },
     };
 }
 
@@ -260,7 +261,8 @@ sub alloc_image {
 
     my $cmd = ['/usr/bin/qemu-img', 'create'];
 
-    push @$cmd, '-o', 'preallocation=metadata' if $fmt eq 'qcow2';
+    my $prealloc_opt = PVE::Storage::Plugin::preallocation_cmd_option($scfg, $fmt);
+    push @$cmd, '-o', $prealloc_opt if defined($prealloc_opt);
 
     push @$cmd, '-f', $fmt, $volumepath, "${size}K";
 
diff --git a/PVE/Storage/NFSPlugin.pm b/PVE/Storage/NFSPlugin.pm
index 39bf15a..21b288a 100644
--- a/PVE/Storage/NFSPlugin.pm
+++ b/PVE/Storage/NFSPlugin.pm
@@ -90,6 +90,7 @@ sub options {
 	format => { optional => 1 },
 	mkdir => { optional => 1 },
 	bwlimit => { optional => 1 },
+	preallocation => { optional => 1 },
     };
 }
 
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index fab2316..aeb4fff 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -41,6 +41,19 @@ our @SHARED_STORAGE = (
     'pbs',
 );
 
+our $QCOW2_PREALLOCATION = {
+    off => 1,
+    metadata => 1,
+    falloc => 1,
+    full => 1,
+};
+
+our $RAW_PREALLOCATION = {
+    off => 1,
+    falloc => 1,
+    full => 1,
+};
+
 our $MAX_VOLUMES_PER_GUEST = 1024;
 
 cfs_register_file ('storage.cfg',
@@ -152,6 +165,13 @@ my $defaultData = {
 	    type => 'string', format => 'pve-storage-format',
 	    optional => 1,
 	},
+	preallocation => {
+	    description => "Preallocation mode for raw and qcow2 images. " .
+		"Using 'metadata' on raw images results in preallocation=off.",
+	    type => 'string', enum => ['off', 'metadata', 'falloc', 'full'],
+	    default => 'metadata',
+	    optional => 1,
+	},
     },
 };
 
@@ -444,6 +464,31 @@ sub parse_config {
     return $cfg;
 }
 
+sub preallocation_cmd_option {
+    my ($scfg, $fmt) = @_;
+
+    my $prealloc = $scfg->{preallocation};
+
+    if ($fmt eq 'qcow2') {
+	$prealloc = $prealloc // 'metadata';
+
+	die "preallocation mode '$prealloc' not supported by format '$fmt'\n"
+	    if !$QCOW2_PREALLOCATION->{$prealloc};
+
+	return "preallocation=$prealloc";
+    } elsif ($fmt eq 'raw') {
+	$prealloc = $prealloc // 'off';
+	$prealloc = 'off' if $prealloc eq 'metadata';
+
+	die "preallocation mode '$prealloc' not supported by format '$fmt'\n"
+	    if !$RAW_PREALLOCATION->{$prealloc};
+
+	return "preallocation=$prealloc";
+    }
+
+    return;
+}
+
 # Storage implementation
 
 # called during addition of storage (before the new storage config got written)
@@ -764,7 +809,8 @@ sub alloc_image {
     } else {
 	my $cmd = ['/usr/bin/qemu-img', 'create'];
 
-	push @$cmd, '-o', 'preallocation=metadata' if $fmt eq 'qcow2';
+	my $prealloc_opt = preallocation_cmd_option($scfg, $fmt);
+	push @$cmd, '-o', $prealloc_opt if defined($prealloc_opt);
 
 	push @$cmd, '-f', $fmt, $path, "${size}K";
 
-- 
2.30.2

[pve-devel] [PATCH v3 manager 1/2] ui: add PreallocationSelector

[Lorenz Stechauner]

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 www/manager6/Makefile                      |  1 +
 www/manager6/form/PreallocationSelector.js | 11 +++++++++++
 2 files changed, 12 insertions(+)
 create mode 100644 www/manager6/form/PreallocationSelector.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 3d1778c2..e5e85aed 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -49,6 +49,7 @@ JSSRC= 							\
 	form/PCISelector.js				\
 	form/PermPathSelector.js			\
 	form/PoolSelector.js				\
+	form/PreallocationSelector.js			\
 	form/PrivilegesSelector.js			\
 	form/QemuBiosSelector.js			\
 	form/SDNControllerSelector.js			\
diff --git a/www/manager6/form/PreallocationSelector.js b/www/manager6/form/PreallocationSelector.js
new file mode 100644
index 00000000..49ea95bb
--- /dev/null
+++ b/www/manager6/form/PreallocationSelector.js
@@ -0,0 +1,11 @@
+Ext.define('PVE.form.preallocationSelector', {
+    extend: 'Proxmox.form.KVComboBox',
+    alias: ['widget.pvePreallocationSelector'],
+    comboItems: [
+	['__default__', Proxmox.Utils.defaultText],
+	['off', 'Off'],
+	['metadata', 'Metadata'],
+	['falloc', 'Full (posix_fallocate)'],
+	['full', 'Full'],
+    ],
+});
-- 
2.30.2

[pve-devel] [PATCH v3 manager 2/2] fix 3850: ui: storage: using PreallocationSelector for file based storage types

[Lorenz Stechauner]

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 www/manager6/controller/StorageEdit.js |  6 ++++++
 www/manager6/storage/Base.js           | 20 ++++++++++++++++++++
 www/manager6/storage/NFSEdit.js        |  2 +-
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/www/manager6/controller/StorageEdit.js b/www/manager6/controller/StorageEdit.js
index 4246d363..cb73b776 100644
--- a/www/manager6/controller/StorageEdit.js
+++ b/www/manager6/controller/StorageEdit.js
@@ -4,6 +4,12 @@ Ext.define('PVE.controller.StorageEdit', {
     control: {
 	'field[name=content]': {
 	    change: function(field, value) {
+		const hasImages = Ext.Array.contains(value, 'images');
+		const prealloc = field.up('form').getForm().findField('preallocation');
+		if (prealloc) {
+		    prealloc.setDisabled(!hasImages);
+		}
+
 		var hasBackups = Ext.Array.contains(value, 'backup');
 		var maxfiles = this.lookupReference('maxfiles');
 		if (!maxfiles) {
diff --git a/www/manager6/storage/Base.js b/www/manager6/storage/Base.js
index f339e8cd..830823af 100644
--- a/www/manager6/storage/Base.js
+++ b/www/manager6/storage/Base.js
@@ -51,6 +51,26 @@ Ext.define('PVE.panel.StorageBase', {
 	    },
 	);
 
+	const qemuImgStorageTypes = ['dir', 'btrfs', 'nfs', 'cifs'];
+
+	if (qemuImgStorageTypes.includes(me.type)) {
+	    const preallocSelector = {
+		xtype: 'pvePreallocationSelector',
+		name: 'preallocation',
+		fieldLabel: gettext('Preallocation'),
+		allowBlank: false,
+		value: '__default__',
+	    };
+
+	    me.advancedColumn1 = me.advancedColumn1 || [];
+	    me.advancedColumn2 = me.advancedColumn2 || [];
+	    if (me.advancedColumn2.length < me.advancedColumn1.length) {
+		me.advancedColumn2.unshift(preallocSelector);
+	    } else {
+		me.advancedColumn1.unshift(preallocSelector);
+	    }
+	}
+
 	me.callParent();
     },
 });
diff --git a/www/manager6/storage/NFSEdit.js b/www/manager6/storage/NFSEdit.js
index faa41732..202c7de0 100644
--- a/www/manager6/storage/NFSEdit.js
+++ b/www/manager6/storage/NFSEdit.js
@@ -143,7 +143,7 @@ Ext.define('PVE.storage.NFSInputPanel', {
 	    },
 	];
 
-	me.advancedColumn1 = [
+	me.advancedColumn2 = [
 	    {
 		xtype: 'proxmoxKVComboBox',
 		fieldLabel: gettext('NFS Version'),
-- 
2.30.2

Re: [pve-devel] [PATCH v3 manager 2/2] fix 3850: ui: storage: using PreallocationSelector for file based storage types

[Fabian Ebner]

Am 11.10.21 um 14:13 schrieb Lorenz Stechauner:
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
>   www/manager6/controller/StorageEdit.js |  6 ++++++
>   www/manager6/storage/Base.js           | 20 ++++++++++++++++++++
>   www/manager6/storage/NFSEdit.js        |  2 +-
>   3 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/www/manager6/controller/StorageEdit.js b/www/manager6/controller/StorageEdit.js
> index 4246d363..cb73b776 100644
> --- a/www/manager6/controller/StorageEdit.js
> +++ b/www/manager6/controller/StorageEdit.js
> @@ -4,6 +4,12 @@ Ext.define('PVE.controller.StorageEdit', {
>       control: {
>   	'field[name=content]': {
>   	    change: function(field, value) {
> +		const hasImages = Ext.Array.contains(value, 'images');
> +		const prealloc = field.up('form').getForm().findField('preallocation');
> +		if (prealloc) {
> +		    prealloc.setDisabled(!hasImages);
> +		}
> +
>   		var hasBackups = Ext.Array.contains(value, 'backup');
>   		var maxfiles = this.lookupReference('maxfiles');
>   		if (!maxfiles) {
> diff --git a/www/manager6/storage/Base.js b/www/manager6/storage/Base.js
> index f339e8cd..830823af 100644
> --- a/www/manager6/storage/Base.js
> +++ b/www/manager6/storage/Base.js
> @@ -51,6 +51,26 @@ Ext.define('PVE.panel.StorageBase', {
>   	    },
>   	);
>   
> +	const qemuImgStorageTypes = ['dir', 'btrfs', 'nfs', 'cifs'];

Missing 'glusterfs'.

> +
> +	if (qemuImgStorageTypes.includes(me.type)) {
> +	    const preallocSelector = {
> +		xtype: 'pvePreallocationSelector',
> +		name: 'preallocation',
> +		fieldLabel: gettext('Preallocation'),
> +		allowBlank: false,
> +		value: '__default__',
> +	    };
> +
> +	    me.advancedColumn1 = me.advancedColumn1 || [];
> +	    me.advancedColumn2 = me.advancedColumn2 || [];
> +	    if (me.advancedColumn2.length < me.advancedColumn1.length) {
> +		me.advancedColumn2.unshift(preallocSelector);
> +	    } else {
> +		me.advancedColumn1.unshift(preallocSelector);
> +	    }
> +	}
> +
>   	me.callParent();
>       },
>   });
> diff --git a/www/manager6/storage/NFSEdit.js b/www/manager6/storage/NFSEdit.js
> index faa41732..202c7de0 100644
> --- a/www/manager6/storage/NFSEdit.js
> +++ b/www/manager6/storage/NFSEdit.js
> @@ -143,7 +143,7 @@ Ext.define('PVE.storage.NFSInputPanel', {
>   	    },
>   	];
>   
> -	me.advancedColumn1 = [
> +	me.advancedColumn2 = [
>   	    {
>   		xtype: 'proxmoxKVComboBox',
>   		fieldLabel: gettext('NFS Version'),
> 

Re: [pve-devel] [PATCH-SERIES v3 storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images

[Fabian Ebner]

If a patch and its context doesn't change code-wise, you can add the 
Reviewed-by and Tested-by tags from the previous version.

While testing, I found that the last patch is missing 'glusterfs' in the 
list of storage types. Except for that:

Reviewed-by: Fabian Ebner <f.ebner@proxmox.com>
Tested-by: Fabian Ebner <f.ebner@proxmox.com>

Am 11.10.21 um 14:13 schrieb Lorenz Stechauner:
> changes to v2:
> * cleaned up commit message for pve-storage
> * changed implementation to put the preallocation selector on the column with fewer items
> 
> changes to v1:
> * adjusted preallocation api description
> * moved sub preallocation_cmd_option above `# Storage implementation`
> * updated PreallocationSelector to work with `default`
> * reworked placement of Prealloc.Selector in Base.js
> 
> 
> pve-storage:
> Lorenz Stechauner (1):
>    fix #3580: plugins: make preallocation mode selectable for qcow2 and
>      raw images
> 
>   PVE/Storage/BTRFSPlugin.pm     |  1 +
>   PVE/Storage/CIFSPlugin.pm      |  1 +
>   PVE/Storage/DirPlugin.pm       |  1 +
>   PVE/Storage/GlusterfsPlugin.pm |  4 ++-
>   PVE/Storage/NFSPlugin.pm       |  1 +
>   PVE/Storage/Plugin.pm          | 48 +++++++++++++++++++++++++++++++++-
>   6 files changed, 54 insertions(+), 2 deletions(-)
> 
> 
> pve-manager
> Lorenz Stechauner (2):
>    ui: add PreallocationSelector
>    fix 3850: ui: storage: using PreallocationSelector for file based
>      storage types
> 
>   www/manager6/Makefile                      |  1 +
>   www/manager6/controller/StorageEdit.js     |  6 ++++++
>   www/manager6/form/PreallocationSelector.js | 11 +++++++++++
>   www/manager6/storage/Base.js               | 20 ++++++++++++++++++++
>   www/manager6/storage/NFSEdit.js            |  2 +-
>   5 files changed, 39 insertions(+), 1 deletion(-)
>   create mode 100644 www/manager6/form/PreallocationSelector.js
>