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

[Friedrich Weber]

On 23/01/2024 11:01, Friedrich Weber wrote:
> On 19/01/2024 12:31, Fiona Ebner wrote:
>> Am 19.01.24 um 11:59 schrieb Fiona Ebner:
[...]
>>> Please use log_warn() from PVE::RESTEnvironment for new warnings, so
>>> they also show up in task logs.
>>
>> Sorry, I mean "show up more visibly", because they count towards the
>> warning count shown in the task result.
> 
> Thanks, wasn't aware of this benefit of `log_warn`.
> 
> Will send a v3 with the two changes.

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.

I'd suggest I use `log_warn` in this patch and keep `log_warn` as it is
(printing only to stderr) for now. The downside is that pvestatd will
not log the `vgs` warning to the syslog anymore (but doing that every 10
seconds could also be considered spammy anyway). The upside is that e.g.
a qmstart task log has the warning in its task log and shows "Warnings:
1" in the GUI, which might be better for visibility than a syslog
message anyway.

What do you think?

[1]
https://git.proxmox.com/?p=pve-manager.git;a=blob;f=bin/pvestatd;h=5cd727e9;hb=d90563e4#l15
[2]
https://git.proxmox.com/?p=pve-common.git;a=blob;f=src/PVE/RESTEnvironment.pm;h=191c6eb;hb=c6ec71d84#l735
[3]
https://git.proxmox.com/?p=qemu-server.git;a=blob;f=PVE/QemuServer.pm;h=b45507aa;hb=eb86f776#l3492