[pve-devel] [PATCH pve-storage] qcow2: resize: add preallocation support

[pve-devel] [PATCH pve-storage] qcow2: resize: add preallocation support

[Alexandre Derumier via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 3911 bytes --]

From: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH pve-storage] qcow2: resize: add preallocation support
Date: Thu, 19 Dec 2024 17:18:16 +0100
Message-ID: <20241219161816.952295-1-alexandre.derumier@groupe-cyllene.com>

Seem that we totally forgot to add it, it's available since 2017
https://www.mail-archive.com/qemu-devel@nongnu.org/msg436979.html
---
 src/PVE/Storage/Plugin.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 6a6a804..d0ce3ae 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1166,7 +1166,9 @@ sub volume_resize {
 
     my $format = ($class->parse_volname($volname))[6];
 
-    my $cmd = ['/usr/bin/qemu-img', 'resize', '-f', $format, $path , $size];
+    my $prealloc_opt = preallocation_cmd_option($scfg, $format);
+
+    my $cmd = ['/usr/bin/qemu-img', 'resize', "--$prealloc_opt", '-f', $format, $path , $size];
 
     run_command($cmd, timeout => 10);
 
-- 
2.39.5



[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH pve-storage] qcow2: resize: add preallocation support

[Fiona Ebner]

Am 19.12.24 um 17:18 schrieb Alexandre Derumier via pve-devel:
> Seem that we totally forgot to add it, it's available since 2017
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg436979.html

Nit: it's better to link to the commit rather than the mailing list for
things that are already applied.

Missing your Signed-off-by.

Hmm, I wanted to suggest to query the image to see what kind of
preallocation it was created with and then use that setting to stay
consistent. But that information doesn't seem to get recorded (on an
image-wide level) AFAICS. It might be surprising that changes to the
storage configuration setting will also apply to already existing images
and we should document the behavior for resize in the description of the
'preallocation' setting.

Seems like the "block_resize" QMP command does not have the setting at
all, so if we add it here, the behavior would still be inconsistent in
that regard :/ But oh well, could still be added on top later if we can
get that feature in upstream. But should also be documented, that it
doesn't apply for live resize.

> ---
>  src/PVE/Storage/Plugin.pm | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index 6a6a804..d0ce3ae 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -1166,7 +1166,9 @@ sub volume_resize {
>  
>      my $format = ($class->parse_volname($volname))[6];
>  
> -    my $cmd = ['/usr/bin/qemu-img', 'resize', '-f', $format, $path , $size];
> +    my $prealloc_opt = preallocation_cmd_option($scfg, $format);
> +
> +    my $cmd = ['/usr/bin/qemu-img', 'resize', "--$prealloc_opt", '-f', $format, $path , $size];
>  
>      run_command($cmd, timeout => 10);
>  
> -- 
> 2.39.5
> 
> 



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH pve-storage] qcow2: resize: add preallocation support

[DERUMIER, Alexandre via pve-devel]

[-- Attachment #1: Type: message/rfc822, Size: 16416 bytes --]

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>, "f.ebner@proxmox.com" <f.ebner@proxmox.com>
Subject: Re: [pve-devel] [PATCH pve-storage] qcow2: resize: add preallocation support
Date: Mon, 3 Feb 2025 16:11:48 +0000
Message-ID: <9b0bfadfca34c384ec139a8a08b7edcdb414da7e.camel@groupe-cyllene.com>

-------- Message initial --------
De: Fiona Ebner <f.ebner@proxmox.com>
À: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Cc: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
Objet: Re: [pve-devel] [PATCH pve-storage] qcow2: resize: add
preallocation support
Date: 03/02/2025 15:39:41

Am 19.12.24 um 17:18 schrieb Alexandre Derumier via pve-devel:
> Seem that we totally forgot to add it, it's available since 2017
> https://antiphishing.vadesecure.com/v4?f=TmtFVlNVNmxSYnFaWFhxYhCRpRpL
> 9Z3oy4_Tk4UXcP5N_qAOXqIRqmal4wpM8L7y&i=d09ZU0Z5WTAyTG85WWdYbKbcMR2wMI
> IXhqLwqdlSH4I&k=DWI7&r=UTEzTUpQcktwRVdhdEg1TKCFOzhw8CGaAiMfyFTpTR_LTs
> pF9zP2JS-LN0ctA-XBzHeMG-
> sD1OqL3ihNxDMXJg&s=ee8fad1f3a3cde35c58d5e5735a648efe5c5270d76a0a57b9a
> 6909d8d3104966&u=https%3A%2F%2Fwww.mail-archive.com%2Fqemu-
> devel%40nongnu.org%2Fmsg436979.html

Nit: it's better to link to the commit rather than the mailing list for
things that are already applied.

>>Missing your Signed-off-by.

oops,sorry

>>Hmm, I wanted to suggest to query the image to see what kind of
>>preallocation it was created with and then use that setting to stay
>>consistent. 
>>But that information doesn't seem to get recorded (on an
>>image-wide level) AFAICS.

for full pre-allocation, I think we can simply check the current qcow2
usage vs the size configured.

for qcow2 metadatas, I really don't known any way to do it.

>> It might be surprising that changes to the
>>storage configuration setting will also apply to already existing
>>images

Personnaly, I was more surprised than this never have worked on resize
before ^_^.

That don't shock me that it's respect the current assigned option at
the moment of the resize.

>>and we should document the behavior for resize in the description of
>>the
>>'preallocation' setting.

But yes, it should be documented.
I'll write a patch of pve-docs


>>Seems like the "block_resize" QMP command does not have the setting
>>at
>>all, so if we add it here, the behavior would still be inconsistent
>>in
>>that regard :/ But oh well, could still be added on top later if we
>>can
>>get that feature in upstream. But should also be documented, that it
>>doesn't apply for live resize.

yes, indeed, it doesn't exist for live running image. (I think to have
seen discussion on the qemu mailing about it, but it require some kind
of block job if I remember correctly).

It's existing a preallocate-filter 

https://qemu.googlesource.com/qemu/+/refs/tags/v8.0.3/block/preallocate.c

but it's a little bit different, it's preallocating live.
(allocating by chunk of 1MB for example, when you have a 4k write
reaching EOF)



Thanks for the review !



> ---
>  src/PVE/Storage/Plugin.pm | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index 6a6a804..d0ce3ae 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -1166,7 +1166,9 @@ sub volume_resize {
>  
>      my $format = ($class->parse_volname($volname))[6];
>  
> -    my $cmd = ['/usr/bin/qemu-img', 'resize', '-f', $format, $path ,
> $size];
> +    my $prealloc_opt = preallocation_cmd_option($scfg, $format);
> +
> +    my $cmd = ['/usr/bin/qemu-img', 'resize', "--$prealloc_opt", '-
> f', $format, $path , $size];
>  
>      run_command($cmd, timeout => 10);
>  
> -- 
> 2.39.5
> 
> 




[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH pve-storage] qcow2: resize: add preallocation support

[Fiona Ebner]

Am 03.02.25 um 17:11 schrieb DERUMIER, Alexandre:
> -------- Message initial --------
> De: Fiona Ebner <f.ebner@proxmox.com>
>>> Hmm, I wanted to suggest to query the image to see what kind of
>>> preallocation it was created with and then use that setting to stay
>>> consistent. 
>>> But that information doesn't seem to get recorded (on an
>>> image-wide level) AFAICS.
> 
> for full pre-allocation, I think we can simply check the current qcow2
> usage vs the size configured.
> 
> for qcow2 metadatas, I really don't known any way to do it.
> 

We don't need to do it based on heuristics. It's really not worth the
effort then IMHO. I just kinda expected to be an image property at
first, but it does make sense that allocation is per-cluster.

>>> It might be surprising that changes to the
>>> storage configuration setting will also apply to already existing
>>> images
> 
> Personnaly, I was more surprised than this never have worked on resize
> before ^_^.
> 
> That don't shock me that it's respect the current assigned option at
> the moment of the resize.

Fair :)

> 
>>> and we should document the behavior for resize in the description of
>>> the
>>> 'preallocation' setting.
> 
> But yes, it should be documented.
> I'll write a patch of pve-docs

Great! There also is the schema in Plugin.pm ;)

> 
> 
>>> Seems like the "block_resize" QMP command does not have the setting
>>> at
>>> all, so if we add it here, the behavior would still be inconsistent
>>> in
>>> that regard :/ But oh well, could still be added on top later if we
>>> can
>>> get that feature in upstream. But should also be documented, that it
>>> doesn't apply for live resize.
> 
> yes, indeed, it doesn't exist for live running image. (I think to have
> seen discussion on the qemu mailing about it, but it require some kind
> of block job if I remember correctly).

I guess that makes sense, because it can be longer running. I noted that
qmp_block_resize() is already a coroutine, maybe that can be leveraged
if we were to implement the feature for live resize. But again, won't be
a blocker here and can still be considered later.

> 
> It's existing a preallocate-filter 
> 
> https://qemu.googlesource.com/qemu/+/refs/tags/v8.0.3/block/preallocate.c
> 
> but it's a little bit different, it's preallocating live.
> (allocating by chunk of 1MB for example, when you have a 4k write
> reaching EOF)

Okay, so not quite what we want here.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel