all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Friedrich Weber <f.weber@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: Re: [pve-devel] [PATCH v2 storage] lvm: improve warning in case vgs output contains unexpected lines
Date: Wed, 31 Jan 2024 12:55:23 +0100	[thread overview]
Message-ID: <74530bb6-6ccc-4b6e-ae3e-c1411fd7f4d7@proxmox.com> (raw)
In-Reply-To: <4f959cf9-5068-4c14-aaf6-885f9cbf982a@proxmox.com>

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




  reply	other threads:[~2024-01-31 11:55 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-18 11:11 Friedrich Weber
2024-01-19 10:59 ` Fiona Ebner
2024-01-19 11:31   ` Fiona Ebner
2024-01-23 10:01     ` Friedrich Weber
2024-01-31 11:55       ` Friedrich Weber [this message]
2024-02-01  9:20         ` Fiona Ebner
2024-02-01  9:30           ` Fiona Ebner
2024-02-02 16:27           ` Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=74530bb6-6ccc-4b6e-ae3e-c1411fd7f4d7@proxmox.com \
    --to=f.weber@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal