From be00118d5d9a3afb41e0edcddec823dff63a7ae1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 25 Mar 2020 00:58:00 +0100 Subject: [PATCH] util: keep the pidfile locked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unfortunately, advisory record locking lose the lock if any fd refering to the file is closed. There doesn't seem to be a way to preserve the lock atomically. We could eventually retake the lock if low pidfilefd is required. This fixes processes being leaked, as they are not killed in virPidFileForceCleanupPath() if the lock can be taken. Here also, we may consider this is not good enough, as a process may leak by simply closing the pidfilefd. Fixes commit d146105f1e4a9e0ab179f0b78c070ea38b9d5334 ("virCommand: Actually acquire pidfile instead of just writing it") Signed-off-by: Marc-André Lureau Reviewed-by: Michal Privoznik --- src/util/vircommand.c | 12 ++---------- tests/commanddata/test4.log | 2 +- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 77078d09fb..b84fb40948 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -797,8 +797,7 @@ virExec(virCommandPtr cmd) virProcessSetMaxCoreSize(0, cmd->maxCore) < 0) goto fork_error; if (cmd->pidfile) { - VIR_AUTOCLOSE pidfilefd = -1; - int newpidfilefd = -1; + int pidfilefd = -1; char c; pidfilefd = virPidFileAcquirePath(cmd->pidfile, false, getpid()); @@ -818,14 +817,7 @@ virExec(virCommandPtr cmd) VIR_FORCE_CLOSE(pipesync[0]); VIR_FORCE_CLOSE(pipesync[1]); - /* This is here only to move the pidfilefd - * to the lowest possible number. */ - if ((newpidfilefd = dup(pidfilefd)) < 0) { - virReportSystemError(errno, "%s", _("Unable to dup FD")); - goto fork_error; - } - - /* newpidfilefd is intentionally leaked. */ + /* pidfilefd is intentionally leaked. */ } if (cmd->hook) { diff --git a/tests/commanddata/test4.log b/tests/commanddata/test4.log index 5820f28307..24a37a7e96 100644 --- a/tests/commanddata/test4.log +++ b/tests/commanddata/test4.log @@ -9,7 +9,7 @@ ENV:USER=test FD:0 FD:1 FD:2 -FD:3 +FD:5 DAEMON:yes CWD:/ UMASK:0022