[pve-devel] [RFC PATCH qemu-server] remove netcat6 dependency

[Dominik Csapak]

On 02/07/2017 12:18 PM, Wolfgang Bumiller wrote:
> By simply using a socket.
> ---
> We've already been trying to get rid of this construct for various reasons.
> The latest motivation was to get rid of the 'command nc6 gibberish failed'
> error message. My main motivation here is the fact that the netcat6 package
> is gone in stretch.
>
> This code is probably a candidate for PVE::Network, and to me this is the
> most straight forward way. (I really don't see the point in calling out to
> external tools for this task.)
> Dominik had looked into this as well, but I don't remember if he had any
> particular reason not to do this.
>
> Note that I moved the run_command() into the if branches. The upper branch
> cannot be used with websockets and I wasn't sure about the error message
> the few users of this code path get.
> I'm also not sure about the RFB protocol errors, I'm not getting any (and
> if I did I'd want to know?)
>
>  PVE/API2/Qemu.pm | 33 +++++++++++++++++++++++++--------
>  control.in       |  2 +-
>  2 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index a41e74f..d27127a 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -5,6 +5,8 @@ use warnings;
>  use Cwd 'abs_path';
>  use Net::SSLeay;
>  use UUID;
> +use POSIX;
> +use IO::Socket::IP;
>
>  use PVE::Cluster qw (cfs_read_file cfs_write_file);;
>  use PVE::SafeSyslog;
> @@ -1408,20 +1410,35 @@ __PACKAGE__->register_method({
>  		$cmd = ['/usr/bin/vncterm', '-rfbport', $port,
>  			'-timeout', $timeout, '-authpath', $authpath,
>  			'-perm', 'Sys.Console', '-c', @$remcmd, @$termcmd];
> +		PVE::Tools::run_command($cmd);
>  	    } else {
>
>  		$ENV{LC_PVE_TICKET} = $ticket if $websocket; # set ticket with "qm vncproxy"
>
> -		my $qmcmd = [@$remcmd, "/usr/sbin/qm", 'vncproxy', $vmid];
> -
> -		my $qmstr = join(' ', @$qmcmd);
> -
> -		# also redirect stderr (else we get RFB protocol errors)
> -		$cmd = ['/bin/nc6', '-l', '-p', $port, '-w', $timeout, '-e', "$qmstr 2>/dev/null"];
> +		$cmd = [@$remcmd, "/usr/sbin/qm", 'vncproxy', $vmid];
> +
> +		my $sock = IO::Socket::IP->new(
> +		    Listen => 1,
> +		    LocalPort => $port,
> +		    Proto => 'tcp',
> +		    GetAddrInfoFlags => 0,
> +		    ) or die "failed to create socket: $!\n";
> +		# Inside the worker we shouldn't have any previous alarms
> +		# running anyway...:
> +		alarm(0);
> +		local $SIG{ALRM} = sub { die "connection timed out\n" };
> +		alarm $timeout;
> +		accept(my $cli, $sock) or die "connection failed: $!\n";
> +		close($sock);
> +		if (PVE::Tools::run_command($cmd,
> +		    output => '>&'.fileno($cli),
> +		    input => '<&'.fileno($cli),
> +		    noerr => 1) != 0)
> +		{
> +		    die "Failed to run vncproxy.\n";
> +		}
>  	    }
>
> -	    PVE::Tools::run_command($cmd);
> -
>  	    return;
>  	};
>
> diff --git a/control.in b/control.in
> index 896d8b5..eafa7f5 100644
> --- a/control.in
> +++ b/control.in
> @@ -3,7 +3,7 @@ Version: @@VERSION@@-@@PKGRELEASE@@
>  Section: admin
>  Priority: optional
>  Architecture: @@ARCH@@
> -Depends: libc6 (>= 2.7-18), perl (>= 5.10.0-19), libterm-readline-gnu-perl, pve-qemu-kvm (>= 2.2-1), netcat6, libpve-access-control, libpve-storage-perl, pve-cluster, libjson-perl, libjson-xs-perl, libio-multiplex-perl, libnet-ssleay-perl, socat, pve-firewall, libuuid-perl, pve-ha-manager, dbus, libpve-common-perl, libpve-guest-common-perl
> +Depends: libc6 (>= 2.7-18), perl (>= 5.10.0-19), libterm-readline-gnu-perl, pve-qemu-kvm (>= 2.2-1), libpve-access-control, libpve-storage-perl, pve-cluster, libjson-perl, libjson-xs-perl, libio-multiplex-perl, libnet-ssleay-perl, socat, pve-firewall, libuuid-perl, pve-ha-manager, dbus, libpve-common-perl, libpve-guest-common-perl
>  Maintainer: Proxmox Support Team <support at proxmox.com>
>  Description: Qemu Server Tools
>   This package contains the Qemu Server tools used by Proxmox VE
>

this is a way better approach than my socat/stderr parsing thing, very 
good :)
what i would like to see is that the stderr output from the (remote) qm 
vncproxy call gets into the task log instead of "failed to run vncproxy" 
because this may be to little information, getting the cause of the 
failure would be much better