* [PATCH v2] vl: defuse PID file path resolve error
@ 2022-10-31 9:47 Fiona Ebner
2022-11-07 13:05 ` Hanna Reitz
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Fiona Ebner @ 2022-10-31 9:47 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, hreitz, t.lamprecht, d.csapak, berrange
Commit 85c4bf8aa6 ("vl: Unlink absolute PID file path") introduced a
critical error when the PID file path cannot be resolved. Before this
commit, it was possible to invoke QEMU when the PID file was a file
created with mkstemp that was already unlinked at the time of the
invocation. There might be other similar scenarios.
It should not be a critical error when the PID file unlink notifier
can't be registered, because the path can't be resolved. If the file
is already gone from QEMU's perspective, silently ignore the error.
Otherwise, only print a warning.
Fixes: 85c4bf8aa6 ("vl: Unlink absolute PID file path")
Reported-by: Dominik Csapak <d.csapak@proxmox.com>
Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
v1 -> v2:
* Ignore error if errno == ENOENT.
softmmu/vl.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index b464da25bc..cf2c591ba5 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2432,10 +2432,11 @@ static void qemu_maybe_daemonize(const char *pid_file)
pid_file_realpath = g_malloc0(PATH_MAX);
if (!realpath(pid_file, pid_file_realpath)) {
- error_report("cannot resolve PID file path: %s: %s",
- pid_file, strerror(errno));
- unlink(pid_file);
- exit(1);
+ if (errno != ENOENT) {
+ warn_report("not removing PID file on exit: cannot resolve PID "
+ "file path: %s: %s", pid_file, strerror(errno));
+ }
+ return;
}
qemu_unlink_pidfile_notifier = (struct UnlinkPidfileNotifier) {
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] vl: defuse PID file path resolve error
2022-10-31 9:47 [PATCH v2] vl: defuse PID file path resolve error Fiona Ebner
@ 2022-11-07 13:05 ` Hanna Reitz
2023-01-24 13:55 ` Fiona Ebner
2023-03-15 10:48 ` Paolo Bonzini
2 siblings, 0 replies; 5+ messages in thread
From: Hanna Reitz @ 2022-11-07 13:05 UTC (permalink / raw)
To: Fiona Ebner, qemu-devel; +Cc: pbonzini, t.lamprecht, d.csapak, berrange
On 31.10.22 10:47, Fiona Ebner wrote:
> Commit 85c4bf8aa6 ("vl: Unlink absolute PID file path") introduced a
> critical error when the PID file path cannot be resolved. Before this
> commit, it was possible to invoke QEMU when the PID file was a file
> created with mkstemp that was already unlinked at the time of the
> invocation. There might be other similar scenarios.
>
> It should not be a critical error when the PID file unlink notifier
> can't be registered, because the path can't be resolved. If the file
> is already gone from QEMU's perspective, silently ignore the error.
> Otherwise, only print a warning.
>
> Fixes: 85c4bf8aa6 ("vl: Unlink absolute PID file path")
> Reported-by: Dominik Csapak <d.csapak@proxmox.com>
> Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> v1 -> v2:
> * Ignore error if errno == ENOENT.
>
> softmmu/vl.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] vl: defuse PID file path resolve error
2022-10-31 9:47 [PATCH v2] vl: defuse PID file path resolve error Fiona Ebner
2022-11-07 13:05 ` Hanna Reitz
@ 2023-01-24 13:55 ` Fiona Ebner
2023-03-15 8:52 ` Fiona Ebner
2023-03-15 10:48 ` Paolo Bonzini
2 siblings, 1 reply; 5+ messages in thread
From: Fiona Ebner @ 2023-01-24 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, hreitz, t.lamprecht, d.csapak, berrange
Am 31.10.22 um 10:47 schrieb Fiona Ebner:
> Commit 85c4bf8aa6 ("vl: Unlink absolute PID file path") introduced a
> critical error when the PID file path cannot be resolved. Before this
> commit, it was possible to invoke QEMU when the PID file was a file
> created with mkstemp that was already unlinked at the time of the
> invocation. There might be other similar scenarios.
>
> It should not be a critical error when the PID file unlink notifier
> can't be registered, because the path can't be resolved. If the file
> is already gone from QEMU's perspective, silently ignore the error.
> Otherwise, only print a warning.
>
> Fixes: 85c4bf8aa6 ("vl: Unlink absolute PID file path")
> Reported-by: Dominik Csapak <d.csapak@proxmox.com>
> Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> v1 -> v2:
> * Ignore error if errno == ENOENT.
>
> softmmu/vl.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index b464da25bc..cf2c591ba5 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2432,10 +2432,11 @@ static void qemu_maybe_daemonize(const char *pid_file)
>
> pid_file_realpath = g_malloc0(PATH_MAX);
> if (!realpath(pid_file, pid_file_realpath)) {
> - error_report("cannot resolve PID file path: %s: %s",
> - pid_file, strerror(errno));
> - unlink(pid_file);
> - exit(1);
> + if (errno != ENOENT) {
> + warn_report("not removing PID file on exit: cannot resolve PID "
> + "file path: %s: %s", pid_file, strerror(errno));
> + }
> + return;
> }
>
> qemu_unlink_pidfile_notifier = (struct UnlinkPidfileNotifier) {
Ping
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] vl: defuse PID file path resolve error
2023-01-24 13:55 ` Fiona Ebner
@ 2023-03-15 8:52 ` Fiona Ebner
0 siblings, 0 replies; 5+ messages in thread
From: Fiona Ebner @ 2023-03-15 8:52 UTC (permalink / raw)
To: qemu-devel, pbonzini; +Cc: hreitz, t.lamprecht, d.csapak, berrange
Am 24.01.23 um 14:55 schrieb Fiona Ebner:
> Am 31.10.22 um 10:47 schrieb Fiona Ebner:
>> Commit 85c4bf8aa6 ("vl: Unlink absolute PID file path") introduced a
>> critical error when the PID file path cannot be resolved. Before this
>> commit, it was possible to invoke QEMU when the PID file was a file
>> created with mkstemp that was already unlinked at the time of the
>> invocation. There might be other similar scenarios.
>>
>> It should not be a critical error when the PID file unlink notifier
>> can't be registered, because the path can't be resolved. If the file
>> is already gone from QEMU's perspective, silently ignore the error.
>> Otherwise, only print a warning.
>>
>> Fixes: 85c4bf8aa6 ("vl: Unlink absolute PID file path")
>> Reported-by: Dominik Csapak <d.csapak@proxmox.com>
>> Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
>> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>> ---
>>
>> v1 -> v2:
>> * Ignore error if errno == ENOENT.
>>
>> softmmu/vl.c | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index b464da25bc..cf2c591ba5 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -2432,10 +2432,11 @@ static void qemu_maybe_daemonize(const char *pid_file)
>>
>> pid_file_realpath = g_malloc0(PATH_MAX);
>> if (!realpath(pid_file, pid_file_realpath)) {
>> - error_report("cannot resolve PID file path: %s: %s",
>> - pid_file, strerror(errno));
>> - unlink(pid_file);
>> - exit(1);
>> + if (errno != ENOENT) {
>> + warn_report("not removing PID file on exit: cannot resolve PID "
>> + "file path: %s: %s", pid_file, strerror(errno));
>> + }
>> + return;
>> }
>>
>> qemu_unlink_pidfile_notifier = (struct UnlinkPidfileNotifier) {
>
> Ping
>
Ping again. While it's not a critical patch, it's also not a big one :)
Best Regards,
Fiona
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] vl: defuse PID file path resolve error
2022-10-31 9:47 [PATCH v2] vl: defuse PID file path resolve error Fiona Ebner
2022-11-07 13:05 ` Hanna Reitz
2023-01-24 13:55 ` Fiona Ebner
@ 2023-03-15 10:48 ` Paolo Bonzini
2 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2023-03-15 10:48 UTC (permalink / raw)
To: Fiona Ebner; +Cc: qemu-devel, pbonzini, hreitz, t.lamprecht, d.csapak, berrange
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-15 10:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-31 9:47 [PATCH v2] vl: defuse PID file path resolve error Fiona Ebner
2022-11-07 13:05 ` Hanna Reitz
2023-01-24 13:55 ` Fiona Ebner
2023-03-15 8:52 ` Fiona Ebner
2023-03-15 10:48 ` Paolo Bonzini
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).