From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH qemu-server 6/6] fix #6543: use qcow2 'discard-no-unref' option when using snapshot-as-volume-chain
Date: Fri, 25 Jul 2025 09:38:54 +0200 [thread overview]
Message-ID: <1753428793.n5phbfd5ll.astroid@yuna.none> (raw)
In-Reply-To: <20250724135956.112138-7-f.ebner@proxmox.com>
On July 24, 2025 3:59 pm, Fiona Ebner wrote:
> Without the 'discard-no-unref', a qcow2 file can grow beyond what
> 'qemu-img measure' reports, because of fragmentation. This can lead to
> IO errors with qcow2 on top of LVM storages, where the containing LV
> is allocated with that size. Guard enabling the option with
> having 'snapshot-as-volume-chain' in the storage configuration for
> now. Enabling it always should be evaluated a bit more and tested on
> different storages. It is a runtime-only option just affecting how
> referencing clusters is handled during discard in qcow2 and nothing
> else, so it is also fine for existing images and migration streams.
>
> While 'snapshot-as-volume-chain' is not the perfect proxy, as that's
> not only for LVM, it's an experimental feature that covers the LVM
> case and it seems like a nice fit to try out the new option on
> file-based storages too.
>
> Suggested-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> src/PVE/QemuServer/Blockdev.pm | 7 ++++++
> src/PVE/QemuServer/QemuImage.pm | 19 +++++++++++++++
> src/test/cfg2cmd/simple-backingchain.conf.cmd | 2 +-
> src/test/run_qemu_img_convert_tests.pl | 24 ++++++++++++++-----
> 4 files changed, 45 insertions(+), 7 deletions(-)
>
> diff --git a/src/PVE/QemuServer/Blockdev.pm b/src/PVE/QemuServer/Blockdev.pm
> index 8528a587..1487bc99 100644
> --- a/src/PVE/QemuServer/Blockdev.pm
> +++ b/src/PVE/QemuServer/Blockdev.pm
> @@ -372,6 +372,13 @@ my sub generate_format_blockdev {
> $blockdev->{size} = int($options->{size});
> }
>
> + # see bug #6543: without this option, fragmentation can lead to the qcow2 file growing larger
> + # than what qemu-img measure reports, which is problematic for qcow2-on-top-of-LVM
> + # TODO test and consider enabling this in general
> + if ($scfg && $scfg->{'snapshot-as-volume-chain'}) {
> + $blockdev->{'discard-no-unref'} = JSON::true if $format eq 'qcow2';
> + }
> +
> return $blockdev;
> }
>
> diff --git a/src/PVE/QemuServer/QemuImage.pm b/src/PVE/QemuServer/QemuImage.pm
> index 026c24e9..7f6d5f01 100644
> --- a/src/PVE/QemuServer/QemuImage.pm
> +++ b/src/PVE/QemuServer/QemuImage.pm
> @@ -3,6 +3,9 @@ package PVE::QemuServer::QemuImage;
> use strict;
> use warnings;
>
> +use Fcntl qw(S_ISBLK);
> +use File::stat;
> +
> use PVE::Format qw(render_bytes);
> use PVE::Storage;
> use PVE::Tools;
> @@ -27,6 +30,18 @@ sub convert_iscsi_path {
> die "cannot convert iscsi path '$path', unknown format\n";
> }
>
> +my sub qcow2_target_image_opts {
> + my ($path, @qcow2_opts) = @_;
> +
> + my $st = File::stat::stat($path) or die "stat for '$path' failed - $!\n";
right now this is only called for PVE-managed volumes.. so we could
actually call qemu_blockdev_options instead in `convert` below, and use
the driver (and possibly other things?) from there?
> +
> + my $driver = S_ISBLK($st->mode) ? 'host_device' : 'file';
> +
> + my $qcow2_opts_str = ',' . join(',', @qcow2_opts);
> +
> + return "driver=qcow2$qcow2_opts_str,file.driver=$driver,file.filename=$path";
> +}
> +
> # The possible options are:
> # bwlimit - The bandwidth limit in KiB/s.
> # is-zero-initialized - If the destination image is zero-initialized.
> @@ -71,6 +86,7 @@ sub convert {
> my $dst_format = checked_volume_format($storecfg, $dst_volid);
> my $dst_path = PVE::Storage::path($storecfg, $dst_volid);
> my $dst_is_iscsi = ($dst_path =~ m|^iscsi://|);
> + my $dst_needs_discard_no_unref = $dst_scfg->{'snapshot-as-volume-chain'};
&& $dst_format eq 'qcow2'
as above in Blockdev.pm?
> my $support_qemu_snapshots = PVE::Storage::volume_qemu_snapshot_method($storecfg, $src_volid);
>
> my $cmd = [];
> @@ -94,6 +110,9 @@ sub convert {
> if ($dst_is_iscsi) {
> push @$cmd, '--target-image-opts';
> $dst_path = convert_iscsi_path($dst_path);
> + } elsif ($dst_needs_discard_no_unref) {
> + push @$cmd, '--target-image-opts';
> + $dst_path = qcow2_target_image_opts($dst_path, 'discard-no-unref=true');
> } else {
> push @$cmd, '-O', $dst_format;
> }
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
next prev parent reply other threads:[~2025-07-25 7:37 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-24 13:59 [pve-devel] [PATCH-SERIES qemu-server 0/6] blockdev and snapshot-as-volume-chain on LVM fixes Fiona Ebner
2025-07-24 13:59 ` [pve-devel] [PATCH qemu-server 1/6] blockdev: helper to add common options Fiona Ebner
2025-07-24 13:59 ` [pve-devel] [PATCH qemu-server 2/6] blockdev: fix discard Fiona Ebner
2025-07-24 13:59 ` [pve-devel] [PATCH qemu-server 3/6] tests: image convert: avoid hard-coded VM ID in result Fiona Ebner
2025-07-24 13:59 ` [pve-devel] [PATCH qemu-server 4/6] tests: image convert: properly set snapshot-as-volume-chain option Fiona Ebner
2025-07-24 13:59 ` [pve-devel] [PATCH qemu-server 5/6] tests: image convert: add tests where storages with 'snapshot-as-volume-chain' are the target Fiona Ebner
2025-07-24 13:59 ` [pve-devel] [PATCH qemu-server 6/6] fix #6543: use qcow2 'discard-no-unref' option when using snapshot-as-volume-chain Fiona Ebner
2025-07-24 18:01 ` DERUMIER, Alexandre via pve-devel
2025-07-25 7:38 ` Fabian Grünbichler [this message]
2025-07-25 8:24 ` Fiona Ebner
2025-07-25 7:40 ` [pve-devel] [PATCH-SERIES qemu-server 0/6] blockdev and snapshot-as-volume-chain on LVM fixes Fabian Grünbichler
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=1753428793.n5phbfd5ll.astroid@yuna.none \
--to=f.gruenbichler@proxmox.com \
--cc=pve-devel@lists.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