* [pve-devel] [PATCH-SERIES storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images
@ 2021-09-06 13:15 Lorenz Stechauner
2021-09-06 13:15 ` [pve-devel] [PATCH storage 1/1] fix #3580: plugins: " Lorenz Stechauner
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Lorenz Stechauner @ 2021-09-06 13:15 UTC (permalink / raw)
To: pve-devel
this series allows users to configure the `qemu-img` preallocation
mode per storage. this only applies to file-based storages and the
file types 'qcow2' and 'raw'.
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 | 46 +++++++++++++++++++++++++++++++++-
6 files changed, 52 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 | 14 ++++++++++++++
www/manager6/storage/Base.js | 18 ++++++++++++++++++
4 files changed, 39 insertions(+)
create mode 100644 www/manager6/form/PreallocationSelector.js
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread* [pve-devel] [PATCH storage 1/1] fix #3580: plugins: make preallocation mode selectable for qcow2 and raw images 2021-09-06 13:15 [pve-devel] [PATCH-SERIES storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images Lorenz Stechauner @ 2021-09-06 13:15 ` Lorenz Stechauner 2021-09-08 8:11 ` alexandre derumier 2021-09-14 9:44 ` Fabian Ebner 2021-09-06 13:15 ` [pve-devel] [PATCH manager 1/2] ui: add PreallocationSelector Lorenz Stechauner 2021-09-06 13:15 ` [pve-devel] [PATCH manager 2/2] fix 3850: ui: storage: using PreallocationSelector for file based storage types Lorenz Stechauner 2 siblings, 2 replies; 12+ messages in thread From: Lorenz Stechauner @ 2021-09-06 13:15 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 | 46 +++++++++++++++++++++++++++++++++- 6 files changed, 52 insertions(+), 2 deletions(-) diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm index fe42082..31a2954 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 0221069..2d94413 100644 --- a/PVE/Storage/CIFSPlugin.pm +++ b/PVE/Storage/CIFSPlugin.pm @@ -140,6 +140,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 b1865cb..4924525 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', @@ -150,6 +163,11 @@ my $defaultData = { type => 'string', format => 'pve-storage-format', optional => 1, }, + preallocation => { + description => "Preallocation mode for raw and qcow2 images.", + type => 'string', enum => ['default', 'off', 'metadata', 'falloc', 'full'], + optional => 1, + }, }, }; @@ -762,7 +780,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"; @@ -1484,4 +1503,29 @@ sub volume_import_formats { return (); } +sub preallocation_cmd_option { + my ($scfg, $fmt) = @_; + + my $prealloc = $scfg->{preallocation}; + + $prealloc = undef if $prealloc eq 'default'; + + 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 undef; +} + 1; -- 2.30.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pve-devel] [PATCH storage 1/1] fix #3580: plugins: make preallocation mode selectable for qcow2 and raw images 2021-09-06 13:15 ` [pve-devel] [PATCH storage 1/1] fix #3580: plugins: " Lorenz Stechauner @ 2021-09-08 8:11 ` alexandre derumier 2021-09-09 10:25 ` Fabian Ebner 2021-09-14 9:44 ` Fabian Ebner 1 sibling, 1 reply; 12+ messages in thread From: alexandre derumier @ 2021-09-08 8:11 UTC (permalink / raw) To: Proxmox VE development discussion Hi, it can be done too with ceph rbd with "rbd create ... –thick-provision" Le lundi 06 septembre 2021 à 15:15 +0200, Lorenz Stechauner a écrit : > 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 | 46 > +++++++++++++++++++++++++++++++++- > 6 files changed, 52 insertions(+), 2 deletions(-) > > diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm > index fe42082..31a2954 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 0221069..2d94413 100644 > --- a/PVE/Storage/CIFSPlugin.pm > +++ b/PVE/Storage/CIFSPlugin.pm > @@ -140,6 +140,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 b1865cb..4924525 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', > @@ -150,6 +163,11 @@ my $defaultData = { > type => 'string', format => 'pve-storage-format', > optional => 1, > }, > + preallocation => { > + description => "Preallocation mode for raw and qcow2 > images.", > + type => 'string', enum => ['default', 'off', 'metadata', > 'falloc', 'full'], > + optional => 1, > + }, > }, > }; > > @@ -762,7 +780,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"; > > @@ -1484,4 +1503,29 @@ sub volume_import_formats { > return (); > } > > +sub preallocation_cmd_option { > + my ($scfg, $fmt) = @_; > + > + my $prealloc = $scfg->{preallocation}; > + > + $prealloc = undef if $prealloc eq 'default'; > + > + 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 undef; > +} > + > 1; ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pve-devel] [PATCH storage 1/1] fix #3580: plugins: make preallocation mode selectable for qcow2 and raw images 2021-09-08 8:11 ` alexandre derumier @ 2021-09-09 10:25 ` Fabian Ebner 2021-09-09 11:11 ` Lorenz Stechauner 0 siblings, 1 reply; 12+ messages in thread From: Fabian Ebner @ 2021-09-09 10:25 UTC (permalink / raw) To: pve-devel, aderumier, Lorenz Stechauner Am 08.09.21 um 10:11 schrieb alexandre derumier: > Hi, > it can be done too with ceph rbd with "rbd create ... –thick-provision" > Hi, there also is the 'sparse' storage config option (currently only used for ZFS plugins). If there is only thick or thin, re-using that one is probably nicer, because the newly proposed preallocation option seems to be closely tied to qemu-img. > Le lundi 06 septembre 2021 à 15:15 +0200, Lorenz Stechauner a écrit : >> 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 | 46 >> +++++++++++++++++++++++++++++++++- >> 6 files changed, 52 insertions(+), 2 deletions(-) >> >> diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm >> index fe42082..31a2954 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 0221069..2d94413 100644 >> --- a/PVE/Storage/CIFSPlugin.pm >> +++ b/PVE/Storage/CIFSPlugin.pm >> @@ -140,6 +140,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 b1865cb..4924525 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', >> @@ -150,6 +163,11 @@ my $defaultData = { >> type => 'string', format => 'pve-storage-format', >> optional => 1, >> }, >> + preallocation => { >> + description => "Preallocation mode for raw and qcow2 >> images.", >> + type => 'string', enum => ['default', 'off', 'metadata', >> 'falloc', 'full'], >> + optional => 1, >> + }, >> }, >> }; >> >> @@ -762,7 +780,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"; >> >> @@ -1484,4 +1503,29 @@ sub volume_import_formats { >> return (); >> } >> >> +sub preallocation_cmd_option { >> + my ($scfg, $fmt) = @_; >> + >> + my $prealloc = $scfg->{preallocation}; >> + >> + $prealloc = undef if $prealloc eq 'default'; >> + >> + 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 undef; >> +} >> + >> 1; > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pve-devel] [PATCH storage 1/1] fix #3580: plugins: make preallocation mode selectable for qcow2 and raw images 2021-09-09 10:25 ` Fabian Ebner @ 2021-09-09 11:11 ` Lorenz Stechauner 2021-09-09 12:04 ` Fabian Ebner 0 siblings, 1 reply; 12+ messages in thread From: Lorenz Stechauner @ 2021-09-09 11:11 UTC (permalink / raw) To: Fabian Ebner, pve-devel, aderumier On 09.09.21 12:25, Fabian Ebner wrote: > Am 08.09.21 um 10:11 schrieb alexandre derumier: >> Hi, >> it can be done too with ceph rbd with "rbd create ... –thick-provision" >> > > Hi, > there also is the 'sparse' storage config option (currently only used > for ZFS plugins). If there is only thick or thin, re-using that one is > probably nicer, because the newly proposed preallocation option seems > to be closely tied to qemu-img. Sounds like a good idea. I doubt, that anyone would use full prellocation anyway, so simply using 'sparse' for prealloc=off and default remains prealloc=metadata sounds good. > >> Le lundi 06 septembre 2021 à 15:15 +0200, Lorenz Stechauner a écrit : >>> 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 | 46 >>> +++++++++++++++++++++++++++++++++- >>> 6 files changed, 52 insertions(+), 2 deletions(-) >>> >>> diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm >>> index fe42082..31a2954 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 0221069..2d94413 100644 >>> --- a/PVE/Storage/CIFSPlugin.pm >>> +++ b/PVE/Storage/CIFSPlugin.pm >>> @@ -140,6 +140,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 b1865cb..4924525 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', >>> @@ -150,6 +163,11 @@ my $defaultData = { >>> type => 'string', format => 'pve-storage-format', >>> optional => 1, >>> }, >>> + preallocation => { >>> + description => "Preallocation mode for raw and qcow2 >>> images.", >>> + type => 'string', enum => ['default', 'off', 'metadata', >>> 'falloc', 'full'], >>> + optional => 1, >>> + }, >>> }, >>> }; >>> @@ -762,7 +780,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"; >>> @@ -1484,4 +1503,29 @@ sub volume_import_formats { >>> return (); >>> } >>> +sub preallocation_cmd_option { >>> + my ($scfg, $fmt) = @_; >>> + >>> + my $prealloc = $scfg->{preallocation}; >>> + >>> + $prealloc = undef if $prealloc eq 'default'; >>> + >>> + 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 undef; >>> +} >>> + >>> 1; >> >> _______________________________________________ >> pve-devel mailing list >> pve-devel@lists.proxmox.com >> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pve-devel] [PATCH storage 1/1] fix #3580: plugins: make preallocation mode selectable for qcow2 and raw images 2021-09-09 11:11 ` Lorenz Stechauner @ 2021-09-09 12:04 ` Fabian Ebner 2021-09-09 12:26 ` Lorenz Stechauner 2021-09-09 12:28 ` Thomas Lamprecht 0 siblings, 2 replies; 12+ messages in thread From: Fabian Ebner @ 2021-09-09 12:04 UTC (permalink / raw) To: Lorenz Stechauner, pve-devel, aderumier Am 09.09.21 um 13:11 schrieb Lorenz Stechauner: > > On 09.09.21 12:25, Fabian Ebner wrote: >> Am 08.09.21 um 10:11 schrieb alexandre derumier: >>> Hi, >>> it can be done too with ceph rbd with "rbd create ... –thick-provision" >>> >> >> Hi, >> there also is the 'sparse' storage config option (currently only used >> for ZFS plugins). If there is only thick or thin, re-using that one is >> probably nicer, because the newly proposed preallocation option seems >> to be closely tied to qemu-img. > > Sounds like a good idea. I doubt, that anyone would use full > prellocation anyway, so simply using 'sparse' for prealloc=off and > default remains prealloc=metadata sounds good. > I actually only meant re-using 'sparse' for the RBD use case. But yes, it seems like re-using it for the qemu-img use case would be enough to fix the bug too. It might be a bit confusing though, because when sparse is not set, the images would still be mostly sparse (except for metadata). >> >>> Le lundi 06 septembre 2021 à 15:15 +0200, Lorenz Stechauner a écrit : >>>> 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 | 46 >>>> +++++++++++++++++++++++++++++++++- >>>> 6 files changed, 52 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm >>>> index fe42082..31a2954 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 0221069..2d94413 100644 >>>> --- a/PVE/Storage/CIFSPlugin.pm >>>> +++ b/PVE/Storage/CIFSPlugin.pm >>>> @@ -140,6 +140,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 b1865cb..4924525 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', >>>> @@ -150,6 +163,11 @@ my $defaultData = { >>>> type => 'string', format => 'pve-storage-format', >>>> optional => 1, >>>> }, >>>> + preallocation => { >>>> + description => "Preallocation mode for raw and qcow2 >>>> images.", >>>> + type => 'string', enum => ['default', 'off', 'metadata', >>>> 'falloc', 'full'], >>>> + optional => 1, >>>> + }, >>>> }, >>>> }; >>>> @@ -762,7 +780,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"; >>>> @@ -1484,4 +1503,29 @@ sub volume_import_formats { >>>> return (); >>>> } >>>> +sub preallocation_cmd_option { >>>> + my ($scfg, $fmt) = @_; >>>> + >>>> + my $prealloc = $scfg->{preallocation}; >>>> + >>>> + $prealloc = undef if $prealloc eq 'default'; >>>> + >>>> + 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 undef; >>>> +} >>>> + >>>> 1; >>> >>> _______________________________________________ >>> pve-devel mailing list >>> pve-devel@lists.proxmox.com >>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >>> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pve-devel] [PATCH storage 1/1] fix #3580: plugins: make preallocation mode selectable for qcow2 and raw images 2021-09-09 12:04 ` Fabian Ebner @ 2021-09-09 12:26 ` Lorenz Stechauner 2021-09-09 12:28 ` Thomas Lamprecht 1 sibling, 0 replies; 12+ messages in thread From: Lorenz Stechauner @ 2021-09-09 12:26 UTC (permalink / raw) To: Fabian Ebner, pve-devel, aderumier On 09.09.21 14:04, Fabian Ebner wrote: > Am 09.09.21 um 13:11 schrieb Lorenz Stechauner: >> >> On 09.09.21 12:25, Fabian Ebner wrote: >>> Am 08.09.21 um 10:11 schrieb alexandre derumier: >>>> Hi, >>>> it can be done too with ceph rbd with "rbd create ... >>>> –thick-provision" >>>> >>> >>> Hi, >>> there also is the 'sparse' storage config option (currently only >>> used for ZFS plugins). If there is only thick or thin, re-using that >>> one is probably nicer, because the newly proposed preallocation >>> option seems to be closely tied to qemu-img. >> >> Sounds like a good idea. I doubt, that anyone would use full >> prellocation anyway, so simply using 'sparse' for prealloc=off and >> default remains prealloc=metadata sounds good. >> > > I actually only meant re-using 'sparse' for the RBD use case. But yes, > it seems like re-using it for the qemu-img use case would be enough to > fix the bug too. It might be a bit confusing though, because when > sparse is not set, the images would still be mostly sparse (except for > metadata). makes sense, I got a bit confused by the rbd stuff. Then I won't update the patch to use 'sparse' :) > >>> >>>> Le lundi 06 septembre 2021 à 15:15 +0200, Lorenz Stechauner a écrit : >>>>> 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> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pve-devel] [PATCH storage 1/1] fix #3580: plugins: make preallocation mode selectable for qcow2 and raw images 2021-09-09 12:04 ` Fabian Ebner 2021-09-09 12:26 ` Lorenz Stechauner @ 2021-09-09 12:28 ` Thomas Lamprecht 1 sibling, 0 replies; 12+ messages in thread From: Thomas Lamprecht @ 2021-09-09 12:28 UTC (permalink / raw) To: Proxmox VE development discussion, Fabian Ebner, Lorenz Stechauner, aderumier On 09.09.21 14:04, Fabian Ebner wrote: > Am 09.09.21 um 13:11 schrieb Lorenz Stechauner: >> >> On 09.09.21 12:25, Fabian Ebner wrote: >>> Am 08.09.21 um 10:11 schrieb alexandre derumier: >>>> Hi, >>>> it can be done too with ceph rbd with "rbd create ... –thick-provision" >>>> >>> >>> Hi, >>> there also is the 'sparse' storage config option (currently only used for ZFS plugins). If there is only thick or thin, re-using that one is probably nicer, because the newly proposed preallocation option seems to be closely tied to qemu-img. >> >> Sounds like a good idea. I doubt, that anyone would use full prellocation anyway, so simply using 'sparse' for prealloc=off and default remains prealloc=metadata sounds good. >> > > I actually only meant re-using 'sparse' for the RBD use case. But yes, it seems like re-using it for the qemu-img use case would be enough to fix the bug too. It might be a bit confusing though, because when sparse is not set, the images would still be mostly sparse (except for metadata). Yeah, I'm not sure that just deriving all from the sparse storage option would be good UX - I think it should be per creation and if we want to expand the scope of such a value we should rather see it as fallback. But IMO this is a bit of an edge case anyway, so I'd not rush the latter without user demand. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pve-devel] [PATCH storage 1/1] fix #3580: plugins: make preallocation mode selectable for qcow2 and raw images 2021-09-06 13:15 ` [pve-devel] [PATCH storage 1/1] fix #3580: plugins: " Lorenz Stechauner 2021-09-08 8:11 ` alexandre derumier @ 2021-09-14 9:44 ` Fabian Ebner 1 sibling, 0 replies; 12+ messages in thread From: Fabian Ebner @ 2021-09-14 9:44 UTC (permalink / raw) To: pve-devel, Lorenz Stechauner Am 06.09.21 um 15:15 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.) > > 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 | 46 +++++++++++++++++++++++++++++++++- > 6 files changed, 52 insertions(+), 2 deletions(-) > > diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm > index fe42082..31a2954 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 0221069..2d94413 100644 > --- a/PVE/Storage/CIFSPlugin.pm > +++ b/PVE/Storage/CIFSPlugin.pm > @@ -140,6 +140,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 b1865cb..4924525 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', > @@ -150,6 +163,11 @@ my $defaultData = { > type => 'string', format => 'pve-storage-format', > optional => 1, > }, > + preallocation => { > + description => "Preallocation mode for raw and qcow2 images.", > + type => 'string', enum => ['default', 'off', 'metadata', 'falloc', 'full'], > + optional => 1, It might be worth to mention that 'metadata' for raw means 'off' in the description. Any reason for an explicit 'default' rather then having 'option not set at all' be the default? You could also write default => 'metadata', as part of the option's properties. > + }, > }, > }; > > @@ -762,7 +780,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"; > > @@ -1484,4 +1503,29 @@ sub volume_import_formats { > return (); > } > > +sub preallocation_cmd_option { I suppose this function should not be part of the Plugin API, so it should either be above the # Storage implementation comment or made into a private helper using my $preallocation_cmd_option = sub > + my ($scfg, $fmt) = @_; > + > + my $prealloc = $scfg->{preallocation}; > + > + $prealloc = undef if $prealloc eq 'default'; Will lead to a warning when $prealloc is already undef: Use of uninitialized value $prealloc in string eq > + > + if ($fmt eq 'qcow2') { > + $prealloc = $prealloc // 'metadata'; > + > + die "preallocation mode '$prealloc' not supported by format '$fmt'\n" if !$QCOW2_PREALLOCATION->{$prealloc}; Style nit: line too long Nit: I'm wondering if the check is worth it. It should not be possible to trigger with the current implementation, and even if there were a bad value and no check here, qemu-img would still complain afterwards. But I'm also fine with the check. > + > + 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}; Same here. > + > + return "preallocation=$prealloc"; > + } > + > + return undef; Nit: 'return;' should be preferred over 'return undef;' (behaves differently when called in list context). > +} > + > 1; > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [pve-devel] [PATCH manager 1/2] ui: add PreallocationSelector 2021-09-06 13:15 [pve-devel] [PATCH-SERIES storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images Lorenz Stechauner 2021-09-06 13:15 ` [pve-devel] [PATCH storage 1/1] fix #3580: plugins: " Lorenz Stechauner @ 2021-09-06 13:15 ` Lorenz Stechauner 2021-09-06 13:15 ` [pve-devel] [PATCH manager 2/2] fix 3850: ui: storage: using PreallocationSelector for file based storage types Lorenz Stechauner 2 siblings, 0 replies; 12+ messages in thread From: Lorenz Stechauner @ 2021-09-06 13:15 UTC (permalink / raw) To: pve-devel Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com> --- www/manager6/Makefile | 1 + www/manager6/form/PreallocationSelector.js | 14 ++++++++++++++ 2 files changed, 15 insertions(+) create mode 100644 www/manager6/form/PreallocationSelector.js diff --git a/www/manager6/Makefile b/www/manager6/Makefile index 75d355a5..f60fad06 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..0a58ee87 --- /dev/null +++ b/www/manager6/form/PreallocationSelector.js @@ -0,0 +1,14 @@ +Ext.define('PVE.form.preallocationSelector', { + extend: 'Proxmox.form.KVComboBox', + alias: ['widget.pvePreallocationSelector'], + config: { + deleteEmpty: false, + }, + comboItems: [ + ['default', 'Default'], + ['off', 'Off'], + ['metadata', 'Metadata'], + ['falloc', 'Full (posix_fallocate)'], + ['full', 'Full'], + ], +}); -- 2.30.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [pve-devel] [PATCH manager 2/2] fix 3850: ui: storage: using PreallocationSelector for file based storage types 2021-09-06 13:15 [pve-devel] [PATCH-SERIES storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images Lorenz Stechauner 2021-09-06 13:15 ` [pve-devel] [PATCH storage 1/1] fix #3580: plugins: " Lorenz Stechauner 2021-09-06 13:15 ` [pve-devel] [PATCH manager 1/2] ui: add PreallocationSelector Lorenz Stechauner @ 2021-09-06 13:15 ` Lorenz Stechauner 2021-09-14 9:55 ` Fabian Ebner 2 siblings, 1 reply; 12+ messages in thread From: Lorenz Stechauner @ 2021-09-06 13:15 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 ++++++++++++++++++ 2 files changed, 24 insertions(+) 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 ee8b54e8..457576a3 100644 --- a/www/manager6/storage/Base.js +++ b/www/manager6/storage/Base.js @@ -51,6 +51,24 @@ Ext.define('PVE.panel.StorageBase', { }, ); + const fileBasedStorageTypes = ['dir', 'btrfs', 'nfs', 'cifs', 'glusterfs']; + + if (fileBasedStorageTypes.includes(me.type)) { + const preallocSelector = { + xtype: 'pvePreallocationSelector', + name: 'preallocation', + fieldLabel: gettext('Preallocation'), + allowBlank: false, + value: 'default', + }; + + // ensures, that the PreallocationSelector is always on the left side + if (me.advancedColumn1) { + me.advancedColumn2 = me.advancedColumn1; + } + me.advancedColumn1 = [preallocSelector]; + } + me.callParent(); }, }); -- 2.30.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pve-devel] [PATCH manager 2/2] fix 3850: ui: storage: using PreallocationSelector for file based storage types 2021-09-06 13:15 ` [pve-devel] [PATCH manager 2/2] fix 3850: ui: storage: using PreallocationSelector for file based storage types Lorenz Stechauner @ 2021-09-14 9:55 ` Fabian Ebner 0 siblings, 0 replies; 12+ messages in thread From: Fabian Ebner @ 2021-09-14 9:55 UTC (permalink / raw) To: pve-devel, Lorenz Stechauner Am 06.09.21 um 15:15 schrieb Lorenz Stechauner: > Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com> > --- > www/manager6/controller/StorageEdit.js | 6 ++++++ > www/manager6/storage/Base.js | 18 ++++++++++++++++++ > 2 files changed, 24 insertions(+) > > 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 ee8b54e8..457576a3 100644 > --- a/www/manager6/storage/Base.js > +++ b/www/manager6/storage/Base.js > @@ -51,6 +51,24 @@ Ext.define('PVE.panel.StorageBase', { > }, > ); > > + const fileBasedStorageTypes = ['dir', 'btrfs', 'nfs', 'cifs', 'glusterfs']; Nit: CephFS is also file-based. What about using qemuImgStorageTypes or if (['dir',...].includes(me.type)) directly? > + > + if (fileBasedStorageTypes.includes(me.type)) { > + const preallocSelector = { > + xtype: 'pvePreallocationSelector', > + name: 'preallocation', > + fieldLabel: gettext('Preallocation'), > + allowBlank: false, > + value: 'default', > + }; > + > + // ensures, that the PreallocationSelector is always on the left side > + if (me.advancedColumn1) { > + me.advancedColumn2 = me.advancedColumn1; Why does it need to be on the left side? What if there already is an advancedColumn2? > + } > + me.advancedColumn1 = [preallocSelector]; > + } > + > me.callParent(); > }, > }); > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-09-14 9:55 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-06 13:15 [pve-devel] [PATCH-SERIES storage/manager] fix #3580: make preallocation mode selectable for qcow2 and raw images Lorenz Stechauner 2021-09-06 13:15 ` [pve-devel] [PATCH storage 1/1] fix #3580: plugins: " Lorenz Stechauner 2021-09-08 8:11 ` alexandre derumier 2021-09-09 10:25 ` Fabian Ebner 2021-09-09 11:11 ` Lorenz Stechauner 2021-09-09 12:04 ` Fabian Ebner 2021-09-09 12:26 ` Lorenz Stechauner 2021-09-09 12:28 ` Thomas Lamprecht 2021-09-14 9:44 ` Fabian Ebner 2021-09-06 13:15 ` [pve-devel] [PATCH manager 1/2] ui: add PreallocationSelector Lorenz Stechauner 2021-09-06 13:15 ` [pve-devel] [PATCH manager 2/2] fix 3850: ui: storage: using PreallocationSelector for file based storage types Lorenz Stechauner 2021-09-14 9:55 ` Fabian Ebner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox