[libvirt] [PATCH v4b] Add functions for handling exponential backoff loops.

[Martin Kletzander]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160415/ce43c532/attachment-0001.sig>