From f91072d56feb9c27e5777a7eb39449baf4b2dd01 Mon Sep 17 00:00:00 2001 From: Dietmar Maurer Date: Thu, 11 Apr 2013 08:17:34 +0200 Subject: [PATCH] code cleanups --- PVE/APIDaemon.pm | 58 ++++++---------------- PVE/HTTPServer.pm | 124 +++++++++++++++++++++++----------------------- bin/pvedaemon | 3 +- bin/pveproxy | 3 +- 4 files changed, 80 insertions(+), 108 deletions(-) diff --git a/PVE/APIDaemon.pm b/PVE/APIDaemon.pm index 71732a6d..7eabf0cf 100755 --- a/PVE/APIDaemon.pm +++ b/PVE/APIDaemon.pm @@ -2,25 +2,17 @@ package PVE::APIDaemon; use strict; use warnings; -use vars qw(@ISA); +use POSIX ":sys_wait_h"; use IO::Socket::INET; use PVE::SafeSyslog; -use PVE::INotify; -use PVE::RPCEnvironment; use PVE::HTTPServer; -use POSIX qw(EINTR); -use POSIX ":sys_wait_h"; -use IO::Handle; -use IO::Select; -use JSON; - my $workers = {}; sub new { my ($this, %args) = @_; - + my $class = ref($this) || $this; die "no lockfile" if !$args{lockfile}; @@ -43,7 +35,7 @@ sub new { $cfg->{lockfh} = $lockfh; $cfg->{max_workers} = 3 if !$cfg->{max_workers}; $cfg->{trusted_env} = 0 if !defined($cfg->{trusted_env}); - + return $self; } @@ -52,9 +44,9 @@ sub worker_finished { syslog('info', "worker $cpid finished"); } - + sub finish_workers { - local $!; local $?; + local $!; local $?; foreach my $cpid (keys %$workers) { my $waitpid = waitpid ($cpid, WNOHANG); if (defined($waitpid) && ($waitpid == $cpid)) { @@ -75,7 +67,7 @@ sub test_workers { } sub start_workers { - my ($self, $rpcenv) = @_; + my ($self) = @_; my $count = 0; foreach my $cpid (keys %$workers) { @@ -104,10 +96,8 @@ sub start_workers { $SIG{TERM} = $SIG{QUIT} = 'DEFAULT'; # we handle that with AnyEvent eval { - # try to init inotify - # fixme: poll - PVE::INotify::inotify_init(); - $self->handle_connections($rpcenv); + my $server = PVE::HTTPServer->new(%{$self->{cfg}}); + $server->run(); }; if (my $err = $@) { syslog('err', $err); @@ -130,7 +120,7 @@ sub terminate_server { my $previous_alarm = alarm (10); eval { local $SIG{ALRM} = sub { die "timeout\n" }; - + while ((my $pid = waitpid (-1, 0)) > 0) { if (defined($workers->{$pid})) { delete ($workers->{$pid}); @@ -140,11 +130,11 @@ sub terminate_server { alarm(0); # avoid race condition }; my $err = $@; - + alarm ($previous_alarm); if ($err) { - syslog('err', "error stopping workers (will kill them now) - $err"); + syslog('err', "error stopping workers (will kill them now) - $err"); foreach my $cpid (keys %$workers) { # KILL childs still alive! if (kill (0, $cpid)) { @@ -159,10 +149,6 @@ sub terminate_server { sub start_server { my $self = shift; - my $atfork = sub { close($self->{cfg}->{socket}); }; - my $rpcenv = PVE::RPCEnvironment->init( - $self->{cfg}->{trusted_env} ? 'priv' : 'pub', atfork => $atfork); - eval { my $old_sig_chld = $SIG{CHLD}; local $SIG{CHLD} = sub { @@ -171,11 +157,11 @@ sub start_server { }; my $old_sig_term = $SIG{TERM}; - local $SIG{TERM} = sub { + local $SIG{TERM} = sub { terminate_server (); &$old_sig_term(@_) if $old_sig_term; }; - local $SIG{QUIT} = sub { + local $SIG{QUIT} = sub { terminate_server(); &$old_sig_term(@_) if $old_sig_term; }; @@ -188,8 +174,8 @@ sub start_server { }; for (;;) { # forever - $self->start_workers($rpcenv); - sleep (5); + $self->start_workers(); + sleep (5); $self->test_workers(); } }; @@ -200,18 +186,4 @@ sub start_server { } } -sub send_error { - my ($c, $code, $msg) = @_; - - $c->send_response(HTTP::Response->new($code, $msg)); -} - -sub handle_connections { - my ($self, $rpcenv) = @_; - - my $server = PVE::HTTPServer->new(%{$self->{cfg}}, rpcenv => $rpcenv); - - $server->run(); -} - 1; diff --git a/PVE/HTTPServer.pm b/PVE/HTTPServer.pm index 1d09f683..33cb7ef9 100755 --- a/PVE/HTTPServer.pm +++ b/PVE/HTTPServer.pm @@ -30,7 +30,6 @@ use CGI; # fixme: remove this! $CGI::DISABLE_UPLOADS = 1; # no uploads $CGI::POST_MAX = 1024 * 10; # max 10K posts -use Scalar::Util qw/weaken/; # fixme: remove? use Data::Dumper; # fixme: remove my $known_methods = { @@ -69,7 +68,7 @@ sub log_aborted_request { if ($error) { syslog("err", "problem with client $reqstate->{peer_host}; $error"); } - + $self->log_request($reqstate); } @@ -87,12 +86,12 @@ sub client_do_disconnect { shutdown($hdl->{fh}, 1); # clear all handlers - $hdl->on_drain(undef); + $hdl->on_drain(undef); $hdl->on_read(undef); $hdl->on_eof(undef); $self->{conn_count}--; - #print "$$: client_do_disconnect $self->{conn_count} $hdl\n"; + print "$$: CLOSE FH" . $hdl->{fh}->fileno() . " CONN$self->{conn_count}\n" if $self->{debug}; } sub finish_response { @@ -106,15 +105,15 @@ sub finish_response { if (!$self->{end_loop} && $reqstate->{keep_alive} > 0) { # print "KEEPALIVE $reqstate->{keep_alive}\n"; - $hdl->on_read(sub { + $hdl->on_read(sub { eval { $self->push_request_header($reqstate); }; warn $@ if $@; }); } else { $hdl->on_drain (sub { - eval { - $self->client_do_disconnect($reqstate); - }; + eval { + $self->client_do_disconnect($reqstate); + }; warn $@ if $@; }); } @@ -140,7 +139,7 @@ sub response { $reqstate->{log}->{code} = $code; my $res = "HTTP/1.0 $code $msg\015\012"; - + my $ctime = time(); my $date = HTTP::Date::time2str($ctime); $resp->header('Date' => $date); @@ -155,12 +154,7 @@ sub response { $resp->header('Server' => "pve-api-daemon/3.0"); my $content_length; - if (ref($content) eq "CODE") { - $reqstate->{keep_alive} = 0; - - # fixme: - - } elsif ($content) { + if ($content) { $content_length = length($content); @@ -172,10 +166,11 @@ sub response { } $resp->header("Content-Length" => $content_length); $reqstate->{log}->{content_length} = $content_length; + } else { $resp->remove_header("Content-Length"); } - + if ($reqstate->{keep_alive} > 0) { $resp->push_header('Connection' => 'Keep-Alive'); } else { @@ -183,7 +178,7 @@ sub response { } $res .= $resp->headers_as_string("\015\012"); - #print "SEND(supress content) $res\n"; + #print "SEND(without content) $res\n" if $self->{debug}; $res .= "\015\012"; $res .= $content; @@ -198,7 +193,7 @@ sub error { my ($self, $reqstate, $code, $msg, $hdr, $content) = @_; eval { - my $resp = HTTP::Response->new($code, $msg, $hdr, $content); + my $resp = HTTP::Response->new($code, $msg, $hdr, $content); $self->response($reqstate, $resp); }; warn $@ if $@; @@ -209,14 +204,14 @@ sub send_file_start { eval { # print "SEND FILE $filename\n"; - # Note: aio_load() this is not really async unless we use IO::AIO! + # Note: aio_load() this is not really async unless we use IO::AIO! eval { my $fh = IO::File->new($filename, '<') || die "$!\n"; my $stat = File::stat::stat($fh) || die "$!\n"; - + my $data; my $len = sysread($fh, $data, $stat->size); die "got short file\n" if !defined($len) || $len != $stat->size; @@ -224,27 +219,27 @@ sub send_file_start { my $ct; if ($filename =~ m/\.css$/) { $ct = 'text/css'; - } elsif ($filename =~ m/\.js$/) { + } elsif ($filename =~ m/\.js$/) { $ct = 'application/javascript'; - } elsif ($filename =~ m/\.png$/) { + } elsif ($filename =~ m/\.png$/) { $ct = 'image/png'; - } elsif ($filename =~ m/\.gif$/) { + } elsif ($filename =~ m/\.gif$/) { $ct = 'image/gif'; - } elsif ($filename =~ m/\.jar$/) { + } elsif ($filename =~ m/\.jar$/) { $ct = 'application/java-archive'; } else { die "unable to detect content type"; } my $header = HTTP::Headers->new(Content_Type => $ct); - my $resp = HTTP::Response->new(200, "OK", $header, $data); + my $resp = HTTP::Response->new(200, "OK", $header, $data); $self->response($reqstate, $resp, $stat->mtime); }; if (my $err = $@) { $self->error($reqstate, 501, $err); } }; - + warn $@ if $@; } @@ -254,9 +249,9 @@ sub proxy_request { eval { my $target; if ($host eq 'localhost') { - $target = "http://$host:85$abs_uri"; + $target = "http://$host:85$abs_uri"; } else { - $target = "https://$host:8006$abs_uri"; + $target = "https://$host:8006$abs_uri"; } my $headers = { @@ -283,19 +278,17 @@ sub proxy_request { } } - # fixme: tls_ctx; - my $w; $w = http_request( - $method => $target, - headers => $headers, - timeout => 30, - resurse => 0, - body => $content, + $method => $target, + headers => $headers, + timeout => 30, + resurse => 0, + body => $content, sub { my ($body, $hdr) = @_; undef $w; - + eval { my $code = delete $hdr->{Status}; my $msg = delete $hdr->{Reason}; @@ -361,24 +354,24 @@ sub handle_api2_request { my $clientip = $headers->header('PVEClientIP'); - $rpcenv->init_request(params => $params); + $rpcenv->init_request(params => $params); my $res = PVE::REST::rest_handler($rpcenv, $clientip, $method, $path, $rel_uri, $ticket, $token); - # fixme: eval { $userid = $rpcenv->get_user(); }; + # todo: eval { $userid = $rpcenv->get_user(); }; my $userid = $rpcenv->{user}; # this is faster $rpcenv->set_user(undef); # clear after request $reqstate->{log}->{userid} = $userid; - + if ($res->{proxy}) { if ($self->{trusted_env}) { $self->error($reqstate, HTTP_INTERNAL_SERVER_ERROR, "proxy not allowed"); return; - } + } - $self->proxy_request($reqstate, $r, $clientip, $res->{proxy}, $method, + $self->proxy_request($reqstate, $r, $clientip, $res->{proxy}, $method, $r->uri, $ticket, $token, $res->{proxy_params}); return; @@ -391,7 +384,7 @@ sub handle_api2_request { $resp->header("Content-Type" => $ct); $resp->content($raw); $self->response($reqstate, $resp); - + return; }; warn $@ if $@; @@ -415,7 +408,7 @@ sub handle_request { return; } - if ($path =~ m!/api2!) { + if ($path =~ m!/api2!) { $self->handle_api2_request($reqstate); return; } @@ -436,7 +429,7 @@ sub handle_request { die "internal error - no handler"; } return; - } + } if ($self->{dirs} && ($method eq 'GET')) { # we only allow simple names @@ -491,7 +484,7 @@ sub unshift_read_header { my $len = $r->header('Content-Length'); my $pveclientip = $r->header('PVEClientIP'); - # fixme: + # fixme: how can we make PVEClientIP header trusted? if ($self->{trusted_env} && $pveclientip) { $reqstate->{peer_host} = $pveclientip; } else { @@ -536,7 +529,7 @@ sub push_request_header { eval { #print "got request header: $line\n"; - + $reqstate->{keep_alive}--; if ($line =~ /(\S+)\040(\S+)\040HTTP\/(\d+)\.(\d+)/o) { @@ -549,7 +542,7 @@ sub push_request_header { $self->{request_count}++; # only count valid request headers if ($self->{request_count} >= $self->{max_requests}) { - $self->{end_loop} = 1; + $self->{end_loop} = 1; } $reqstate->{log} = { requestline => $line }; $reqstate->{proto}->{maj} = $maj; @@ -609,12 +602,10 @@ sub accept { die $errmsg if $errmsg; } - fh_nonblocking $clientfh, 1; + fh_nonblocking $clientfh, 1; $self->{conn_count}++; - print "$$: ACCEPT OK $self->{conn_count} FH" . $clientfh->fileno() . "\n"; - return $clientfh; } @@ -624,7 +615,7 @@ sub wait_end_loop { $self->{end_loop} = 1; undef $self->{socket_watch}; - + if ($self->{conn_count} <= 0) { $self->{end_cond}->send(1); return; @@ -633,7 +624,7 @@ sub wait_end_loop { # else we need to wait until all open connections gets closed my $w; $w = AnyEvent->timer (after => 1, interval => 1, cb => sub { eval { - # fixme: test for active connections instead? + # todo: test for active connections instead (we can abort idle connections) if ($self->{conn_count} <= 0) { undef $w; $self->{end_cond}->send(1); @@ -642,7 +633,7 @@ sub wait_end_loop { warn $@ if $@; }); } - + sub accept_connections { my ($self) = @_; @@ -670,7 +661,7 @@ sub accept_connections { }; if (my $err = $@) { syslog('err', $err); } }, - on_error => sub { + on_error => sub { my ($hdl, $fatal, $message) = @_; eval { $self->log_aborted_request($reqstate, $message); @@ -680,7 +671,7 @@ sub accept_connections { }, ($self->{tls_ctx} ? (tls => "accept", tls_ctx => $self->{tls_ctx}) : ())); - print "$$: ACCEPT OK $reqstate->{hdl} $self->{conn_count}\n"; + print "$$: ACCEPT FH" . $clientfh->fileno() . " CONN$self->{conn_count}\n" if $self->{debug}; $self->push_request_header($reqstate); } @@ -696,7 +687,7 @@ sub accept_connections { # Note: We can't open log file in non-blocking mode and use AnyEvent::Handle, # because we write from multiple processes, and that would arbitrarily mix output -# of all processes. +# of all processes. sub open_access_log { my ($self, $filename) = @_; @@ -729,12 +720,19 @@ sub new { my $class = ref($this) || $this; - foreach my $req (qw(rpcenv socket lockfh lockfile)) { + foreach my $req (qw(socket lockfh lockfile)) { die "misssing required argument '$req'" if !defined($args{$req}); } my $self = bless { %args }, $class; + # init inotify + PVE::INotify::inotify_init(); + + my $atfork = sub { close($self->{socket}); }; + $self->{rpcenv} = PVE::RPCEnvironment->init( + $self->{trusted_env} ? 'priv' : 'pub', atfork => $atfork); + fh_nonblocking($self->{socket}, 1); $self->{end_loop} = 0; @@ -748,11 +746,11 @@ sub new { $self->{end_cond} = AnyEvent->condvar; if ($self->{ssl}) { - $self->{tls_ctx} = AnyEvent::TLS->new(%{$self->{ssl}}); + $self->{tls_ctx} = AnyEvent::TLS->new(%{$self->{ssl}}); } $self->open_access_log($self->{logfile}) if $self->{logfile}; - + $self->{socket_watch} = AnyEvent->io(fh => $self->{socket}, poll => 'r', cb => sub { eval { if ($self->{conn_count} >= $self->{max_conn}) { @@ -764,7 +762,7 @@ sub new { }); } else { $self->accept_connections(); - } + } }; warn $@ if $@; }); @@ -774,11 +772,15 @@ sub new { $self->wait_end_loop(); }); - $self->{quit_watch} = AnyEvent->signal(signal => "QUIT", cb => sub { + $self->{quit_watch} = AnyEvent->signal(signal => "QUIT", cb => sub { undef $self->{quit_watch}; $self->wait_end_loop(); }); + $self->{inotify_poll} = AnyEvent->timer(after => 5, interval => 5, cb => sub { + PVE::INotify::poll(); # read inotify events + }); + return $self; } diff --git a/bin/pvedaemon b/bin/pvedaemon index 218e440c..75928ce8 100755 --- a/bin/pvedaemon +++ b/bin/pvedaemon @@ -34,8 +34,6 @@ $SIG{'__WARN__'} = sub { $0 = "pvedaemon"; -PVE::APIDaemon::enable_debug() if $opt_debug; - # create dir for dtach sockets mkdir "/var/run/dtach"; @@ -47,6 +45,7 @@ eval { port => 85, trusted_env => 1, # partly trusted, because only local programs can connect lockfile => $lockfile, + debug => $opt_debug, keep_alive => 100, max_conn => 500, max_requests => 1000); diff --git a/bin/pveproxy b/bin/pveproxy index a94bf60b..6c274471 100755 --- a/bin/pveproxy +++ b/bin/pveproxy @@ -52,8 +52,6 @@ POSIX::setuid($uid) || die "setuid $uid failed - $!\n"; # just to be sure die "detected strange uid/gid\n" if !($UID == $uid && $EUID == $uid && $GID eq "$gid $gid" && $EGID eq "$gid $gid"); -PVE::APIDaemon::enable_debug() if $opt_debug; - sub add_dirs { my ($result_hash, $alias, $subdir) = @_; @@ -86,6 +84,7 @@ eval { keep_alive => 100, max_conn => 500, max_requests => 1000, + debug => $opt_debug, trusted_env => 0, # not trusted, anyone can connect logfile => '/var/log/pveproxy/access.log', lockfile => $lockfile,