[pve-devel] [PATCH qemu-server 1/4] parse nbd_protocol_version if available

[Fabian Grünbichler]

nit: this should be ordered after #2 - first implement without breakage, 
then expose new feature. not first expose new-feature as no-op, then 
implement new feature ;)

On March 11, 2020 11:44 am, Mira Limbeck wrote:
> For secure live migration with local disks via NBD over a unix socket,
> we have to somehow communicate support for it from the source node to the
> target node. This is because there can only be one NBD server with either a TCP
> or Unix socket bound, not both.
> 
> The source node passes that information via STDIN after the spice
> ticket followed by a newline for backwards compatibility. If there is no
> spice ticket we just pass the newline followed by the
> 'nbd_protocol_version: <version>' line.
> 
> For unix socket support we parse the line 'nbd_protocol_version:
> <version>'. A version >= 1 means unix socket support. We use a fallback
> to 0 should there be no line containing that pattern which is the case
> if the source node has an older qemu-server version.
> 
> Signed-off-by: Mira Limbeck <m.limbeck at proxmox.com>
> ---
>  PVE/API2/Qemu.pm | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 3b61334..8891db9 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -2035,10 +2035,17 @@ __PACKAGE__->register_method({
>  
>  	# read spice ticket from STDIN
>  	my $spice_ticket;
> +	my $nbd_protocol_version = 0;
>  	if ($stateuri && ($stateuri eq 'tcp' || $stateuri eq 'unix') && $migratedfrom && ($rpcenv->{type} eq 'cli')) {
>  	    if (defined(my $line = <STDIN>)) {
>  		chomp $line;
>  		$spice_ticket = $line;
> +		while (defined(my $line = <STDIN>)) {
> +		    chomp $line;
> +		    if ($line =~ m/^nbd_protocol_version: (\d+)$/) {
> +			$nbd_protocol_version = $1;
> +		    }
> +		}

could be switched to 

if $line =~ nbd ... {

} elsif $line =~ ^spice_ticket: (...)$ {
  $spice_ticket = $1
} else {
  $spice_ticket = $line
}

switching spice ticket to a prefixed variant (and keeping the unprefixed 
default == spice_ticket until 7.0). we don't need to assume that the 
spice_ticket is the first line and un-prefixed forever that way.

probably depends on whether other parameters are added like this as well 
(likely for the storage mapping mechanism we are currently discussing 
w.r.t. cross-cluster migration, same for network mapping/targetbridge).

I intend to refactor vm_start anyway, so I can also clean this up then 
since I'll need to touch this anyway.

>  	    }
>  	}
>  
> @@ -2067,7 +2074,8 @@ __PACKAGE__->register_method({
>  		syslog('info', "start VM $vmid: $upid\n");
>  
>  		PVE::QemuServer::vm_start($storecfg, $vmid, $stateuri, $skiplock, $migratedfrom, undef, $machine,
> -					  $spice_ticket, $migration_network, $migration_type, $targetstorage, $timeout);
> +					  $spice_ticket, $migration_network, $migration_type, $targetstorage, $timeout,
> +					  $nbd_protocol_version);
>  		return;
>  	    };
>  
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>