Re: [pve-devel] [RFC common 2/2] REST environment: fork worker: install custom __WARN__ handler

[Fiona Ebner <f.ebner@proxmox.com>]

Am 05.02.24 um 13:39 schrieb Thomas Lamprecht:
> Am 05/02/2024 um 13:28 schrieb Fiona Ebner:
>> So all warnings will be treated consistently inside a worker task. In
>> particular, all warnings will count towards the task warning count and
>> be more visible in the UI. Avoids the need to switch existing warnings
>> to log_warn().
>>
>> When pvedaemon forked a worker, the worker would inherit its __WARN__
>> handler and also log any warnings to syslog, so make sure those
>> warnings are not less visible than before, by also logging to syslog.
>>
>> The same warning would not show up in syslog when the task was invoked
>> via the CLI instead, which was another inconsistency.
>>
>> The __WARN__ handler needs to increment the warning_count for warnings
>> issued with Perl's warn. But then in RESTEnvironment.pm's warn method,
>> warning_count cannot be incremented anymore, because otherwise a call
>> to log_warn() would increment it once, call warn, and then the
>> __WARN__ handler would increment it again. The variable is only ever
>> read in fork_worker(), so it is safe to just move the code
>> incrementing it to the __WARN__ handler, because that will be called
>> by both, Perl's warn and log_warn().
>>
>> This effectively makes log_warn() and RESTEnvironment.pm's warn method
>> not worth using anymore, so deprecate them.
>>
> 
> they where deliberately added though, warn and task-log warnings can
> be two very different things, which IMO should stay that way..

Then I misunderstood. I though the plan was to have all warnings show up
as task warnings in the long term. It feels a bit strange to me to have
two different kinds of warnings. We should document when log_warn()
should and shouldn't be used then. I honestly don't know.