From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <t.lamprecht@proxmox.com>
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 B0A9691258
 for <pve-devel@lists.proxmox.com>; Mon, 26 Sep 2022 17:38:53 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 5CF73652A
 for <pve-devel@lists.proxmox.com>; Mon, 26 Sep 2022 17:38:53 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (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 firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pve-devel@lists.proxmox.com>; Mon, 26 Sep 2022 17:38:52 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id EDD3444435
 for <pve-devel@lists.proxmox.com>; Mon, 26 Sep 2022 17:38:51 +0200 (CEST)
Message-ID: <c96cd4a7-e35c-ede7-57c8-68e468949233@proxmox.com>
Date: Mon, 26 Sep 2022 17:38:50 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:106.0) Gecko/20100101
 Thunderbird/106.0
Content-Language: en-GB
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
 Stefan Hanreich <s.hanreich@proxmox.com>
References: <20220922141321.1510795-1-s.hanreich@proxmox.com>
 <20220922141321.1510795-5-s.hanreich@proxmox.com>
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
In-Reply-To: <20220922141321.1510795-5-s.hanreich@proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 1.858 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
 NICE_REPLY_A           -3.766 Looks like a legit reply (A)
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH qemu-server 1/1] Add VM hooks for
 pre/post-migrate on target/source
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Mon, 26 Sep 2022 15:38:53 -0000

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') {