[pve-devel] [PATCH v2 qemu] init: daemonize: defuse PID file resolve error to warning

[pve-devel] [PATCH v2 qemu] init: daemonize: defuse PID file resolve error to warning

[Fiona Ebner]

When proxmox-file-restore invokes QEMU, the PID file is a (temporary)
file that's already unlinked, so resolving the absolute path here
failed.

It should not be a critical error when the PID file unlink handler
can't be registered, because the path can't be resolved for whatever
reason. Just turn it into a warning instead.

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>
---

Changes from v1:
    * Added R-b and S-b tags.

 softmmu/vl.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index 0d233d55f3..1fa992ab09 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2445,10 +2445,9 @@ 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);
+            warn_report("not removing PID file on exit: cannot resolve path: "
+                        "%s: %s", pid_file, strerror(errno));
+            return;
         }
 
         qemu_unlink_pidfile_notifier = (struct UnlinkPidfileNotifier) {
-- 
2.30.2

Re: [pve-devel] [PATCH v2 qemu] init: daemonize: defuse PID file resolve error to warning

[Thomas Lamprecht]

On 27/10/2022 10:13, Fiona Ebner wrote:
> When proxmox-file-restore invokes QEMU, the PID file is a (temporary)
> file that's already unlinked, so resolving the absolute path here
> failed.
> 
> It should not be a critical error when the PID file unlink handler
> can't be registered, because the path can't be resolved for whatever
> reason. Just turn it into a warning instead.

thx for the patch!

> 
> 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>
> ---
> 
> Changes from v1:
>     * Added R-b and S-b tags.
> 
>  softmmu/vl.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 0d233d55f3..1fa992ab09 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2445,10 +2445,9 @@ 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);
> +            warn_report("not removing PID file on exit: cannot resolve path: "
> +                        "%s: %s", pid_file, strerror(errno));

As my reply to the upstream patch at Daniel's query w.r.t. to warn,
I'd also like to drop the warning here, at least for ENOENT, which
would cover our, and IMO most other, use cases.

> +            return;
>          }
>  
>          qemu_unlink_pidfile_notifier = (struct UnlinkPidfileNotifier) {