[pve-devel] [PATCH pve-guest-common 1/1] add pre/post-snapshot hooks

[Stefan Hanreich <s.hanreich@proxmox.com>]

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---

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