From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: "Fiona Ebner" <f.ebner@proxmox.com>,
"Daniel P. Berrangé" <berrange@redhat.com>
Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, hreitz@redhat.com,
d.csapak@proxmox.com
Subject: Re: [PATCH] vl: change PID file path resolve error to warning
Date: Fri, 28 Oct 2022 09:26:28 +0200 [thread overview]
Message-ID: <7c7cfb50-fd32-f26e-7f82-5cedbe47c4bb@proxmox.com> (raw)
In-Reply-To: <6fdd4b06-4c9b-5a7c-d16d-c3b362fecb81@proxmox.com>
On 28/10/2022 09:11, Fiona Ebner wrote:
> Am 27.10.22 um 14:17 schrieb Daniel P. Berrangé:
>> On Thu, Oct 27, 2022 at 12:14:43PM +0200, Fiona Ebner wrote:
>>> + warn_report("not removing PID file on exit: cannot resolve PID file"
>>> + " path: %s: %s", pid_file, strerror(errno));
>>> + return;
>>> }
>> I don't think using warn_report is desirable here.
>>
>> If the behaviour of passing a pre-unlinked pidfile is considered
>> valid, then we should allow it without printing a warning every
>> time an application does this.
>>
>> warnings are to highlight non-fatal mistakes by applications, and
>> this is not a mistake, it is intentionally supported behaviour.
>
> But what if the path resolution fails in a scenario where the caller did
> not pre-unlik the PID file? Should the warning only be printed when the
> errno is not ENOENT? Might still not be accurate in all cases though.
ENOENT would be IMO a good heuristic for silence, as I see no point in
warning that something won't be cleaned up if it's already gone from
POV of QEMU.
Iff, I'd then personally only log at some level that shows up on
debug/high-verbosity settings, that should cover the a bit odd setups where,
e.g. the PID file is there but not visible to QEMU, e.g., because being
located in another mount namespace (in which case the management stack that
put it there probably wants to handle that more explicitly anyway). Or do
you meant something else with "not accurate in all cases"?
best regards,
Thomas
next prev parent reply other threads:[~2022-10-28 7:27 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-27 10:14 [PATCH] vl: change PID file path resolve error to warning Fiona Ebner
2022-10-27 12:06 ` Hanna Reitz
2022-10-27 12:17 ` Daniel P. Berrangé
2022-10-28 7:11 ` Fiona Ebner
2022-10-28 7:26 ` Thomas Lamprecht [this message]
2022-10-28 7:54 ` Fiona Ebner
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=7c7cfb50-fd32-f26e-7f82-5cedbe47c4bb@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=berrange@redhat.com \
--cc=d.csapak@proxmox.com \
--cc=f.ebner@proxmox.com \
--cc=hreitz@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).