[pve-devel] [PATCH pve-container/pve-docs/qemu-server 0/1] Add pre/post-restore hooks

[pve-devel] [PATCH pve-container/pve-docs/qemu-server 0/1] Add pre/post-restore hooks

[Stefan Hanreich]

This patch adds hooks that run when the user restores a backup from the Web UI
/ CLI. I have tested this with both VMs/CTs via Web UI and CLI. Are there any
other places where the hook should get triggered that I missed?

pve-container:

Stefan Hanreich (1):
  Add pre/post-restore hooks to CTs

 src/PVE/API2/LXC.pm | 6 ++++++
 1 file changed, 6 insertions(+)

qemu-server:

Stefan Hanreich (1):
  Add pre/post-restore hooks to VMs

 PVE/API2/Qemu.pm | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

pve-docs:

Stefan Hanreich (1):
  add pre/post-restore events to example hookscript

 examples/guest-example-hookscript.pl | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

-- 
2.30.2

[pve-devel] [PATCH pve-docs 1/1] add pre/post-restore events to example hookscript

[Stefan Hanreich]

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

Similar to the snapshot patch series, I have refrained from continuing the
numbering. This should be harmonized in a follow-up patch.

 examples/guest-example-hookscript.pl | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/examples/guest-example-hookscript.pl b/examples/guest-example-hookscript.pl
index adeed59..19fe213 100755
--- a/examples/guest-example-hookscript.pl
+++ b/examples/guest-example-hookscript.pl
@@ -54,6 +54,20 @@ if ($phase eq 'pre-start') {
 
     print "$vmid stopped. Doing cleanup.\n";
 
+} elsif ($phase eq 'pre-restore') {
+
+    # Phase 'pre-restore' will be executed before restoring a backup. (via UI or
+    # CLI)
+
+    print "Restoring $vmid from backup.\n";
+
+} elsif ($phase eq 'post-restore') {
+
+    # Phase 'pre-restore' will be after before restoring a backup. (via UI or
+    # CLI)
+
+    print "$vmid finished restoring from backup.\n";
+
 } else {
     die "got unknown phase '$phase'\n";
 }
-- 
2.30.2

[pve-devel] [PATCH pve-container 1/1] Add pre/post-restore hooks to CTs

[Stefan Hanreich]

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/API2/LXC.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 589f96f..3ecf5e5 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -376,6 +376,8 @@ __PACKAGE__->register_method({
 	    eval {
 		my $orig_mp_param; # only used if $restore
 		if ($restore) {
+		    PVE::GuestHelpers::exec_hookscript($old_conf, $vmid, 'pre-restore', 1);
+
 		    die "can't overwrite running container\n" if PVE::LXC::check_running($vmid);
 		    if ($archive ne '-') {
 			my $orig_conf;
@@ -502,6 +504,10 @@ __PACKAGE__->register_method({
 
 	    PVE::API2::LXC::Status->vm_start({ vmid => $vmid, node => $node })
 		if $start_after_create;
+
+	    if ($restore) {
+		PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'post-restore');
+	    }
 	};
 
 	my $workername = $restore ? 'vzrestore' : 'vzcreate';
-- 
2.30.2

[pve-devel] [PATCH qemu-server 1/1] Add pre/post-restore hooks to VMs

[Stefan Hanreich]

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

There might be a better way to differentiate the different errors from
restorefn in the error handling logic, although I think in this case it is
still fine. This might get a bit messy though if in the future someone adds
another source for errors. Maybe add a single if before the restored_data checks
instead?

 PVE/API2/Qemu.pm | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 3ec31c2..fe41634 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -884,9 +884,13 @@ __PACKAGE__->register_method({
 	die "$emsg $@" if $@;
 
 	my $restored_data = 0;
+	my $hook_executed = 0;
 	my $restorefn = sub {
 	    my $conf = PVE::QemuConfig->load_config($vmid);
 
+	    PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'pre-restore', 1);
+	    $hook_executed = 1;
+
 	    PVE::QemuConfig->check_protection($conf, $emsg);
 
 	    die "$emsg vm is running\n" if PVE::QemuServer::check_running($vmid);
@@ -918,6 +922,8 @@ __PACKAGE__->register_method({
 		    eval { PVE::QemuServer::template_create($vmid, $restored_conf) };
 		    warn $@ if $@;
 		}
+
+		PVE::GuestHelpers::exec_hookscript($restored_conf, $vmid, 'post-restore');
 	    };
 
 	    # ensure no old replication state are exists
@@ -1012,10 +1018,10 @@ __PACKAGE__->register_method({
 		if (my $err = $@) {
 		    eval { PVE::QemuConfig->remove_lock($vmid, 'create') };
 		    warn $@ if $@;
-		    if ($restored_data) {
+		    if ($hook_executed && $restored_data) {
 			warn "error after data was restored, VM disks should be OK but config may "
 			    ."require adaptions. VM $vmid state is NOT cleaned up.\n";
-		    } else {
+		    } elsif ($hook_executed && !$restored_data) {
 			warn "error before or during data restore, some or all disks were not "
 			    ."completely restored. VM $vmid state is NOT cleaned up.\n";
 		    }
-- 
2.30.2

Re: [pve-devel] [PATCH pve-container 1/1] Add pre/post-restore hooks to CTs

[Wolfgang Bumiller]

On Thu, Sep 22, 2022 at 03:19:42PM +0200, Stefan Hanreich wrote:
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  src/PVE/API2/LXC.pm | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 589f96f..3ecf5e5 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -376,6 +376,8 @@ __PACKAGE__->register_method({
>  	    eval {
>  		my $orig_mp_param; # only used if $restore
>  		if ($restore) {
> +		    PVE::GuestHelpers::exec_hookscript($old_conf, $vmid, 'pre-restore', 1);
> +
>  		    die "can't overwrite running container\n" if PVE::LXC::check_running($vmid);

I think this check should happen before the hook.

>  		    if ($archive ne '-') {
>  			my $orig_conf;
> @@ -502,6 +504,10 @@ __PACKAGE__->register_method({
>  
>  	    PVE::API2::LXC::Status->vm_start({ vmid => $vmid, node => $node })
>  		if $start_after_create;
> +
> +	    if ($restore) {
> +		PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'post-restore');
> +	    }
>  	};
>  
>  	my $workername = $restore ? 'vzrestore' : 'vzcreate';
> -- 
> 2.30.2

Re: [pve-devel] [PATCH qemu-server 1/1] Add pre/post-restore hooks to VMs

[Wolfgang Bumiller]

On Thu, Sep 22, 2022 at 03:19:43PM +0200, Stefan Hanreich wrote:
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> 
> There might be a better way to differentiate the different errors from
> restorefn in the error handling logic, although I think in this case it is
> still fine. This might get a bit messy though if in the future someone adds
> another source for errors. Maybe add a single if before the restored_data checks
> instead?
> 
>  PVE/API2/Qemu.pm | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 3ec31c2..fe41634 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -884,9 +884,13 @@ __PACKAGE__->register_method({
>  	die "$emsg $@" if $@;
>  
>  	my $restored_data = 0;
> +	my $hook_executed = 0;
>  	my $restorefn = sub {
>  	    my $conf = PVE::QemuConfig->load_config($vmid);
>  
> +	    PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'pre-restore', 1);
> +	    $hook_executed = 1;
> +
>  	    PVE::QemuConfig->check_protection($conf, $emsg);

This should probalby happen before the hook?

>  
>  	    die "$emsg vm is running\n" if PVE::QemuServer::check_running($vmid);
> @@ -918,6 +922,8 @@ __PACKAGE__->register_method({
>  		    eval { PVE::QemuServer::template_create($vmid, $restored_conf) };
>  		    warn $@ if $@;
>  		}
> +
> +		PVE::GuestHelpers::exec_hookscript($restored_conf, $vmid, 'post-restore');
>  	    };
>  
>  	    # ensure no old replication state are exists
> @@ -1012,10 +1018,10 @@ __PACKAGE__->register_method({
>  		if (my $err = $@) {
>  		    eval { PVE::QemuConfig->remove_lock($vmid, 'create') };
>  		    warn $@ if $@;
> -		    if ($restored_data) {
> +		    if ($hook_executed && $restored_data) {
>  			warn "error after data was restored, VM disks should be OK but config may "
>  			    ."require adaptions. VM $vmid state is NOT cleaned up.\n";
> -		    } else {
> +		    } elsif ($hook_executed && !$restored_data) {
>  			warn "error before or during data restore, some or all disks were not "
>  			    ."completely restored. VM $vmid state is NOT cleaned up.\n";
>  		    }
> -- 
> 2.30.2