On Fri, Apr 15, 2016 at 09:49:42AM +0100, Richard W.M. Jones wrote:
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 two functions to hide the details, and modifies a few
places where we currently busy-wait.
---
src/fdstream.c | 10 +++++----
src/libvirt_private.syms | 2 ++
src/qemu/qemu_agent.c | 9 ++++----
src/qemu/qemu_monitor.c | 10 +++++----
src/util/virtime.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
src/util/virtime.h | 38 ++++++++++++++++++++++++++++++++++
6 files changed, 111 insertions(+), 12 deletions(-)
diff --git a/src/fdstream.c b/src/fdstream.c
index ef118b5..94de52e 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
@@ -520,8 +521,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);
@@ -541,7 +541,9 @@ int virFDStreamConnectUNIX(virStreamPtr st,
goto error;
}
- do {
+ if (virTimeBackOffStart(&timeout, 1, 3*1000 /* ms */) < 0)
+ goto error;
+ while (virTimeBackOffWhile(&timeout)) {
I'm not keeping up with these patches, but why don't we have one function that initializes the @timeout on it's first run based on the parameters? Actually, it would just use the timeout to keep one number, so it could be uint or something. You could then basically do: while (virBackOffWait(&timeout, 1, 3000)) And if you don't like having the numbers in here, then we could do an initialization macro for the timeout variable. You already ignore the return value of virTimeMillisNowRaw(), so either you could just return false if you're in the initialization phase or if you *really* want to error out, then mark it in the struct. As I said, I don't know what the discussions were and if this was already discussed at all. I just wanted to chip in with my two cents. Martin
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list