diff mbox series

[1/3] qsd: Unlink absolute PID file path

Message ID 20220609122701.17172-2-hreitz@redhat.com
State New
Headers show
Series qemu/qsd: Unlink absolute PID file path | expand

Commit Message

Hanna Czenczek June 9, 2022, 12:26 p.m. UTC
After writing the PID file, we register an atexit() handler to unlink it
when the process terminates.  However, if the process has changed its
working directory in the meantime (e.g. in os_setup_post() when
daemonizing), this will not work when the PID file path was relative.
Therefore, pass the absolute path (created with realpath()) to the
unlink() call in the atexit() handler.

(realpath() needs a path pointing to an existing file, so we cannot use
it before qemu_write_pidfile().)

Reproducer:
$ cd /tmp
$ qemu-storage-daemon --daemonize --pidfile qsd.pid
$ file qsd.pid
qsd.pid: ASCII text
$ kill $(cat qsd.pid)
$ file qsd.pid
qsd.pid: ASCII text

(qsd.pid should be gone after the process has terminated.)

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2092322
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
 storage-daemon/qemu-storage-daemon.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Daniel P. Berrangé July 12, 2022, 12:14 p.m. UTC | #1
On Thu, Jun 09, 2022 at 02:26:59PM +0200, Hanna Reitz wrote:
> After writing the PID file, we register an atexit() handler to unlink it
> when the process terminates.  However, if the process has changed its
> working directory in the meantime (e.g. in os_setup_post() when
> daemonizing), this will not work when the PID file path was relative.
> Therefore, pass the absolute path (created with realpath()) to the
> unlink() call in the atexit() handler.
> 
> (realpath() needs a path pointing to an existing file, so we cannot use
> it before qemu_write_pidfile().)
> 
> Reproducer:
> $ cd /tmp
> $ qemu-storage-daemon --daemonize --pidfile qsd.pid
> $ file qsd.pid
> qsd.pid: ASCII text
> $ kill $(cat qsd.pid)
> $ file qsd.pid
> qsd.pid: ASCII text
> 
> (qsd.pid should be gone after the process has terminated.)
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2092322
> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
> ---
>  storage-daemon/qemu-storage-daemon.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
diff mbox series

Patch

diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index c104817cdd..7b8d6cf381 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -61,6 +61,7 @@ 
 #include "trace/control.h"
 
 static const char *pid_file;
+static char *pid_file_realpath;
 static volatile bool exit_requested = false;
 
 void qemu_system_killed(int signal, pid_t pid)
@@ -349,7 +350,7 @@  static void process_options(int argc, char *argv[], bool pre_init_pass)
 
 static void pid_file_cleanup(void)
 {
-    unlink(pid_file);
+    unlink(pid_file_realpath);
 }
 
 static void pid_file_init(void)
@@ -365,6 +366,14 @@  static void pid_file_init(void)
         exit(EXIT_FAILURE);
     }
 
+    pid_file_realpath = g_malloc(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(EXIT_FAILURE);
+    }
+
     atexit(pid_file_cleanup);
 }