public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "DERUMIER, Alexandre via pve-devel" <pve-devel@lists.proxmox.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>,
	"f.ebner@proxmox.com" <f.ebner@proxmox.com>
Cc: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
Subject: Re: [pve-devel] [PATCH pve-storage] qcow2: resize: add preallocation support
Date: Mon, 3 Feb 2025 16:11:48 +0000	[thread overview]
Message-ID: <mailman.8.1738599119.293.pve-devel@lists.proxmox.com> (raw)
In-Reply-To: <060e9962-b4e4-4342-8f20-94930bdc21e9@proxmox.com>

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

  reply	other threads:[~2025-02-03 16:12 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-19 16:18 Alexandre Derumier via pve-devel
2025-02-03 14:39 ` Fiona Ebner
2025-02-03 16:11   ` DERUMIER, Alexandre via pve-devel [this message]
     [not found]   ` <9b0bfadfca34c384ec139a8a08b7edcdb414da7e.camel@groupe-cyllene.com>
2025-02-04  9:21     ` Fiona Ebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=mailman.8.1738599119.293.pve-devel@lists.proxmox.com \
    --to=pve-devel@lists.proxmox.com \
    --cc=alexandre.derumier@groupe-cyllene.com \
    --cc=f.ebner@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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