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

[Andrea Bolognani]

On Fri, 2016-04-15 at 09:49 +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(-)

Just a couple of comments passing by...

> -    do {
> +    if (virTimeBackOffStart(&timeout, 1, 3*1000 /* ms */) < 0)
> +        goto error;
> +    while (virTimeBackOffWhile(&timeout)) {
>          ret = connect(fd, (struct sockaddr *)&sa, sizeof(sa));
>          if (ret == 0)
>              break;

Having two "whiles" like that looks kinda off... I'd rather
have something like

  while (!virTimeBackOffHasExpired(&timeout))

or preferably something better than what I can come up with :)

> +/**
> + * virTimeBackOffWhile
> + * @var: Timeout variable (with type virTimeBackOffVar *).
> + *
> + * You must initialize @var first by calling the following function,
> + * which also starts the timer:
> + *
> + * if (virTimeBackOffStart(&var, first, timeout) < 0) {
> + *   // handle errors
> + * }
> + *
> + * Then you use a while loop:
> + *
> + * while (virTimeBackOffWhile(&var)) {
> + *   //...
> + * }
> + *
> + * The while loop that runs the body of the code repeatedly, with an
> + * exponential backoff.  It first waits for first milliseconds, then
> + * runs the body, then waits for 2*first ms, then runs the body again.
> + * Then 4*first ms, and so on.
> + *
> + * When timeout milliseconds 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).
> + */
> +bool virTimeBackOffWhile(virTimeBackOffVar *var);
> +
>  #endif

API documentation should live in the .c file, like you did
with virTimeBackOffStart(). I guess it's just a consequence
of the "a" implementation using a macro for this part :)

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team