From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id B777F9079A for ; Thu, 22 Sep 2022 13:54:32 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A10CF187AC for ; Thu, 22 Sep 2022 13:54:32 +0200 (CEST) Received: from lana.proxmox.com (unknown [94.136.29.99]) by firstgate.proxmox.com (Proxmox) with ESMTP for ; Thu, 22 Sep 2022 13:54:31 +0200 (CEST) Received: by lana.proxmox.com (Postfix, from userid 10043) id A25F42C21BD; Thu, 22 Sep 2022 13:54:25 +0200 (CEST) From: Stefan Hanreich To: pve-devel@lists.proxmox.com Date: Thu, 22 Sep 2022 13:54:21 +0200 Message-Id: <20220922115421.1406405-3-s.hanreich@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220922115421.1406405-1-s.hanreich@proxmox.com> References: <20220922115421.1406405-1-s.hanreich@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.244 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods NO_DNS_FOR_FROM 0.001 Envelope sender has no MX or A DNS records RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an 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. [abstractconfig.pm] Subject: [pve-devel] [PATCH pve-guest-common 1/1] add pre/post-snapshot hooks 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: , X-List-Received-Date: Thu, 22 Sep 2022 11:54:32 -0000 Signed-off-by: Stefan Hanreich --- I have opted to include the snapshot hooks in the abstract class, since this seemed like the more straightforward way. The other option would have been duplicating the calls of the hooks into the respective Backends, but I couldn't see any upsides to this. This hook runs before the preparation steps, since otherwise in case of a failing pre-snapshot hook the VM/CT is left in a locked state, which I would need to clean up, which would add unnecessary complexity imo. Otoh, there is a case to be made that the hook should only run after we checked every precondition and are as certain as we can be that the snapshot will be successfully created. This would be more convenient from a user's pov. This can be particularly convenient as it would avoid errors with user scripts that are not idempotent. Although those would still fail in case of a failure during the snapshotting process. What do you think would be the better solution? src/PVE/AbstractConfig.pm | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/PVE/AbstractConfig.pm b/src/PVE/AbstractConfig.pm index a0c0bc6..8052fde 100644 --- a/src/PVE/AbstractConfig.pm +++ b/src/PVE/AbstractConfig.pm @@ -799,11 +799,15 @@ sub __snapshot_activate_storages { sub snapshot_create { my ($class, $vmid, $snapname, $save_vmstate, $comment) = @_; + my $conf = $class->load_config($vmid); + PVE::GuestHelpers::exec_hookscript($conf, $vmid, "pre-snapshot", 1); + my $snap = $class->__snapshot_prepare($vmid, $snapname, $save_vmstate, $comment); $save_vmstate = 0 if !$snap->{vmstate}; - my $conf = $class->load_config($vmid); + # reload config after changes in snapshot preparation step + $conf = $class->load_config($vmid); my ($running, $freezefs) = $class->__snapshot_check_freeze_needed($vmid, $conf, $snap->{vmstate}); @@ -843,6 +847,8 @@ sub snapshot_create { } $class->__snapshot_commit($vmid, $snapname); + + PVE::GuestHelpers::exec_hookscript($conf, $vmid, "post-snapshot"); } # Check if the snapshot might still be needed by a replication job. -- 2.30.2