fix #4872: run env: use run_command() for country detection

This fixes a rather longstanding issue [0][1] with the country
detection, in that it might get completely stuck and thus hangs the
installation.

This is due how Perl, signals and line reading interacts.

A minimal reproducer, how the installer currently works, looks like
this:
```
    #!/usr/bin/env perl

    use strict;
    use warnings;

    open (my $fh, '-|', 'sleep', '1000') or die;

    my $prev = alarm(2);
    eval {
	local $SIG{ALRM} = sub { die "timed out!\n" };

	my $line;
	while (defined ($line = <$fh>)) {
	    print "line: $line";
	}
    };

    alarm($prev);
    close($fh);
```

One might expect that this times out after 2 seconds, as specified in
`alarm(2)`. The thruth is that `$line = <$fh>` apparently prevents the
signal to go through. This then causes the installer to hang there
indefinitely, if `traceroute` never progresses - which seems to happen
on lots of (weird) networks, as evidently can be seen in the forum [1].

Proxmox::Sys::Command::run_command() handles of these weird cases, takes
care of the nitty-gritty details and - most importantly - interacts
properly with SIGALRM, so just use that instead.

This _should_ really fix that issue, but reproducing it 1:1 as part of
the installation process is _very_ hard, basically pure luck. But
rewriting the reproducer using run_command (in the exact same way that
this patch rewrites detect_country_tracing_to()) fixes the issue there,
so it's the best we can probably do.

NB: This causes that the traceroute command is now printed to the log
(as run_command() logs that by default), which we could also hide e.g.
through another parameter if wanted.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=4872
[1] https://forum.proxmox.com/threads/proxmox-installation-trying-to-detect-country.134301/

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>

diff --git a/Proxmox/Install/RunEnv.pm b/Proxmox/Install/RunEnv.pm
index c393f679cb8d5c8bb23da81644451f32e48079f7..d7ccea6510c2a8cb2d66d708148d6ef417465404 100644 (file)
--- a/Proxmox/Install/RunEnv.pm
+++ b/Proxmox/Install/RunEnv.pm
@@ -7,6 +7,7 @@ use Carp;
 use JSON qw(from_json to_json);
 
 use Proxmox::Log;
+use Proxmox::Sys::Command qw(run_command CMD_FINISHED);
 use Proxmox::Sys::File qw(file_read_firstline);
 use Proxmox::Sys::Block;
 use Proxmox::Sys::Net;
@@ -188,35 +189,36 @@ my sub detect_country_tracing_to : prototype($$) {
     my ($ipver, $destination) = @_;
 
     print STDERR "trying to detect country...\n";
-    open(my $TRACEROUTE_FH, '-|', 'traceroute', "-$ipver", '-N', '1', '-q', '1', '-n', $destination)
-       or return undef;
 
+    my $traceroute_cmd = ['traceroute', "-$ipver", '-N', '1', '-q', '1', '-n', $destination];
     my $geoip_bin = ($ipver == 6) ? 'geoiplookup6' : 'geoiplookup';
 
     my $country;
-
-    my $previous_alarm = alarm (10);
+    my $previous_alarm;
     eval {
        local $SIG{ALRM} = sub { die "timed out!\n" };
-       my $line;
-       while (defined ($line = <$TRACEROUTE_FH>)) {
+       $previous_alarm = alarm (10);
+
+       run_command($traceroute_cmd, sub {
+           my $line = shift;
+
            log_debug("DC TRACEROUTE: $line");
            if ($line =~ m/^\s*\d+\s+(\S+)\s/) {
                my $geoip = qx/$geoip_bin $1/;
                log_debug("DC GEOIP: $geoip");
+
                if ($geoip =~ m/GeoIP Country Edition:\s*([A-Z]+),/) {
                    $country = lc ($1);
                    log_info("DC FOUND: $country\n");
-                   last;
+                   return CMD_FINISHED;
                }
            }
-       }
+       }, undef, undef, 1);
+
     };
     my $err = $@;
     alarm ($previous_alarm);
 
-    close($TRACEROUTE_FH);
-
     if ($err) {
        die "unable to detect country - $err\n";
     } elsif ($country) {