In a few places in libvirt we busy-wait for events, for example qemu
creating a monitor socket. This is problematic because:
- We need to choose a sufficiently small polling period so that
libvirt doesn't add unnecessary delays.
- We need to choose a sufficiently large polling period so that
the effect of busy-waiting doesn't affect the system.
The solution to this conflict is to use an exponential backoff.
This patch adds a macro VIR_WHILE_EXPONENTIAL_BACKOFF to hide the
details, and modifies a few places where we currently busy-wait.
---
src/fdstream.c | 8 ++++----
src/libvirt_private.syms | 2 ++
src/qemu/qemu_agent.c | 8 ++++----
src/qemu/qemu_monitor.c | 8 ++++----
src/util/virtime.c | 42 ++++++++++++++++++++++++++++++++++++++++++
src/util/virtime.h | 39 +++++++++++++++++++++++++++++++++++++++
6 files changed, 95 insertions(+), 12 deletions(-)
diff --git a/src/fdstream.c b/src/fdstream.c
index a85cf9d..7a311f5 100644
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -42,6 +42,7 @@
#include "virfile.h"
#include "configmake.h"
#include "virstring.h"
+#include "virtime.h"
#define VIR_FROM_THIS VIR_FROM_STREAMS
@@ -516,8 +517,7 @@ int virFDStreamConnectUNIX(virStreamPtr st,
bool abstract)
{
struct sockaddr_un sa;
- size_t i = 0;
- int timeout = 3;
+ virTimeBackOffVar timeout;
int ret;
int fd = socket(AF_UNIX, SOCK_STREAM, 0);
@@ -537,7 +537,7 @@ int virFDStreamConnectUNIX(virStreamPtr st,
goto error;
}
- do {
+ VIR_WHILE_EXPONENTIAL_BACKOFF(timeout, 1, 3*1000 /* ms */) {
ret = connect(fd, (struct sockaddr *)&sa, sizeof(sa));
if (ret == 0)
break;
@@ -549,7 +549,7 @@ int virFDStreamConnectUNIX(virStreamPtr st,
}
goto error;
- } while ((++i <= timeout*5) && (usleep(.2 * 1000000) <= 0));
+ }
if (virFDStreamOpenInternal(st, fd, NULL, -1, 0) < 0)
goto error;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 068bc00..b69d2e0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2360,6 +2360,8 @@ virThreadPoolSendJob;
# util/virtime.h
+virTimeBackOffCondition;
+virTimeBackOffInit;
virTimeFieldsNow;
virTimeFieldsNowRaw;
virTimeFieldsThen;
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index bee8d4c..adf38c5 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -42,6 +42,7 @@
#include "virtime.h"
#include "virobject.h"
#include "virstring.h"
+#include "virtime.h"
#include "base64.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -173,9 +174,8 @@ qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool *inProgress)
{
struct sockaddr_un addr;
int monfd;
- int timeout = 3; /* In seconds */
+ virTimeBackOffVar timeout;
int ret;
- size_t i = 0;
*inProgress = false;
@@ -207,7 +207,7 @@ qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool *inProgress)
goto error;
}
- do {
+ VIR_WHILE_EXPONENTIAL_BACKOFF(timeout, 1, 3*1000 /* ms */) {
ret = connect(monfd, (struct sockaddr *) &addr, sizeof(addr));
if (ret == 0)
@@ -232,7 +232,7 @@ qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool *inProgress)
_("failed to connect to monitor socket"));
goto error;
- } while ((++i <= timeout*5) && (usleep(.2 * 1000000) <= 0));
+ }
if (ret != 0) {
virReportSystemError(errno, "%s",
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 10a6713..52f0d20 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -42,6 +42,7 @@
#include "virobject.h"
#include "virprobe.h"
#include "virstring.h"
+#include "virtime.h"
#ifdef WITH_DTRACE_PROBES
# include "libvirt_qemu_probes.h"
@@ -327,9 +328,8 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid)
{
struct sockaddr_un addr;
int monfd;
- int timeout = 30; /* In seconds */
+ virTimeBackOffVar timeout;
int ret;
- size_t i = 0;
if ((monfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
virReportSystemError(errno,
@@ -345,7 +345,7 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid)
goto error;
}
- do {
+ VIR_WHILE_EXPONENTIAL_BACKOFF(timeout, 1, 30*1000 /* ms */) {
ret = connect(monfd, (struct sockaddr *) &addr, sizeof(addr));
if (ret == 0)
@@ -362,7 +362,7 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid)
_("failed to connect to monitor socket"));
goto error;
- } while ((++i <= timeout*5) && (usleep(.2 * 1000000) <= 0));
+ }
if (ret != 0) {
virReportSystemError(errno, "%s",
diff --git a/src/util/virtime.c b/src/util/virtime.c
index 9d365d5..78622e7 100644
--- a/src/util/virtime.c
+++ b/src/util/virtime.c
@@ -34,14 +34,18 @@
#include <config.h>
#include <stdio.h>
+#include <unistd.h>
#include <sys/time.h>
#include "virtime.h"
#include "viralloc.h"
#include "virerror.h"
+#include "virlog.h"
#define VIR_FROM_THIS VIR_FROM_NONE
+VIR_LOG_INIT("util.time");
+
/* We prefer clock_gettime if available because that is officially
* async signal safe according to POSIX. Many platforms lack it
* though, so fallback to gettimeofday everywhere else
@@ -363,3 +367,41 @@ virTimeLocalOffsetFromUTC(long *offset)
*offset = current - utc;
return 0;
}
+
+void
+virTimeBackOffInit(virTimeBackOffVar *var,
+ unsigned long long first, unsigned long long limit)
+{
+ ignore_value(virTimeMillisNowRaw(&var->start_t));
+ var->next = first;
+ var->limit = var->start_t + limit;
+}
+
+int
+virTimeBackOffCondition(virTimeBackOffVar *var)
+{
+ unsigned long long t, next;
+
+ ignore_value(virTimeMillisNowRaw(&t));
+
+ VIR_DEBUG("t=%llu, limit=%llu", t, var->limit);
+
+ if (t > var->limit)
+ return 0; /* ends the while loop */
+
+ next = var->next;
+ var->next *= 2;
+
+ /* If sleeping would take us beyond the limit, then shorten the
+ * sleep. This is so we always run the body just before the final
+ * timeout.
+ */
+ if (t + next > var->limit) {
+ next = var->limit - t - 2;
+ }
+
+ VIR_DEBUG("sleeping for %llu ms", next);
+
+ usleep(next * 1000);
+ return 1;
+}
diff --git a/src/util/virtime.h b/src/util/virtime.h
index 8ebad38..723a93a 100644
--- a/src/util/virtime.h
+++ b/src/util/virtime.h
@@ -64,4 +64,43 @@ char *virTimeStringThen(unsigned long long when);
int virTimeLocalOffsetFromUTC(long *offset)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+/**
+ * VIR_WHILE_EXPONENTIAL_BACKOFF:
+ * @var: A scratch variable used by the loop (with type virTimeBackOffVar).
+ * @first: First time period to wait, in milliseconds.
+ * @limit: Total time we run before the loop times out, in milliseconds.
+ *
+ * This macro is a while loop that runs the body of the code
+ * repeatedly, with an exponential backoff. It first waits for @first
+ * ms, then runs the body, then waits for 2*@first ms, then runs the
+ * body again. Then 4*@first ms, and so on.
+ *
+ * When @limit ms (total running time) is reached, the while loop
+ * ends.
+ *
+ * The body should use "break" or "goto" when whatever condition it is
+ * testing for succeeds (or there is an unrecoverable error).
+ *
+ * NOTE: VIR_WHILE_EXPONENTIAL_BACKOFF is not a single atomic C
+ * statement (unfortunately), so you will get incorrect code if you
+ * write something like:
+ *
+ * if (foo)
+ * VIR_WHILE_EXPONENTIAL_BACKOFF(...) {
+ * }
+ */
+#define VIR_WHILE_EXPONENTIAL_BACKOFF(var, first, limit) \
+ virTimeBackOffInit(&(var), (first), (limit)); \
+ while (virTimeBackOffCondition(&(var)))
+
+typedef struct {
+ unsigned long long start_t;
+ unsigned long long next;
+ unsigned long long limit;
+} virTimeBackOffVar;
+
+void virTimeBackOffInit(virTimeBackOffVar *var,
+ unsigned long long first, unsigned long long limit);
+int virTimeBackOffCondition(virTimeBackOffVar *var);
+
#endif
--
2.7.4
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list