Re: [pve-devel] [PATCH v2 storage] lvm: improve warning in case vgs output contains unexpected lines

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

Am 01.02.24 um 10:20 schrieb Fiona Ebner:
> Am 31.01.24 um 12:55 schrieb Friedrich Weber:
>>
>> After changing `warn` to `log_warn` I noticed that pvestatd does not
>> write the warning to the syslog every 10s anymore. Turns out `warn`
>> triggers a custom __WARN__ handler we install for our daemons which also
>> writes to syslog (e.g. pvestatd [1]). But `log_warn` only writes to
>> stderr [2], thus the `log_warn` warning is not written to the syslog.
>>
>> Briefly discussed with Fiona off-list, she suggested to try replacing
>> `print` in `log_warn` with `warn` [2]. But with this, all `log_warn`
>> warnings also appear in the syslog (e.g. the "no efidisk configured"
>> warning [3]), which I think would be too spammy.
>>
> 
> But that is the original intention behind the __WARN__ handler. I'd
> argue that replacing a warn with a log_warn() shouldn't change these
> semantics. The intention of log_warn() was to make warnings more visible
> in task logs. Making them less visible outside of tasks was not intended!
> 
> The issue that task warnings also appear in syslog for pvedaemon and
> others is a bit orthogonal and can be fixed too.
> 
> My proposed solutions:
> 
> 1) Switch to using "warn" instead of "print STDERR" in log_warn() and
> either:
> 1A) Clear the inherited __WARN__ handler in fork_worker().
> 1B) Have the __WARN__ handler check if it's in a worker and only log to
> syslog if it's not.
> 
The above would also address instances of "warn" (and not just
log_warn()) that currently end up in syslog even though they happen in a
task worker. OTOH, one could argue such warnings are not visible enough
if not also logged to syslog, because there is no task warning count and
it's not obvious you should read the task log.

> 2. Keep using "print STDERR" in log_warn() and do print to syslog there
> if not in a worker.

We'd also need to check that it's not an interactive invocation, because
for those it should also not log to syslog.