From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <s.hanreich@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) server-digest SHA256)
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id DFB689138C
 for <pve-devel@lists.proxmox.com>; Tue, 27 Sep 2022 09:40:13 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id C9AA91BBD8
 for <pve-devel@lists.proxmox.com>; Tue, 27 Sep 2022 09:40:13 +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) server-digest SHA256)
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pve-devel@lists.proxmox.com>; Tue, 27 Sep 2022 09:40:12 +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 52E5344510
 for <pve-devel@lists.proxmox.com>; Tue, 27 Sep 2022 09:40:11 +0200 (CEST)
Message-ID: <054232e8-9398-224d-c78c-880319c6b64c@proxmox.com>
Date: Tue, 27 Sep 2022 09:40:10 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.0
Content-Language: en-US
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
 Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20220922141321.1510795-1-s.hanreich@proxmox.com>
 <20220922141321.1510795-5-s.hanreich@proxmox.com>
 <c96cd4a7-e35c-ede7-57c8-68e468949233@proxmox.com>
From: Stefan Hanreich <s.hanreich@proxmox.com>
In-Reply-To: <c96cd4a7-e35c-ede7-57c8-68e468949233@proxmox.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 1.874 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           -2.319 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
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [qm.pm, qemumigrate.pm, qemumigratemock.pm]
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: Tue, 27 Sep 2022 07:40:13 -0000



On 9/26/22 17:38, Thomas Lamprecht wrote:
> 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.

Makes sense, I will look into using mtunnel for this! Will also add a 
short note to the documentation.

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