From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 3FE2F7535A for ; Wed, 21 Apr 2021 15:25:25 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3D6A4FC36 for ; Wed, 21 Apr 2021 15:25:25 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 301E8FC2C for ; Wed, 21 Apr 2021 15:25:24 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 088D745B8F for ; Wed, 21 Apr 2021 15:25:24 +0200 (CEST) Date: Wed, 21 Apr 2021 15:25:16 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20210421111539.29261-1-s.reiter@proxmox.com> <20210421111539.29261-8-s.reiter@proxmox.com> In-Reply-To: <<20210421111539.29261-8-s.reiter@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1619010604.kigx9lk11q.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [anyevent.pm, proxmox.com, metacpan.org] Subject: Re: [pve-devel] [PATCH http-server 07/10] support streaming data form fh to client X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Apr 2021 13:25:25 -0000 some nits inline, looks good to me otherwise! On April 21, 2021 1:15 pm, Stefan Reiter wrote: > Use an explicit AnyEvent::Handle similar to websocket proxying. >=20 > Needs some special care to make sure we apply backpressure correctly to > avoid caching too much data. Note that because of AnyEvent restrictions, > specifying a "fh" to point to a file or a packet-based socket may result > in unwanted behaviour[0]. >=20 > [0]: https://metacpan.org/pod/AnyEvent::Handle#DESCRIPTION >=20 > Signed-off-by: Stefan Reiter > --- > PVE/APIServer/AnyEvent.pm | 97 +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 93 insertions(+), 4 deletions(-) >=20 > diff --git a/PVE/APIServer/AnyEvent.pm b/PVE/APIServer/AnyEvent.pm > index 60a2a1c..643ae88 100644 > --- a/PVE/APIServer/AnyEvent.pm > +++ b/PVE/APIServer/AnyEvent.pm > @@ -189,7 +189,7 @@ sub finish_response { > } > =20 > sub response { > - my ($self, $reqstate, $resp, $mtime, $nocomp, $delay) =3D @_; > + my ($self, $reqstate, $resp, $mtime, $nocomp, $delay, $stream_fh) = =3D @_; > =20 > #print "$$: send response: " . Dumper($resp); > =20 > @@ -231,7 +231,7 @@ sub response { > $resp->header('Server' =3D> "pve-api-daemon/3.0"); > =20 > my $content_length; > - if ($content) { > + if ($content && !$stream_fh) { > =20 > $content_length =3D length($content); > =20 > @@ -258,11 +258,93 @@ sub response { > #print "SEND(without content) $res\n" if $self->{debug}; > =20 > $res .=3D "\015\012"; > - $res .=3D $content if $content; > + $res .=3D $content if $content && !$stream_fh; > =20 > $self->log_request($reqstate, $reqstate->{request}); > =20 > - if ($delay && $delay > 0) { > + if ($stream_fh) { nit the code inside these braces might be worthy of becoming its own sub? > + # write headers and preamble > + $reqstate->{hdl}->push_write($res); > + > + # then attach an AnyEvent::Handle to pass through the data > + my $buf_size =3D 4*1024*1024; > + > + my $on_read; > + $on_read =3D sub { > + my ($hdl) =3D @_; > + my $reqhdl =3D $reqstate->{hdl}; > + return if !$reqhdl; > + > + my $wbuf_len =3D length($reqhdl->{wbuf}); > + my $rbuf_len =3D length($hdl->{rbuf}); > + # TODO: Take into account $reqhdl->{wbuf_max} ? Right now > + # that's unbounded, so just assume $buf_size > + my $to_read =3D $buf_size - $wbuf_len; > + $to_read =3D $rbuf_len if $rbuf_len < $to_read; > + if ($to_read > 0) { > + my $data =3D substr($hdl->{rbuf}, 0, $to_read, ''); > + $reqhdl->push_write($data); > + $rbuf_len -=3D $to_read; > + } elsif ($hdl->{_eof}) { > + # workaround: AnyEvent gives us a fake EPIPE if we don't consume > + # any data when called at EOF, so unregister ourselves - data is > + # flushed by on_eof anyway > + # see: https://sources.debian.org/src/libanyevent-perl/7.170-2/lib/Any= Event/Handle.pm/#L1329 > + $hdl->on_read(); > + return; > + } > + > + # apply backpressure so we don't accept any more data into > + # buffer if the client isn't downloading fast enough > + # note: read_size can double upon read, and we also need to > + # account for one more read after start_read, so *4 > + if ($rbuf_len + $hdl->{read_size}*4 > $buf_size) { > + # stop reading=20 stop reading until write buffer is empty > + $hdl->on_read(); > + my $prev_on_drain =3D $reqhdl->{on_drain}; > + $reqhdl->on_drain(sub { > + my ($wrhdl) =3D @_; > + # write buffer is empty, continue reading on_drain was called because write_buffer is empty, start reading again > + $hdl->on_read($on_read); > + if ($prev_on_drain) { > + $wrhdl->on_drain($prev_on_drain); > + $prev_on_drain->($wrhdl); > + } > + }); > + } > + }; > + > + $reqstate->{proxyhdl} =3D AnyEvent::Handle->new( > + fh =3D> $stream_fh, > + rbuf_max =3D> $buf_size, > + timeout =3D> 0, > + on_read =3D> $on_read, > + on_eof =3D> sub { > + my ($hdl) =3D @_; > + eval { > + if (my $reqhdl =3D $reqstate->{hdl}) { > + $self->log_aborted_request($reqstate); > + # write out any remaining data > + $reqhdl->push_write($hdl->{rbuf}) if length($hdl->{rbuf}) > 0; > + $hdl->{rbuf} =3D ""; > + $reqhdl->push_shutdown(); > + $self->finish_response($reqstate); > + } > + }; > + if (my $err =3D $@) { syslog('err', "$err"); } > + $on_read =3D undef; > + }, > + on_error =3D> sub { > + my ($hdl, $fatal, $message) =3D @_; > + eval { > + $self->log_aborted_request($reqstate, $message); > + $self->client_do_disconnect($reqstate); > + }; > + if (my $err =3D $@) { syslog('err', "$err"); } > + $on_read =3D undef; > + }, > + ); > + } elsif ($delay && $delay > 0) { > my $w; $w =3D AnyEvent->timer(after =3D> $delay, cb =3D> sub { > undef $w; # delete reference > $reqstate->{hdl}->push_write($res); > @@ -322,6 +404,13 @@ sub send_file_start { > if (ref($download) eq 'HASH') { > $fh =3D $download->{fh}; > $mime =3D $download->{'content-type'}; > + > + if ($download->{stream}) { > + my $header =3D HTTP::Headers->new(Content_Type =3D> $mime); > + my $resp =3D HTTP::Response->new(200, "OK", $header); > + $self->response($reqstate, $resp, undef, 1, 0, $fh); > + return; > + } > } else { > my $filename =3D $download; > $fh =3D IO::File->new($filename, '<') || > --=20 > 2.20.1 >=20 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20 =