Am 02.06.2016 um 17:06 schrieb Dietmar Maurer:
>> I can see the reason to use waitpid instead of check_process_running(),
>> but why do you change the rest of the code?
>>
>> Can we have minimal patches, where each patch states the reason for the
>> change
>> in the commit log?
>
> I thought about something like this:
>
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index a25efff..ce5774e 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -71,6 +71,8 @@ sub finish_command_pipe {
> $self->log('info', "ssh tunnel still running - terminating now with
> SIGKILL\n");
> kill 9, $cpid;
> sleep 1;
> +
> + waitpid($cpid); # avoid zombies
That doesn't fixes it :)
You have returns above so this statement is only reached if all fails
and the tunnel has to be killed, in normal cases the child does not get
selected.
Also the writer/reader may get double closed here, i thought about
something like
diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index a25efff..d869fa3 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -42,37 +42,53 @@ sub fork_command_pipe {
}
sub finish_command_pipe {
my ($self, $cmdpipe, $timeout) = @_;
+ my $cpid = $cmdpipe->{pid};
+ return if !defined($cpid);
+
my $writer = $cmdpipe->{writer};
my $reader = $cmdpipe->{reader};
$writer->close();
$reader->close();
- my $cpid = $cmdpipe->{pid};
-
+ my $waitpid;
if ($timeout) {
for (my $i = 0; $i < $timeout; $i++) {
- return if !PVE::ProcFSTools::check_process_running($cpid);
+ $waitpid = waitpid($cpid, WNOHANG);
+ if (defined($waitpid) && $waitpid == $cpid) {
+ delete $cmdpipe->{cpid};
+ return;
+ }
sleep(1);
}
}
$self->log('info', "ssh tunnel still running - terminating now with
SIGTERM\n");
kill(15, $cpid);
# wait again
for (my $i = 0; $i < 10; $i++) {
- return if !PVE::ProcFSTools::check_process_running($cpid);
+ $waitpid = waitpid($cpid, WNOHANG);
+ if (defined($waitpid) && $waitpid == $cpid) {
+ delete $cmdpipe->{cpid};
+ return;
+ }
sleep(1);
}
$self->log('info', "ssh tunnel still running - terminating now with
SIGKILL\n");
kill 9, $cpid;
sleep 1;
+
+ $waitpid = waitpid($cpid, WNOHANG);
+ $self->log('err', "Tunnel process (PID $cpid) could not be
collected after kill")
+ if (!(defined($waitpid) && $waitpid == $cpid));
+
+ delete $cmdpipe->{cpid};
}
> }
>
> sub fork_tunnel {
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
