public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH-SERIES v2 storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images
@ 2021-09-28 13:07 Lorenz Stechauner
  2021-09-28 13:07 ` [pve-devel] [PATCH v2 storage 1/1] fix #3580: plugins: " Lorenz Stechauner
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Lorenz Stechauner @ 2021-09-28 13:07 UTC (permalink / raw)
  To: pve-devel

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               | 18 ++++++++++++++++++
 www/manager6/storage/NFSEdit.js            |  2 +-
 5 files changed, 37 insertions(+), 1 deletion(-)
 create mode 100644 www/manager6/form/PreallocationSelector.js

-- 
2.30.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [PATCH v2 storage 1/1] fix #3580: plugins: make preallocation mode selectable for qcow2 and raw images
  2021-09-28 13:07 [pve-devel] [PATCH-SERIES v2 storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images Lorenz Stechauner
@ 2021-09-28 13:07 ` Lorenz Stechauner
  2021-10-07 12:44   ` Fabian Ebner
  2021-09-28 13:07 ` [pve-devel] [PATCH v2 manager 1/2] ui: add PreallocationSelector Lorenz Stechauner
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Lorenz Stechauner @ 2021-09-28 13:07 UTC (permalink / raw)
  To: pve-devel

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 (I was unable to find any documentation on this, so
    could only test this and found, that 'off' was the most
    fitting.)

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 b30000b..97633a2 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 417d1fd..4308522 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





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [PATCH v2 manager 1/2] ui: add PreallocationSelector
  2021-09-28 13:07 [pve-devel] [PATCH-SERIES v2 storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images Lorenz Stechauner
  2021-09-28 13:07 ` [pve-devel] [PATCH v2 storage 1/1] fix #3580: plugins: " Lorenz Stechauner
@ 2021-09-28 13:07 ` Lorenz Stechauner
  2021-09-28 13:07 ` [pve-devel] [PATCH v2 manager 2/2] fix 3850: ui: storage: using PreallocationSelector for file based storage types Lorenz Stechauner
  2021-10-07 12:47 ` [pve-devel] [PATCH-SERIES v2 storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images Fabian Ebner
  3 siblings, 0 replies; 7+ messages in thread
From: Lorenz Stechauner @ 2021-09-28 13:07 UTC (permalink / raw)
  To: pve-devel

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 7d491f57..0804786d 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





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [PATCH v2 manager 2/2] fix 3850: ui: storage: using PreallocationSelector for file based storage types
  2021-09-28 13:07 [pve-devel] [PATCH-SERIES v2 storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images Lorenz Stechauner
  2021-09-28 13:07 ` [pve-devel] [PATCH v2 storage 1/1] fix #3580: plugins: " Lorenz Stechauner
  2021-09-28 13:07 ` [pve-devel] [PATCH v2 manager 1/2] ui: add PreallocationSelector Lorenz Stechauner
@ 2021-09-28 13:07 ` Lorenz Stechauner
  2021-10-07 12:45   ` Fabian Ebner
  2021-10-07 12:47 ` [pve-devel] [PATCH-SERIES v2 storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images Fabian Ebner
  3 siblings, 1 reply; 7+ messages in thread
From: Lorenz Stechauner @ 2021-09-28 13:07 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 www/manager6/controller/StorageEdit.js |  6 ++++++
 www/manager6/storage/Base.js           | 18 ++++++++++++++++++
 www/manager6/storage/NFSEdit.js        |  2 +-
 3 files changed, 25 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..404a90f5 100644
--- a/www/manager6/storage/Base.js
+++ b/www/manager6/storage/Base.js
@@ -51,6 +51,24 @@ 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__',
+	    };
+
+	    if (me.advancedColumn1) {
+		me.advancedColumn1.unshift(preallocSelector);
+	    } else {
+		me.advancedColumn1 = [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





^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pve-devel] [PATCH v2 storage 1/1] fix #3580: plugins: make preallocation mode selectable for qcow2 and raw images
  2021-09-28 13:07 ` [pve-devel] [PATCH v2 storage 1/1] fix #3580: plugins: " Lorenz Stechauner
@ 2021-10-07 12:44   ` Fabian Ebner
  0 siblings, 0 replies; 7+ messages in thread
From: Fabian Ebner @ 2021-10-07 12:44 UTC (permalink / raw)
  To: pve-devel, Lorenz Stechauner

Am 28.09.21 um 15:07 schrieb 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 (I was unable to find any documentation on this, so
>      could only test this and found, that 'off' was the most
>      fitting.)

Yes, should be off. See raw_co_create in QEMU's block/file-posix.c

> 
> 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 b30000b..97633a2 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 417d1fd..4308522 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";
>   
> 




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pve-devel] [PATCH v2 manager 2/2] fix 3850: ui: storage: using PreallocationSelector for file based storage types
  2021-09-28 13:07 ` [pve-devel] [PATCH v2 manager 2/2] fix 3850: ui: storage: using PreallocationSelector for file based storage types Lorenz Stechauner
@ 2021-10-07 12:45   ` Fabian Ebner
  0 siblings, 0 replies; 7+ messages in thread
From: Fabian Ebner @ 2021-10-07 12:45 UTC (permalink / raw)
  To: pve-devel, Lorenz Stechauner

Am 28.09.21 um 15:07 schrieb Lorenz Stechauner:
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
>   www/manager6/controller/StorageEdit.js |  6 ++++++
>   www/manager6/storage/Base.js           | 18 ++++++++++++++++++
>   www/manager6/storage/NFSEdit.js        |  2 +-
>   3 files changed, 25 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..404a90f5 100644
> --- a/www/manager6/storage/Base.js
> +++ b/www/manager6/storage/Base.js
> @@ -51,6 +51,24 @@ 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__',
> +	    };
> +
> +	    if (me.advancedColumn1) {
> +		me.advancedColumn1.unshift(preallocSelector);
> +	    } else {
> +		me.advancedColumn1 = [preallocSelector];
> +	    }

Nit: Pushing (or unshifting) the selector onto the advanced column with 
fewer elements seems a bit more future-proof to me:

me.advancedColumn1 = me.advancedColumn1 || [];
me.advancedColumn2 = me.advancedColumn2 || [];
if (me.advancedColumn2.length < me.advancedColumn1.length) {
     me.advancedColumn2.push(preallocSelector);
} else {
     me.advancedColumn1.push(preallocSelector);
}

Then the change for NFS below wouldn't be necessary either, but of 
course no big deal.

> +	}
> +
>   	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'),
> 




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pve-devel] [PATCH-SERIES v2 storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images
  2021-09-28 13:07 [pve-devel] [PATCH-SERIES v2 storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images Lorenz Stechauner
                   ` (2 preceding siblings ...)
  2021-09-28 13:07 ` [pve-devel] [PATCH v2 manager 2/2] fix 3850: ui: storage: using PreallocationSelector for file based storage types Lorenz Stechauner
@ 2021-10-07 12:47 ` Fabian Ebner
  3 siblings, 0 replies; 7+ messages in thread
From: Fabian Ebner @ 2021-10-07 12:47 UTC (permalink / raw)
  To: pve-devel, Lorenz Stechauner

Series looks good to me, except for a small nit.

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

Am 28.09.21 um 15:07 schrieb Lorenz Stechauner:
> 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               | 18 ++++++++++++++++++
>   www/manager6/storage/NFSEdit.js            |  2 +-
>   5 files changed, 37 insertions(+), 1 deletion(-)
>   create mode 100644 www/manager6/form/PreallocationSelector.js
> 




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-10-07 12:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28 13:07 [pve-devel] [PATCH-SERIES v2 storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images Lorenz Stechauner
2021-09-28 13:07 ` [pve-devel] [PATCH v2 storage 1/1] fix #3580: plugins: " Lorenz Stechauner
2021-10-07 12:44   ` Fabian Ebner
2021-09-28 13:07 ` [pve-devel] [PATCH v2 manager 1/2] ui: add PreallocationSelector Lorenz Stechauner
2021-09-28 13:07 ` [pve-devel] [PATCH v2 manager 2/2] fix 3850: ui: storage: using PreallocationSelector for file based storage types Lorenz Stechauner
2021-10-07 12:45   ` Fabian Ebner
2021-10-07 12:47 ` [pve-devel] [PATCH-SERIES v2 storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images Fabian Ebner

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