[pve-devel] [RFC common] fix misleading warning caused by Devel::Cycle
Thomas Lamprecht
t.lamprecht at proxmox.com
Fri Nov 22 19:03:14 CET 2019
On 11/22/19 6:21 PM, Stoiko Ivanov wrote:
> When validating the parameters (or return values) of an API call against the
> schema we run the parameter hash through Devel::Cycle::find_cycle.
>
> find_cycle has no handler for GLOB types (e.g. filehandles).
> Recently we introduced passing a filehandle as return attribute in
> PMG::API2::Quarantine::download
>
> When find_cycle encounters such an object it emits a warning:
> `Unhandled type: GLOB at /usr/share/perl5/Devel/Cycle.pm line 109`
> which ends up in the journal of pmgproxy (in that example).
>
> Since the line might mislead someone to think that this is a more serious bug
> we silence it, by overriding the warnhandler ($SIG{'__WARN__'}, [0,1]) for the
> time when find_cycle runs, to ignore messages starting with
> 'Unhandled type: GLOB at'. The particular location of and in in Devel::Cycle
> might change and is left out of the match.
>
> [0] man perlvar
> [1] perldoc -f warn
>
> Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
> ---
>
> sending as RFC, since the comment above the patch already indicate that we
> could do away with the cycle-search on every call
I'd really rather to that, this is a build check, and thus we
do not want to do it on every validation - besides those errors
you ran into, it's also a bit costly..
I'd check for an environment variable, e.g. for "DEB_BUILD_ARCH",
which we always have set when building. As some may want to have
validation also in other cases we could also check a opt-in one,
for example:
return if $ENV{DEB_BUILD_ARCH} && !$ENV{PROXMOX_FORCE_SCHEMA_VALIDATION};
>
> The choice of saving the old handler and restoring it instead of using
> local $SIG{'__WARN__'} was done to minimize the effect - although local'izing
> is a bit more idiomatic it seems.
>
> Caught this in my PMG's journal - traced it down, by editing
> /usr/share/perl5/Devel/Cycle and adding a Carp::cluck at the location the
> warning came from)
>
> at /usr/share/perl5/Devel/Cycle.pm line 108.
> Devel::Cycle::_find_cycle_dispatch(IO::File=GLOB(0x559b8b002420), HASH(0x559b8ad928b8), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40), ARRAY(0x559b8acfd088)) called at /usr/share/perl5/Devel/Cycle.pm line 97
> Devel::Cycle::_find_cycle(IO::File=GLOB(0x559b8b002420), HASH(0x559b8b0b6c80), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40), ARRAY(0x559b8acfd088)) called at /usr/share/perl5/Devel/Cycle.pm line 156
> Devel::Cycle::_find_cycle_HASH(HASH(0x559b8b002450), HASH(0x559b8ad8d088), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40)) called at /usr/share/perl5/Devel/Cycle.pm line 114
> Devel::Cycle::_find_cycle_dispatch(HASH(0x559b8b002450), HASH(0x559b8ad8d088), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40)) called at /usr/share/perl5/Devel/Cycle.pm line 97
> Devel::Cycle::_find_cycle(HASH(0x559b8b002450), HASH(0x559b87def760), CODE(0x559b8acfe4f0), 0, HASH(0x559b8ad36c40)) called at /usr/share/perl5/Devel/Cycle.pm line 70
> Devel::Cycle::find_cycle(HASH(0x559b8b002450), CODE(0x559b8acfe4f0)) called at /usr/share/perl5/PVE/JSONSchema.pm line 1053
> PVE::JSONSchema::validate(HASH(0x559b8b002450), HASH(0x559b8ab6d330), "Result verification failed\x{a}") called at /usr/share/perl5/PVE/RESTHandler.pm line 449
> PVE::RESTHandler::handle("PMG::API2::Quarantine", HASH(0x559b8ab6d738), HASH(0x559b8ad23c10)) called at /usr/share/perl5/PMG/HTTPServer.pm line 157
> eval {...} called at /usr/share/perl5/PMG/HTTPServer.pm line 111
> PMG::HTTPServer::rest_handler(PMG::HTTPServer=HASH(0x559b8ac60ad8), "192.168.16.68", "GET", "/quarantine/download", HASH(0x559b8acc0300), HASH(0x559b8706b9c8), "json") called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 725
> eval {...} called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 699
> PVE::APIServer::AnyEvent::handle_api2_request(PMG::HTTPServer=HASH(0x559b8ac60ad8), HASH(0x559b8708f268), HASH(0x559b8acc0300), "GET", "/api2/json/quarantine/download") called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 950
> eval {...} called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 942
> PVE::APIServer::AnyEvent::handle_request(PMG::HTTPServer=HASH(0x559b8ac60ad8), HASH(0x559b8708f268), HASH(0x559b8acc0300), "GET", "/api2/json/quarantine/download") called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 1342
> eval {...} called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 1160
> PVE::APIServer::AnyEvent::__ANON__(AnyEvent::Handle=HASH(0x559b87057838), "", "\x{d}\x{a}") called at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Handle.pm line 1541
> AnyEvent::Handle::__ANON__(AnyEvent::Handle=HASH(0x559b87057838)) called at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Handle.pm line 1315
> AnyEvent::Handle::_drain_rbuf(AnyEvent::Handle=HASH(0x559b87057838)) called at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Handle.pm line 2100
> AnyEvent::Handle::_dotls(AnyEvent::Handle=HASH(0x559b87057838)) called at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Handle.pm line 2013
> AnyEvent::Handle::__ANON__(EV::IO=SCALAR(0x559b8acbfcd0), 1) called at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Impl/EV.pm line 88
> eval {...} called at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent/Impl/EV.pm line 88
> AnyEvent::CondVar::Base::_wait(AnyEvent::CondVar=HASH(0x559b8708f8b0)) called at /usr/lib/x86_64-linux-gnu/perl5/5.28/AnyEvent.pm line 2026
> AnyEvent::CondVar::Base::recv(AnyEvent::CondVar=HASH(0x559b8708f8b0)) called at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 1623
> PVE::APIServer::AnyEvent::run(PMG::HTTPServer=HASH(0x559b8ac60ad8)) called at /usr/share/perl5/PMG/Service/pmgproxy.pm line 132
> PMG::Service::pmgproxy::run(PMG::Service::pmgproxy=HASH(0x559b8ac1ca00)) called at /usr/share/perl5/PVE/Daemon.pm line 171
> eval {...} called at /usr/share/perl5/PVE/Daemon.pm line 171
> PVE::Daemon::__ANON__(PMG::Service::pmgproxy=HASH(0x559b8ac1ca00)) called at /usr/share/perl5/PVE/Daemon.pm line 391
> eval {...} called at /usr/share/perl5/PVE/Daemon.pm line 380
> PVE::Daemon::__ANON__(PMG::Service::pmgproxy=HASH(0x559b8ac1ca00), 1) called at /usr/share/perl5/PVE/Daemon.pm line 552
> eval {...} called at /usr/share/perl5/PVE/Daemon.pm line 550
> PVE::Daemon::start(PMG::Service::pmgproxy=HASH(0x559b8ac1ca00), 1) called at /usr/share/perl5/PVE/Daemon.pm line 660
> PVE::Daemon::__ANON__(HASH(0x559b8543fbd8)) called at /usr/share/perl5/PVE/RESTHandler.pm line 446
> PVE::RESTHandler::handle("PMG::Service::pmgproxy", HASH(0x559b8ac4bf68), HASH(0x559b8543fbd8)) called at /usr/share/perl5/PVE/RESTHandler.pm line 855
> eval {...} called at /usr/share/perl5/PVE/RESTHandler.pm line 838
> PVE::RESTHandler::cli_handler("PMG::Service::pmgproxy", "pmgproxy start", "start", ARRAY(0x559b8574f330), ARRAY(0x559b8ac55bc0), undef, undef, undef) called at /usr/share/perl5/PVE/CLIHandler.pm line 590
> PVE::CLIHandler::__ANON__(ARRAY(0x559b8543ff38), CODE(0x559b85489128), undef) called at /usr/share/perl5/PVE/CLIHandler.pm line 667
> PVE::CLIHandler::run_cli_handler("PMG::Service::pmgproxy", "prepare", CODE(0x559b85489128)) called at /usr/bin/pmgproxy line 31
>
>
> src/PVE/JSONSchema.pm | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
> index 51c3881..6954241 100644
> --- a/src/PVE/JSONSchema.pm
> +++ b/src/PVE/JSONSchema.pm
> @@ -1038,8 +1038,22 @@ sub validate {
> # todo: cycle detection is only needed for debugging, I guess
> # we can disable that in the final release
> # todo: is there a better/faster way to detect cycles?
> +
> + # override warn handler to suppress log-messages for e.g. File::IO objects
> + # in the API definitions (e.g. PMG::API2::Quarantine::download)
> + my $oldwarn = $SIG{'__WARN__'};
> + $SIG{'__WARN__'} = sub {
> + my ($warn) = @_;
> + if ($warn !~ /^Unhandled type: GLOB at/) {
> + print STDERR "$warn\n";
> + }
> + };
> +
> my $cycles = 0;
> find_cycle($instance, sub { $cycles = 1 });
> +
> + $SIG{'__WARN__'} = $oldwarn;
> +
> if ($cycles) {
> add_error($errors, undef, "data structure contains recursive cycles");
> } elsif ($schema) {
>
More information about the pve-devel
mailing list