[pve-devel] [PATCH] migration : enable mtunnel for insecure migration

[Alexandre DERUMIER]

Thanks for the review Thomas,
I'll work on it this week.


----- Mail original -----
De: "Thomas Lamprecht" <t.lamprecht at proxmox.com>
À: "pve-devel" <pve-devel at pve.proxmox.com>, "aderumier" <aderumier at odiso.com>
Envoyé: Jeudi 17 Août 2017 11:56:08
Objet: Re: [pve-devel] [PATCH] migration : enable mtunnel for insecure migration

the base idea is good, IMO, some comments in line, 

On 08/10/2017 09:15 AM, Alexandre Derumier wrote: 
> We only use it to send commands faster like resume 
> 
> Signed-off-by: Alexandre Derumier <aderumier at odiso.com> 
> --- 
> PVE/QemuMigrate.pm | 8 ++++++++ 
> 1 file changed, 8 insertions(+) 
> 
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm 
> index fc847cc..8271adf 100644 
> --- a/PVE/QemuMigrate.pm 
> +++ b/PVE/QemuMigrate.pm 
> @@ -635,6 +635,14 @@ sub phase2 { 
> } else { 
> die "unsupported protocol in migration URI: $ruri\n"; 
> } 
> + } else { 
> + #fork tunnel for insecure migration, to send faster commands like resume 
> + $self->log('info', "start remote tunnel"); 

I'd rather move the log line from the if (secure) branch out so that we 
have this line only once. 

> + my $pfamily = PVE::Tools::get_host_address_family($nodename); 
> + my $lport = PVE::Tools::next_migrate_port($pfamily); 
> + my $tunnel_addr = "$lport:localhost:$rport"; 
> + 

I would omit those three lines above, its not needed for the insecure 
case, the $tunnel_addr parameter from fork tunnel is optional. 

> + $self->{tunnel} = $self->fork_tunnel($tunnel_addr); 

use: 

$self->{tunnel} = $self->fork_tunnel(); 

instead. 

> } 
> 
> my $start = time(); 
> 

I.e., a minified (slightly tested) version of your patch could be: 

------8<------ 

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm 
index fc847cc..7f39138 100644 
--- a/PVE/QemuMigrate.pm 
+++ b/PVE/QemuMigrate.pm 
@@ -601,9 +601,9 @@ sub phase2 { 

die "unable to detect remote migration address\n" if !$raddr; 

- if ($migration_type eq 'secure') { 
- $self->log('info', "start remote tunnel"); 
+ $self->log('info', "start remote tunnel"); 

+ if ($migration_type eq 'secure') { 
if ($ruri =~ /^unix:/) { 
unlink $raddr; 
$self->{tunnel} = $self->fork_tunnel("$raddr:$raddr"); 
@@ -635,6 +635,9 @@ sub phase2 { 
} else { 
die "unsupported protocol in migration URI: $ruri\n"; 
} 
+ } else { 
+ # use mtunnel to send commands without the ssh/perl startup delay 
+ $self->{tunnel} = $self->fork_tunnel(); 
} 

my $start = time(); 


------>8------