[pve-devel] [PATCH common 1/2] fork_worker: refactor child exit

[Stoiko Ivanov]

On Tue, Nov 20, 2018 at 09:56:03AM +0100, Thomas Lamprecht wrote:
> On 11/20/18 9:46 AM, Stoiko Ivanov wrote:
> > * factor out duplicated code.
> > * drop final kill(-9, $$), since the POSIX:_exit call does not return
> >   (see man 2 _exit).
> 
> that's known, this is more like a safety measure to guarantee better sleep
> for a developer, and it does no harm... If you remove it do I'd maybe do it
> in a separate patch this does not belong in a patch themed with refactoring,
> IMO. Looks OK besides that nit.
It just struck me after putting the POSIX::_exit right on top of it (and
reading the manpages). But I do see the point, that it's a safety-measure
against potential future refactorings. I'll send a v2 with the kill still
in place, unless you do not mind to fix it up while applying?

Thanks for the review!
> 
> > 
> > Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
> > ---
> >  src/PVE/RESTEnvironment.pm | 17 ++++++++---------
> >  1 file changed, 8 insertions(+), 9 deletions(-)
> > 
> > diff --git a/src/PVE/RESTEnvironment.pm b/src/PVE/RESTEnvironment.pm
> > index 69af004..c2fea2b 100644
> > --- a/src/PVE/RESTEnvironment.pm
> > +++ b/src/PVE/RESTEnvironment.pm
> > @@ -579,22 +579,21 @@ sub fork_worker {
> >  	    }
> >  	    &$function($upid);
> >  	};
> > +	my ($msg, $exitcode);
> >  	my $err = $@;
> >  	if ($err) {
> >  	    chomp $err;
> >  	    $err =~ s/\n/ /mg;
> >  	    syslog('err', $err);
> > -	    my $msg = "TASK ERROR: $err\n";
> > -	    POSIX::write($resfh, $msg, length($msg));
> > -	    POSIX::close($resfh) if $sync;
> > -	    POSIX::_exit(-1);
> > +	    $msg = "TASK ERROR: $err\n";
> > +	    $exitcode = -1;
> >  	} else {
> > -	    my $msg = "TASK OK\n";
> > -	    POSIX::write($resfh, $msg, length($msg));
> > -	    POSIX::close($resfh) if $sync;
> > -	    POSIX::_exit(0);
> > +	    $msg = "TASK OK\n";
> > +	    $exitcode = 0;
> >  	}
> > -	kill(-9, $$);
> > +	POSIX::write($resfh, $msg, length($msg));
> > +	POSIX::close($resfh) if $sync;
> > +	POSIX::_exit($exitcode);
> >      }
> >  
> >      # parent
> > 
>