From: Hanna Reitz <hreitz@redhat.com>
To: qemu-block@nongnu.org
Cc: qemu-devel@nongnu.org, Hanna Reitz <hreitz@redhat.com>,
Kevin Wolf <kwolf@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>
Subject: [PATCH 0/3] qemu/qsd: Unlink absolute PID file path
Date: Thu, 9 Jun 2022 14:26:58 +0200 [thread overview]
Message-ID: <20220609122701.17172-1-hreitz@redhat.com> (raw)
Hi,
QEMU (the system emulator) and the storage daemon (QSD) write their PID
to the given file when you specify --pidfile. They keep the path around
and register exit handlers (QEMU uses an exit notifier, QSD an atexit()
function) to unlink this file when the process terminates. These
handlers unlink precisely the path that the user has specified via
--pidfile, so if it was a relative path and the process has at any point
changed its working directory, the path no longer points to the PID
file, and so the unlink() will fail (or worse).
When using --daemonize, the process will always change its working
directory to /, so this problem basically always appears when using
--daemonize and --pidfile in conjunction.
(It gets even worse with QEMUâs --chroot, but Iâm not sure whether
thereâs any trivial fix for that (whether chroot is used or not is
confined to os-posix.c, so this would need to be externally visible; and
I guess the plain would be to skip the unlink() in that case?), so Iâd
rather just skip that for now... :/)
We can fix the problem by running realpath() once the PID file has been
created, so we get an absolute path that we can unlink in the exit
handler. This is done here.
(Another way to fix this might be to open the directory the PID file is
in, keep the FD around, and use unlinkat() in the exit handler. I
couldnât see any real benefit for this, though, so I didnât go that
route. It might be beneficial for the --chroot case, but then again,
precisely in that case we probably donât want to keep random directory
FDs around.)
Hanna Reitz (3):
qsd: Unlink absolute PID file path
vl: Conditionally register PID file unlink notifier
vl: Unlink absolute PID file path
softmmu/vl.c | 42 +++++++++++++++++++++-------
storage-daemon/qemu-storage-daemon.c | 11 +++++++-
2 files changed, 42 insertions(+), 11 deletions(-)
--
2.35.3
next reply other threads:[~2022-06-09 13:57 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-09 12:26 Hanna Reitz [this message]
2022-06-09 12:26 ` [PATCH 1/3] qsd: Unlink absolute PID file path Hanna Reitz
2022-07-12 12:14 ` Daniel P. Berrangé
2022-06-09 12:27 ` [PATCH 2/3] vl: Conditionally register PID file unlink notifier Hanna Reitz
2022-07-12 12:14 ` Daniel P. Berrangé
2022-06-09 12:27 ` [PATCH 3/3] vl: Unlink absolute PID file path Hanna Reitz
2022-07-12 12:19 ` Daniel P. Berrangé
2022-07-12 11:47 ` [PATCH 0/3] qemu/qsd: " Hanna Reitz
2022-07-12 12:37 ` Hanna Reitz
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=20220609122701.17172-1-hreitz@redhat.com \
--to=hreitz@redhat.com \
--cc=kwolf@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--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).