On Wed, Aug 09, 2017 at 06:17:20PM +0200, Dietmar Maurer wrote: > Looks quite strange to me, because PVE::HTTPServer calls: > > PVE::API2->find_handler > > so we should use PVE::API2 in PVE::HTTPServer? > > Not sure why it works without?? >
it does, but only in rest_handler, which AFAICT, is not called in the spiceproxy case: PVE::APIServer::AnyEvent has: new which calls accept_connections which calls push_request_header which calls unshift_read_header which after being done with HTTP header processing, checks for the spiceproxy flag (set in spiceproxy.pm), and calls verify_spice_connect_url and handle_spice_proxy_request and returns. verify_spice_connect_url is implemented in PVE::HTTPServer, and only uses PVE::RPCEnvironment and PVE::AccessControl handle_spice_proxy_request is implemented in PVE::APIServer::AnyEvent, and AFAICT does not use the API in any way. unshift_read_header also calls both file_upload_multipart and handle_request, which call handle_api2_request which calls the problematic rest_handler but those calls in unshift_read_header happen after the spiceproxy check mentioned above, so those paths are never taken. so while it looks safe to remove "use PVE::API2" from spiceproxy.pm, technically it is missing from PVE::HTTPServer.pm and the memory usage would still be the same. we could move PVE::API2 into the server config / $self, and let only pveproxy.pm and pvedaemon.pm (which both have the appropriate use statement) set the config option? alternatively, spiceproxy.pm could use a new SpiceProxyHTTPServer, which does not implement rest_handler and auth_handler and thus does not need to use PVE::API2. PVE::HTTPServer could then use PVE::API2 without affecting spiceproxy's memory usage. pveproxy, pvedaemon and pvespiceproxy could all clean up their use statements to reflect what is actually directly used in the respective perl file. > > On August 9, 2017 at 2:08 PM Dominik Csapak <d.csa...@proxmox.com> wrote: > > > > > > we do not need it there and withouth this we save ~30MB memory for > > this daemon and its workers > > > > Signed-off-by: Dominik Csapak <d.csa...@proxmox.com> > > --- > > PVE/Service/spiceproxy.pm | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/PVE/Service/spiceproxy.pm b/PVE/Service/spiceproxy.pm > > index 20fd5b24..22a501b0 100755 > > --- a/PVE/Service/spiceproxy.pm > > +++ b/PVE/Service/spiceproxy.pm > > @@ -10,7 +10,6 @@ use warnings; > > use PVE::SafeSyslog; > > use PVE::Daemon; > > use PVE::API2Tools; > > -use PVE::API2; > > use PVE::HTTPServer; > > > > use base qw(PVE::Daemon); > > -- > > 2.11.0 > > > > > > _______________________________________________ > > pve-devel mailing list > > pve-devel@pve.proxmox.com > > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > _______________________________________________ > pve-devel mailing list > pve-devel@pve.proxmox.com > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
