[pve-devel] [PATCH qemu-server 4/4] add unix socket support for NBD storage migration

[Fabian Grünbichler]

On March 11, 2020 11:44 am, Mira Limbeck wrote:
> The reuse of the tunnel, which we're opening to communicate with the target
> node and to forward the unix socket for the state migration, for the NBD unix
> socket requires adding support for an array of sockets to forward, not just a
> single one. We also have to change the $sock_addr variable to an array
> for the cleanup of the socket file as SSH does not remove the file.
> 
> To communicate to the target node the support of unix sockets for NBD
> storage migration, we're specifying an nbd_protocol_version which is set
> to 1. This version is then passed to the target node via STDIN. For this
> we always have to pass either the spice ticket followed by a newline, or
> just a newline if no spice ticket is defined. Otherwise the target side
> would assume the 'nbd_protocol_version: 1' line to be the spice ticket.
> 
> When the NBD server on the target node is started with a unix socket, we
> get a different line containing all the information required to start
> the drive-mirror. This contains the unix socket path used on the target node
> which we require for forwarding and cleanup.
> 
> Signed-off-by: Mira Limbeck <m.limbeck at proxmox.com>
> ---
>  PVE/QemuMigrate.pm | 32 +++++++++++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index e569a2c..a52746c 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -142,7 +142,10 @@ sub write_tunnel {
>  sub fork_tunnel {
>      my ($self, $tunnel_addr) = @_;
>  
> -    my @localtunnelinfo = defined($tunnel_addr) ? ('-L' , $tunnel_addr ) : ();
> +    my @localtunnelinfo = ();
> +    foreach my $addr (@$tunnel_addr) {
> +	push @localtunnelinfo, '-L', $addr;
> +    }
>  
>      my $cmd = [@{$self->{rem_ssh}}, '-o ExitOnForwardFailure=yes', @localtunnelinfo, '/usr/sbin/qm', 'mtunnel' ];
>  
> @@ -184,7 +187,7 @@ sub finish_tunnel {
>  
>      if ($tunnel->{sock_addr}) {
>  	# ssh does not clean up on local host
> -	my $cmd = ['rm', '-f', $tunnel->{sock_addr}]; #
> +	my $cmd = ['rm', '-f', @{$tunnel->{sock_addr}}]; #
>  	PVE::Tools::run_command($cmd);
>  
>  	# .. and just to be sure check on remote side
> @@ -594,10 +597,16 @@ sub phase2 {
>      }
>  
>      my $spice_port;
> +    my $tunnel_addr = [];
> +    my $sock_addr = [];
> +    # version > 0 for unix socket support
> +    my $nbd_protocol_version = 1;
> +    my $input = $spice_ticket ? "$spice_ticket\n" : "\n";
> +    $input .= "nbd_protocol_version: $nbd_protocol_version\n";
>  
>      # Note: We try to keep $spice_ticket secret (do not pass via command line parameter)
>      # instead we pipe it through STDIN
> -    my $exitcode = PVE::Tools::run_command($cmd, input => $spice_ticket, outfunc => sub {
> +    my $exitcode = PVE::Tools::run_command($cmd, input => $input, outfunc => sub {
>  	my $line = shift;
>  
>  	if ($line =~ m/^migration listens on tcp:(localhost|[\d\.]+|\[[\d\.:a-fA-F]+\]):(\d+)$/) {
> @@ -626,7 +635,18 @@ sub phase2 {
>  
>  	    $self->{target_drive}->{$targetdrive}->{drivestr} = $drivestr;
>  	    $self->{target_drive}->{$targetdrive}->{nbd_uri} = $nbd_uri;
> +	} elsif ($line =~ m!^storage migration listens on nbd:unix:(/run/qemu-server/(\d+)_nbd\.migrate):exportname=(\S+) volume:(\S+)$!) {
> +	    my $drivestr = $4;
> +	    die "Destination UNIX socket's VMID does not match source VMID" if $vmid ne $2;
> +	    my $nbd_unix_addr = $1;
> +	    my $nbd_uri = "nbd:unix:$nbd_unix_addr:exportname=$3";
> +	    my $targetdrive = $3;
> +	    $targetdrive =~ s/drive-//g;
>  
> +	    $self->{target_drive}->{$targetdrive}->{drivestr} = $drivestr;
> +	    $self->{target_drive}->{$targetdrive}->{nbd_uri} = $nbd_uri;
> +	    push @$tunnel_addr, "$nbd_unix_addr:$nbd_unix_addr";
> +	    push @$sock_addr, $nbd_unix_addr;
>  	} elsif ($line =~ m/^QEMU: (.*)$/) {
>  	    $self->log('info', "[$self->{node}] $1\n");
>  	}
> @@ -645,8 +665,9 @@ sub phase2 {
>  
>  	if ($ruri =~ /^unix:/) {
>  	    unlink $raddr;
> -	    $self->{tunnel} = $self->fork_tunnel("$raddr:$raddr");
> -	    $self->{tunnel}->{sock_addr} = $raddr;
> +	    push @$tunnel_addr, "$raddr:$raddr";
> +	    $self->{tunnel} = $self->fork_tunnel($tunnel_addr);
> +	    push @$sock_addr, $raddr;
>  
>  	    my $unix_socket_try = 0; # wait for the socket to become ready
>  	    while (! -S $raddr) {

are we sure we don't need to wait for all (UNIX) sockets to become ready 
here?

> @@ -678,6 +699,7 @@ sub phase2 {
>  	#fork tunnel for insecure migration, to send faster commands like resume
>  	$self->{tunnel} = $self->fork_tunnel();
>      }
> +    $self->{tunnel}->{sock_addr} = $sock_addr if (@$sock_addr);
>  
>      my $start = time();
>  
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>