From 83e3a339d958373a96cb569a343a41c95e860797 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Tue, 28 Feb 2012 20:30:26 +0100 Subject: [PATCH] keep the PID file locked for the lifetime of the process RH-Author: Laszlo Ersek Message-id: <1330461026-8713-1-git-send-email-lersek@redhat.com> Patchwork-id: 37685 O-Subject: [RHEL-6.3 qemu-kvm PATCH BZ#758194] keep the PID file locked for the lifetime of the process Bugzilla: 758194 RH-Acked-by: Markus Armbruster RH-Acked-by: Kevin Wolf RH-Acked-by: Paolo Bonzini Bugzilla: 758194 Brew: https://brewweb.devel.redhat.com/taskinfo?taskID=4094145 Upstream commit 1bbd1592 was backported as RHEL-6 commit c38bf687 in the Coverity Omnibus series (same BZ#758194): os-posix: Plug fd leak in qemu_create_pidfile() Backport upstream c/s 93dd748b too: The lockf() call in qemu_create_pidfile() aims at ensuring mutual exclusion. We shouldn't close the pidfile on success (as introduced by commit 1bbd1592), because that drops the lock as well [1]: "File locks shall be released on first close by the locking process of any file descriptor for the file." Coverity may complain again about the leaked file descriptor; let's worry about that later. v1->v2: - add reference to 1bbd1592 - explain the intentional fd leak in the source [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/lockf.html Testing: I hijacked /usr/libexec/qemu-kvm by a shell script that prepended -pidfile /tmp/pidfile.$$ to the command line arguments. (Libvirt might provide a more sanctioned way.) I started a guest with the 234 build and checked the pidfiles under /tmp with "fuser -v" -- no process was reported. Then I did the same with the RPM from the above Brew link, and indeed one pidfile was open (and presumably locked), with matching contents. Conflicts: os-posix.c Signed-off-by: Laszlo Ersek --- Considering that testing was this tedious, I'm not sure we support the -pidfile flag at all, so this backport may not be necessary. Please review. Thanks! osdep.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) Signed-off-by: Michal Novotny --- osdep.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/osdep.c b/osdep.c index 81a82a6..e71a8b8 100644 --- a/osdep.c +++ b/osdep.c @@ -162,7 +162,7 @@ int qemu_create_pidfile(const char *filename) return -1; } - close(fd); + /* keep pidfile open & locked forever */ #else HANDLE file; DWORD flags; -- 1.7.7.6