From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 98F2E1FF183 for ; Wed, 8 Oct 2025 17:11:46 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6438ABD03; Wed, 8 Oct 2025 17:11:49 +0200 (CEST) From: Fiona Ebner To: pve-devel@lists.proxmox.com Date: Wed, 8 Oct 2025 17:11:32 +0200 Message-ID: <20251008151140.127355-3-f.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20251008151140.127355-1-f.ebner@proxmox.com> References: <20251008151140.127355-1-f.ebner@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1759936273800 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.023 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [lvmplugin.pm] Subject: [pve-devel] [RFC storage 2/2] lvm plugin: disallow disabling 'snapshot-as-volume-chain' while a qcow2 image exists X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" There are multiple reasons to disallow disabling 'snapshot-as-volume-chain' while a qcow2 image exists: 1. The list of allowed formats depends on 'snapshot-as-volume-chain'. 2. Snapshot functionality is broken. This includes creating snapshots, but also rollback, which removes the current volume and then fails. 3. There already is coupling between having qcow2 on LVM and having 'snapshot-as-volume-chain' enabled. For example, the 'discard-no-unref' option is required for qcow2 on LVM, but qemu-server only checks for 'snapshot-as-volume-chain' to avoid hard-coding LVM. Another one is that volume_qemu_snapshot_method() returns 'mixed' when the format is qcow2 even when 'snapshot-as-volume-chain' is disabled. Hunting down these corner cases just to make it easier to disable does not seem to be worth it, considering there's already 1. and 2. as reasons too. 4. There might be other similar issues that have not surfaced yet, because disabling the feature while qcow2 is present is essentially untested and very uncommon. For file-based storages, the 'snapshot-as-volume-chain' property is already fixed, i.e. is determined upon storage creation. Signed-off-by: Fiona Ebner --- src/PVE/Storage/LVMPlugin.pm | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm index 0416c9e..c884941 100644 --- a/src/PVE/Storage/LVMPlugin.pm +++ b/src/PVE/Storage/LVMPlugin.pm @@ -439,6 +439,25 @@ sub on_add_hook { return; } +sub on_update_hook_full { + my ($class, $storeid, $scfg, $update, $delete, $sensitive) = @_; + + if ( + $scfg->{'snapshot-as-volume-chain'} # currently set + && ( # and won't be set after update, because: + ( + defined($update->{'snapshot-as-volume-chain'}) + && !$update->{'snapshot-as-volume-chain'} + ) # explicitly set to disabled + || grep { $_ eq 'snapshot-as-volume-chain' } $delete->@* # or deleted + ) + ) { + my $images = $class->list_images($storeid, $scfg, undef, undef, undef); + die "$storeid - cannot disable 'snapshot-as-volume-chain' while a qcow2 image exists\n" + if grep { $_->{format} eq 'qcow2' } $images->@*; + } +} + sub parse_volname { my ($class, $volname) = @_; -- 2.47.3 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel