Re: [pve-devel] [PATCH qemu-server 1/1] Add VM hooks for pre/post-migrate on target/source

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

I don't like that there's no commit message (the cover letter is more for general/meta
info, it doesn't gets into git after all, would require doing pull requests which we
(currently) don't do in most cases).

Besides that I'd rather avoid extending SSH usage further, a long term goal is to
allow user running PVE clusters with all its features without any SSH connection
required. For VM migration we already got the mtunnel which can be used for such
control stuff; otherwise one can get an API client instead, but there we'd need to
make some sort of execution path part of the public API, can be fine; as the CLI
really isn't /that/ different from the API in publicity/stableness regard (if its
there it will be used, a description comment is a very weak gate), so I'd find a
fully internal handling (i.e., neither CLI nor API exposure) nicer, mtunnel should
make that relatively simple, for CTs one may need to add a similar mechanism.

Also, this requires the script to be available on all notes, the docs should reflect
that.

Am 22/09/2022 um 16:13 schrieb Stefan Hanreich:
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  PVE/CLI/qm.pm                         | 50 +++++++++++++++++++++++++++
>  PVE/QemuMigrate.pm                    | 23 ++++++++++++
>  test/MigrationTest/QemuMigrateMock.pm |  4 +++
>  3 files changed, 77 insertions(+)
> 
> diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
> index 6a2e161..4161e6e 100755
> --- a/PVE/CLI/qm.pm
> +++ b/PVE/CLI/qm.pm
> @@ -838,6 +838,54 @@ __PACKAGE__->register_method({
>  	return;
>      }});
>  
> +__PACKAGE__->register_method ({
> +    name => 'pre_migrate',
> +    path => 'pre_migrate',
> +    method => 'POST',
> +    description => "Run pre-migrate hook for VM <vmid> - do not use manually.",
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
> +	    'source-node' => get_standard_option('pve-node'),
> +	},
> +    },
> +    returns => { type => 'null'},
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $vmid = $param->{vmid};
> +	my $source_node = $param->{'source-node'};
> +	my $conf = PVE::QemuConfig->load_config($vmid, $source_node);
> +
> +	PVE::GuestHelpers::exec_hookscript($conf, $vmid, "pre-migrate-target", 1);
> +
> +	return;
> +    }});
> +
> +__PACKAGE__->register_method ({
> +    name => 'post_migrate',
> +    path => 'post_migrate',
> +    method => 'POST',
> +    description => "Run post-migrate hook for VM <vmid> - do not use manually.",
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
> +	},
> +    },
> +    returns => { type => 'null'},
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $vmid = $param->{vmid};
> +	my $conf = PVE::QemuConfig->load_config($vmid);
> +
> +	PVE::GuestHelpers::exec_hookscript($conf, $vmid, "post-migrate-target");
> +
> +	return;
> +    }});
> +
>  my $print_agent_result = sub {
>      my ($data) = @_;
>  
> @@ -988,6 +1036,8 @@ our $cmddef = {
>  	    }],
>      },
>  
> +    'pre-migrate' => [ __PACKAGE__, 'pre_migrate', ['vmid', 'source-node']],
> +    'post-migrate' => [ __PACKAGE__, 'post_migrate', ['vmid']],
>  };
>  
>  1;
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index d52dc8d..b113dec 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -1284,4 +1284,27 @@ sub round_powerof2 {
>      return 2 << int(log($_[0]-1)/log(2));
>  }
>  
> +sub pre_migration_hooks {
> +    my ($self, $vmid) = @_;
> +
> +    PVE::GuestHelpers::exec_hookscript($self->{vmconf}, $vmid, 'pre-migrate-source', 1);
> +
> +    my $node = PVE::INotify::nodename();
> +    my $cmd = [ @{$self->{rem_ssh}}, "qm", "pre-migrate", $vmid, $node ];
> +
> +    eval { $self->cmd($cmd); };
> +    die "$@\n" if $@;
> +}
> +
> +sub post_migration_hooks {
> +    my ($self, $vmid) = @_;
> +
> +    PVE::GuestHelpers::exec_hookscript($self->{vmconf}, $vmid, 'post-migrate-source');
> +
> +    my $cmd = [ @{$self->{rem_ssh}}, "qm", "post-migrate", $vmid ];
> +
> +    eval { $self->cmd($cmd); };
> +    die "$@\n" if $@;
> +}
> +
>  1;
> diff --git a/test/MigrationTest/QemuMigrateMock.pm b/test/MigrationTest/QemuMigrateMock.pm
> index f2c0281..461b390 100644
> --- a/test/MigrationTest/QemuMigrateMock.pm
> +++ b/test/MigrationTest/QemuMigrateMock.pm
> @@ -298,6 +298,10 @@ $MigrationTest::Shared::tools_module->mock(
>  			return 0;
>  		    } elsif ($cmd eq 'stop') {
>  			return 0;
> +		    } elsif ($cmd eq 'pre-migrate') {
> +			return 0;
> +		    } elsif ($cmd eq 'post-migrate') {
> +			return 0;
>  		    }
>  		    die "run_command (mocked) ssh qm command - implement me: ${cmd_msg}";
>  		} elsif ($cmd eq 'pvesm') {