both STDOUT and STDERR are written into `$info` which is then parsed for
IP and port of the target socket listening.
when the ports file can't be locked immediately `trying to acquire
lock...` is printed on STDERR and in turn written into `$info`.
trying to parse the IP then fails, resulting in a migration or
replication failing.

the bare open3 call is replaced by the run_command wrapper from
pve-common to use a safe wrapper around open3 with the same
functionality.
STDERR is now read separately from STDOUT and the last line of STDERR
is kept in case of errors.

Signed-off-by: Mira Limbeck <m.limb...@proxmox.com>
---
I've replaced open3 with run_command on recommendation from others. tt
complicates the logic a little bit compared to the `simple` open3
solution, but may be less error prone since run_command abstracts open3
nicely behind a safe wrapper.

one thing I could not verify was the now removed line:
`if (!close($info)) { # does waitpid()`

I could not find any information mentioning this behavior of close() on
a simple file.


 src/PVE/Storage.pm | 79 +++++++++++++++++++++++++---------------------
 1 file changed, 43 insertions(+), 36 deletions(-)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 40314a8..b6045d5 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -851,44 +851,51 @@ sub storage_migrate {
 
     eval {
        if ($insecure) {
-           my $input = IO::File->new();
-           my $info = IO::File->new();
-           open3($input, $info, $info, @$recv)
-               or die "receive command failed: $!\n";
-           close($input);
-
-           my $try_ip = <$info> // '';
-           my ($ip) = $try_ip =~ /^($PVE::Tools::IPRE)$/ # untaint
-               or die "no tunnel IP received, got '$try_ip'\n";
-
-           my $try_port = <$info> // '';
-           my ($port) = $try_port =~ /^(\d+)$/ # untaint
-               or die "no tunnel port received, got '$try_port'\n";
-
-           my $socket = IO::Socket::IP->new(PeerHost => $ip, PeerPort => 
$port, Type => SOCK_STREAM)
-               or die "failed to connect to tunnel at $ip:$port\n";
-           # we won't be reading from the socket
-           shutdown($socket, 0);
-
-           eval { run_command($cmds, output => '>&'.fileno($socket), errfunc 
=> $match_volid_and_log); };
-           my $send_error = $@;
-
-           # don't close the connection entirely otherwise the receiving end
-           # might not get all buffered data (and fails with 'connection reset 
by peer')
-           shutdown($socket, 1);
-
-           # wait for the remote process to finish
-           while (my $line = <$info>) {
-               $match_volid_and_log->("[$target_sshinfo->{name}] $line");
+           my $last_err = '';
+           my $ip;
+           my $port;
+           my $socket;
+           my $send_error;
+           my $handle_insecure_migration = sub {
+               my $line = shift;
+
+               if (!$ip) {
+                   if ($line =~ /^($PVE::Tools::IPRE)$/) {
+                       $ip = $1;
+                   } else {
+                       die "no tunnel IP received, got $line\n";
+                   }
+               } elsif(!$port) {
+                   if ($line =~ /^(\d+)$/) {
+                       $port = $1;
+                   } else {
+                       die "no tunnel port received, got $line\n";
+                   }
+               }
+               if ($ip && $port && !$socket) {
+                   # create socket, run command
+                   $socket = IO::Socket::IP->new(PeerHost => $ip, PeerPort => 
$port, Type => SOCK_STREAM)
+                       or die "failed to connect to tunnel at $ip:$port\n";
+                   # we won't be reading from the socket
+                   shutdown($socket, 0);
+
+                   eval { run_command($cmds, output => '>&'.fileno($socket), 
errfunc => $match_volid_and_log); };
+                   $send_error = $@;
+
+                   # don't close the connection entirely otherwise the 
receiving end
+                   # might not get all buffered data (and fails with 
'connection reset by peer')
+                   shutdown($socket, 1);
+               } elsif ($ip && $port) {
+                   $match_volid_and_log->("[$target_sshinfo->{name}] $line");
+               }
+           };
+           eval { run_command($recv, outfunc => $handle_insecure_migration, 
errfunc => sub { $last_err = shift; }); };
+           if (my $err = $@) {
+               chomp($err);
+               die "failed to run insecure migration: $err - $last_err\n";
            }
-
            # now close the socket
-           close($socket);
-           if (!close($info)) { # does waitpid()
-               die "import failed: $!\n" if $!;
-               die "import failed: exit code ".($?>>8)."\n";
-           }
-
+           close($socket) if $socket;
            die $send_error if $send_error;
        } else {
            push @$cmds, $recv;
-- 
2.39.2


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

Reply via email to