[pve-devel] r5694 - pve-access-control/trunk/PVE

svn-commits at proxmox.com svn-commits at proxmox.com
Mon Mar 14 10:35:15 CET 2011


Author: dietmar
Date: 2011-03-14 10:35:15 +0100 (Mon, 14 Mar 2011)
New Revision: 5694

Modified:
   pve-access-control/trunk/PVE/RPCEnvironment.pm
Log:
cleanup worker code


Modified: pve-access-control/trunk/PVE/RPCEnvironment.pm
===================================================================
--- pve-access-control/trunk/PVE/RPCEnvironment.pm	2011-03-14 09:34:30 UTC (rev 5693)
+++ pve-access-control/trunk/PVE/RPCEnvironment.pm	2011-03-14 09:35:15 UTC (rev 5694)
@@ -6,6 +6,7 @@
 use IO::File;
 use Fcntl qw(:flock);
 use PVE::SafeSyslog;
+use PVE::Tools;
 use PVE::INotify;
 use PVE::Cluster;
 use PVE::ProcFSTools;
@@ -267,63 +268,10 @@
     return $self->{user};
 }
 
-# UPID helper
-# WARN: $res->{filename} must not depend on PID, because we 
-# use it before we know the PID
 
-sub upid_decode {
-    my $upid = shift;
-
-    my $res;
-
-    # "UPID:$node:$pid:$start:$type:$data"
-    if ($upid =~ m/^UPID:(\w+):(\d+)(-(\d+))?:(\d+):([^:\s]+):(.*)$/) {
-	$res->{node} = $1;
-	$res->{pid} = $2;
-	$res->{pstart} = $4 || 0;
-	$res->{starttime} = $5;
-	$res->{type} = $6;
-	$res->{data} = $7;
-
-	if ($res->{type} eq 'vmops') {
-	    if ($res->{data} =~ m/^([^:\s]+):(\d+):(\S+)$/) {
-		$res->{command} = $1;
-		$res->{veid} = $2;
-		$res->{user} = $3;
-
-		$res->{filename} = "/tmp/vmops-$res->{veid}.out";
-	    } else {
-		return undef;
-	    }
-	} elsif ($res->{type} eq 'apldownload') {
-	    if ($res->{data} =~ m/^([^:\s]+):(.+)$/) {
-		$res->{user} = $1;
-		$res->{apl} = $2;
-		$res->{filename} = "/tmp/apldownload-$res->{user}.out";
-	    } else {
-		return undef;
-	    }	
-	}
-    }
-
-    return $res;
-}
-
-sub upid_encode {
-    my $uip_hash = shift;
-
-    my $d = $uip_hash; # shortcut
-
-    return "UPID:$d->{node}:$d->{pid}-$d->{pstart}:$d->{starttime}:$d->{type}:$d->{data}";
-}
-
 # start long running workers
-# $data append to the returned uniquely identifier, which
-# has the following format: "UPID:$node:$pid-$pstart:$startime:$dtype:$data"
 # STDIN is redirected to /dev/null
 # STDOUT,STDERR are redirected to the filename returned by upid_decode
-# that file is locked wit flock to make sure only one process 
-# is writing it
 
 sub fork_worker {
     my ($self, $dtype, $data, $function) = @_;
@@ -338,36 +286,18 @@
 
     my $node = $self->{nodename};
 
-    # detect filename with faked PID
-    my $tmp = upid_decode ("UPID:$node:0-0:0:$dtype:$data"); 
-    my $filename = $tmp->{filename};
+    my $cpid = fork();
+    die "unable to fork worker - $!" if !defined($cpid);
 
-    my $lockfh;
-    # lock output file
-    if ($filename) {
+    my $workerpuid = $cpid ? $cpid : $$;
 
-	$lockfh = IO::File->new ($filename, O_WRONLY|O_CREAT) ||
-	    die "unable to open output file - $!\n";
+    my $pstart = PVE::ProcFSTools::read_proc_starttime($workerpuid) ||
+	die "unable to read process start time";
 
-	my $wwwid = getpwnam('www-data');	    
-	chown $wwwid,  $filename;
+    my $upid = PVE::Tools::upid_encode ({
+	node => $node, pid => $workerpuid, pstart => $pstart, 
+	starttime => $starttime, type => $dtype, data => $data });
 
-	if (!flock ($lockfh, LOCK_EX|LOCK_NB)) {
-	    undef $lockfh; # close
-	    die "unable to lock output file\n";
-	}
-
-	if (!truncate ($lockfh, 0)) {
-	    die "unable to truncate output file - $!\n";
-	}
-    }
-
-    my $cpid = fork();
-    if (!defined($cpid)) {
-	undef $lockfh; # close
-	die "unable to fork worker - $!";
-    }
-
     if ($cpid == 0) {
 
 	$SIG{INT} = $SIG{QUIT} = $SIG{TERM} = sub { die "received interrupt\n"; };
@@ -380,67 +310,69 @@
 
 	POSIX::close ($psync[0]);
 
-	PVE::INotify::inotify_close();
+	my $outfh;
 
-	if (my $atfork = $self->{atfork}) {
-	    &$atfork();
-	}
+	eval {
+	    PVE::INotify::inotify_close();
 
-	# same algorythm as used inside SA
+	    if (my $atfork = $self->{atfork}) {
+		&$atfork();
+	    }
 
-	# STDIN = /dev/null
-	my $fd = fileno (STDIN);
-	close STDIN;
-	POSIX::close(0) if $fd != 0;
+	    # same algorythm as used inside SA
 
-	if (!open (STDIN, "</dev/null")) {
-	    POSIX::_exit (1); 
-	    kill ('KILL', $$); 
-	}
+	    # STDIN = /dev/null
+	    my $fd = fileno (STDIN);
+	    close STDIN;
+	    POSIX::close(0) if $fd != 0;
 
-	# redirect STDOUT
-	$fd = fileno(STDOUT);
-	close STDOUT;
-	POSIX::close (1) if $fd != 1;
+	    die "unable to redirect STDIN - $!" 
+		if !open(STDIN, "</dev/null");
 
-	if ($filename) {
-	    if (!open (STDOUT, ">&", $lockfh)) {
-		POSIX::_exit (1); 
-		kill ('KILL', $$); 
-	    }
+	    # redirect STDOUT
+	    $fd = fileno(STDOUT);
+	    close STDOUT;
+	    POSIX::close (1) if $fd != 1;
 
+	    my $tmp = PVE::Tools::upid_decode($upid); 
+	    my $filename = $tmp->{filename};
+
+	    $outfh = IO::File->new ($filename, O_WRONLY|O_CREAT|O_EXCL) ||
+		die "unable to open output file - $!\n";
+
+	    my $wwwid = getpwnam('www-data');	    
+	    chown $wwwid,  $outfh;
+
+	    die "unable to redirect STDOUT - $!" 
+		if !open(STDOUT, ">&", $outfh);
+
 	    STDOUT->autoflush (1);
-	} else {
-	    if (!open (STDOUT, ">/dev/null")) {
-		POSIX::_exit (1); 
-		kill ('KILL', $$); 
-	    }
-	}
       
-	#  redirect STDERR to STDOUT
-	$fd = fileno (STDERR);
-	close STDERR;
-	POSIX::close(2) if $fd != 2;
+	    #  redirect STDERR to STDOUT
+	    $fd = fileno (STDERR);
+	    close STDERR;
+	    POSIX::close(2) if $fd != 2;
 
-	if (!open (STDERR, ">&1")) {
-	    POSIX::_exit (1); 
-	    kill ('KILL', $$); 
+	    die "unable to redirect STDERR - $!" 
+		if !open(STDERR, ">&1");
+	    
+	    STDERR->autoflush(1);
+	};
+	if (my $err = $@) {
+	    my $msg =  "ERROR: $err";
+	    print STDERR "$msg\n";
+	    STDERR->flush();
+	    POSIX::write($psync[1], $msg, length ($msg));
+	    POSIX::close($psync[1]);
+	    POSIX::_exit(1); 
+	    kill('KILL', $$); 
 	}
-	
-	STDERR->autoflush (1);
 
-	my $pstart = PVE::ProcFSTools::read_proc_starttime ($$) ||
-	    die "unable to read process starttime";
-
-	my $upid = upid_encode ({
-	    node => $node, pid => $$, pstart => $pstart, 
-	    starttime => $starttime, type => $dtype, data => $data });
-
 	# sync with parent
-	POSIX::write ($psync[1], $upid, length ($upid));
-	POSIX::close ($psync[1]);
+	POSIX::write($psync[1], $upid, length ($upid));
+	POSIX::close($psync[1]);
 
-	eval { &$function ($upid); };
+	eval { &$function($upid); };
 	my $err = $@;
 	if ($err) {
 	    my $msg = "worker function failed: $err";
@@ -450,28 +382,28 @@
 	} else {
 	    POSIX::_exit (0);
 	} 
-	kill ('KILL', $$); 
-   }
+	kill('KILL', $$); 
+    }
 
     POSIX::close ($psync[1]);
 
     # sync with child (wait until child starts)
-    my $upid = '';
-    POSIX::read($psync[0], $upid, 4096);
+    my $readbuf = '';
+    POSIX::read($psync[0], $readbuf, 4096);
     POSIX::close ($psync[0]);
 
-    undef $lockfh; # close
-
     &$register_worker($cpid);
 
-    my $uh = upid_decode ($upid);
-    if (!$uh || 
-	!($uh->{node} eq $node && $uh->{pid} == $cpid && 
-	  $uh->{starttime} == $starttime &&
-	  $uh->{type} eq $dtype && $uh->{data} eq $data)) {
-	die "got strange worker upid '$upid'\n";
+    die "got no worker upid - start worker failed\n" if !$readbuf;
+
+    if ($readbuf =~ m/^ERROR:\s*(.+)$/m) {
+	die "starting worker failed: $1\n";
     }
 
+    if ($readbuf ne $upid) {
+	die "got strange worker upid ('$readbuf' != '$upid') - start worker failed\n";
+    }
+
     return $upid;
 }
 




More information about the pve-devel mailing list