[pve-devel] [PATCH v5 qemu-server 08/11] migrate: refactor remote VM/tunnel start

[Fabian Grünbichler]

On February 11, 2022 2:01 pm, Fabian Ebner wrote:
> Am 09.02.22 um 14:07 schrieb Fabian Grünbichler:
>> no semantic changes intended, except for:
>> - no longer passing the main migration UNIX socket to SSH twice for
>> forwarding
>> - dropping the 'unix:' prefix in start_remote_tunnel's timeout error message
>> 
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
>>
>> [..]
>>
>> @@ -82,23 +87,23 @@ sub start_remote_tunnel {
>>  	    if ($unix_socket_try > 100) {
>>  		$self->{errors} = 1;
>>  		PVE::Tunnel::finish_tunnel($self->{tunnel});
>> -		die "Timeout, migration socket $ruri did not get ready";
>> +		die "Timeout, migration socket $tunnel_info->{addr} did not get ready";
>>  	    }
>>  	    $self->{tunnel}->{unix_sockets} = $unix_sockets if (@$unix_sockets);
>>  
>> -	} elsif ($ruri =~ /^tcp:/) {
>> +	} elsif ($tunnel_info->{proto} eq 'tcp') {
>>  	    my $ssh_forward_info = [];
>> -	    if ($raddr eq "localhost") {
>> +	    if ($tunnel_info->{addr} eq "localhost") {
>>  		# for backwards compatibility with older qemu-server versions
>>  		my $pfamily = PVE::Tools::get_host_address_family($nodename);
>>  		my $lport = PVE::Tools::next_migrate_port($pfamily);
>> -		push @$ssh_forward_info, "$lport:localhost:$rport";
>> +		push @$ssh_forward_info, "$lport:localhost:$tunnel_info->{rport}";
> 
> Should be $tunnel_info->{port}

right! never triggered since AFAICT this is dead code. we switched to 
using unix sockets for SSH migration in 2016/PVE 4[0], and this was just 
the fallback for compat reasons. unless I am missing something this can 
probably just be dropped altogether/replaced with a

die "secure TCP migration not supported\n";

0: https://git.proxmox.com/?p=qemu-server.git;a=commitdiff;h=1c9d54bfd05e0d017a6e2ac5524d75466b1a4455

source node will always use unix for secure and tcp for insecure
target node will only use 'localhost' as addr for tcp + secure, which no 
PVE 5/6/7 source node will ever set

> 
>>  	    }
>>  
>>  	    $self->{tunnel} = $self->fork_tunnel($ssh_forward_info);
>>  
>>  	} else {
>> -	    die "unsupported protocol in migration URI: $ruri\n";
>> +	    die "unsupported protocol in migration URI: $tunnel_info->{proto}\n";
>>  	}
>>      } else {
>>  	#fork tunnel for insecure migration, to send faster commands like resume
>> @@ -650,52 +655,40 @@ sub phase1_cleanup {
>>      }
>>  }
>>  
>> -sub phase2 {
>> -    my ($self, $vmid) = @_;
>> +sub phase2_start_local_cluster {
>> +    my ($self, $vmid, $params) = @_;
>>  
>>      my $conf = $self->{vmconf};
>>      my $local_volumes = $self->{local_volumes};
>>      my @online_local_volumes = $self->filter_local_volumes('online');
>>  
>>      $self->{storage_migration} = 1 if scalar(@online_local_volumes);
>> +    my $start = $params->{start_params};
>> +    my $migrate = $params->{migrate_opts};
>>  
>>      $self->log('info', "starting VM $vmid on remote node '$self->{node}'");
>>  
>> -    my $raddr;
>> -    my $rport;
>> -    my $ruri; # the whole migration dst. URI (protocol:address[:port])
>> -    my $nodename = PVE::INotify::nodename();
>> +    my $tunnel_info = {};
>>  
>>      ## start on remote node
>>      my $cmd = [@{$self->{rem_ssh}}];
>>  
>> -    my $spice_ticket;
>> -    if (PVE::QemuServer::vga_conf_has_spice($conf->{vga})) {
>> -	my $res = mon_cmd($vmid, 'query-spice');
>> -	$spice_ticket = $res->{ticket};
>> -    }
>> +    push @$cmd, 'qm', 'start', $vmid, '--skiplock';
> 
> Nit: the parameter $start->{skiplock} that's passed in is ignored
> (although it is always 1 currently)

fixed.

> 
>> +    push @$cmd, '--migratedfrom', $migrate->{migratedfrom};
>>  
>> -    push @$cmd , 'qm', 'start', $vmid, '--skiplock', '--migratedfrom', $nodename;
>> +    push @$cmd, '--migration_type', $migrate->{type};
>>  
>> -    my $migration_type = $self->{opts}->{migration_type};
>> +    push @$cmd, '--migration_network', $migrate->{network}
>> +      if $migrate->{network};
>>  
>> -    push @$cmd, '--migration_type', $migration_type;
>> +    push @$cmd, '--stateuri', $start->{statefile};
>>  
>> -    push @$cmd, '--migration_network', $self->{opts}->{migration_network}
>> -      if $self->{opts}->{migration_network};
>> -
>> -    if ($migration_type eq 'insecure') {
>> -	push @$cmd, '--stateuri', 'tcp';
>> -    } else {
>> -	push @$cmd, '--stateuri', 'unix';
>> +    if ($start->{forcemachine}) {
>> +	push @$cmd, '--machine', $start->{forcemachine};
>>      }
>>  
>> -    if ($self->{forcemachine}) {
>> -	push @$cmd, '--machine', $self->{forcemachine};
>> -    }
>> -
>> -    if ($self->{forcecpu}) {
>> -	push @$cmd, '--force-cpu', $self->{forcecpu};
>> +    if ($start->{forcecpu}) {
>> +	push @$cmd, '--force-cpu', $start->{forcecpu};
>>      }
>>  
>>      if ($self->{storage_migration}) {
>