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 <supp...@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


_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to