* [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
* 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
* [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 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