132 lines
4.7 KiB
Diff
132 lines
4.7 KiB
Diff
From 28b27787c5bb545df3c178fef682151c45b66784 Mon Sep 17 00:00:00 2001
|
|
From: Martin Kletzander <mkletzan@redhat.com>
|
|
Date: Mon, 8 Sep 2014 07:46:39 +0200
|
|
Subject: [PATCH] rpc: make daemon spawning a bit more intelligent
|
|
|
|
This way it behaves more like the daemon itself does (acquiring a
|
|
pidfile, deleting the socket before binding, etc.).
|
|
|
|
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369
|
|
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138604
|
|
|
|
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
|
|
---
|
|
src/rpc/virnetsocket.c | 57 ++++++++++++++++++++++++++++++++++++++++++++------
|
|
1 file changed, 51 insertions(+), 6 deletions(-)
|
|
|
|
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
|
|
index 306c9ea..fa9ba99 100644
|
|
--- a/src/rpc/virnetsocket.c
|
|
+++ b/src/rpc/virnetsocket.c
|
|
@@ -51,9 +51,11 @@
|
|
#include "virlog.h"
|
|
#include "virfile.h"
|
|
#include "virthread.h"
|
|
+#include "virpidfile.h"
|
|
#include "virprobe.h"
|
|
#include "virprocess.h"
|
|
#include "virstring.h"
|
|
+#include "dirname.h"
|
|
#include "passfd.h"
|
|
|
|
#if WITH_SSH2
|
|
@@ -544,7 +546,10 @@ int virNetSocketNewConnectUNIX(const char *path,
|
|
const char *binary,
|
|
virNetSocketPtr *retsock)
|
|
{
|
|
+ char *binname = NULL;
|
|
+ char *pidpath = NULL;
|
|
int fd, passfd = -1;
|
|
+ int pidfd = -1;
|
|
virSocketAddr localAddr;
|
|
virSocketAddr remoteAddr;
|
|
|
|
@@ -583,16 +588,45 @@ int virNetSocketNewConnectUNIX(const char *path,
|
|
goto error;
|
|
}
|
|
|
|
+ if (!(binname = last_component(binary)) || binname[0] == '\0') {
|
|
+ virReportError(VIR_ERR_INTERNAL_ERROR,
|
|
+ _("Cannot determine basename for binary '%s'"),
|
|
+ binary);
|
|
+ goto error;
|
|
+ }
|
|
+
|
|
+ if (virPidFileConstructPath(false, NULL, binname, &pidpath) < 0)
|
|
+ goto error;
|
|
+
|
|
+ if ((pidfd = virPidFileAcquirePath(pidpath, false, getpid())) < 0) {
|
|
+ /*
|
|
+ * This can happen in a very rare case of two clients spawning two
|
|
+ * daemons at the same time, and the error in the logs that gets
|
|
+ * reset here can be a clue to some future debugging.
|
|
+ */
|
|
+ virResetLastError();
|
|
+ spawnDaemon = false;
|
|
+ goto retry;
|
|
+ }
|
|
+
|
|
if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0) {
|
|
virReportSystemError(errno, "%s", _("Failed to create socket"));
|
|
goto error;
|
|
}
|
|
|
|
/*
|
|
- * We have to fork() here, because umask() is set
|
|
- * per-process, chmod() is racy and fchmod() has undefined
|
|
- * behaviour on sockets according to POSIX, so it doesn't
|
|
- * work outside Linux.
|
|
+ * We already even acquired the pidfile, so noone else should be using
|
|
+ * @path right now. So we're OK to unlink it and paying attention to
|
|
+ * the return value makes no real sense here. Only if it's not an
|
|
+ * abstract socket, of course.
|
|
+ */
|
|
+ if (path[0] != '@')
|
|
+ unlink(path);
|
|
+
|
|
+ /*
|
|
+ * We have to fork() here, because umask() is set per-process, chmod()
|
|
+ * is racy and fchmod() has undefined behaviour on sockets according to
|
|
+ * POSIX, so it doesn't work outside Linux.
|
|
*/
|
|
if ((pid = virFork()) < 0)
|
|
goto error;
|
|
@@ -612,8 +646,9 @@ int virNetSocketNewConnectUNIX(const char *path,
|
|
/*
|
|
* OK, so the subprocces failed to bind() the socket. This may mean
|
|
* that another daemon was starting at the same time and succeeded
|
|
- * with its bind(). So we'll try connecting again, but this time
|
|
- * without spawning the daemon.
|
|
+ * with its bind() (even though it should not happen because we
|
|
+ * using a pidfile for the race). So we'll try connecting again,
|
|
+ * but this time without spawning the daemon.
|
|
*/
|
|
spawnDaemon = false;
|
|
goto retry;
|
|
@@ -632,6 +667,12 @@ int virNetSocketNewConnectUNIX(const char *path,
|
|
goto error;
|
|
}
|
|
|
|
+ /*
|
|
+ * Do we need to eliminate the super-rare race here any more? It would
|
|
+ * need incorporating the following VIR_FORCE_CLOSE() into a
|
|
+ * virCommandHook inside a virNetSocketForkDaemon().
|
|
+ */
|
|
+ VIR_FORCE_CLOSE(pidfd);
|
|
if (virNetSocketForkDaemon(binary, passfd) < 0)
|
|
goto error;
|
|
}
|
|
@@ -648,8 +689,12 @@ int virNetSocketNewConnectUNIX(const char *path,
|
|
return 0;
|
|
|
|
error:
|
|
+ if (pidfd > 0)
|
|
+ virPidFileDeletePath(pidpath);
|
|
+ VIR_FREE(pidpath);
|
|
VIR_FORCE_CLOSE(fd);
|
|
VIR_FORCE_CLOSE(passfd);
|
|
+ VIR_FORCE_CLOSE(pidfd);
|
|
if (spawnDaemon)
|
|
unlink(path);
|
|
return -1;
|