Re: [PATCH v4] Add macro for handling exponential backoff loops.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 15, 2016 at 09:38:42 +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 a macro VIR_TIME_WHILE_WITH_BACKOFF 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       | 34 ++++++++++++++++++++++++++++++
>  6 files changed, 107 insertions(+), 12 deletions(-)
> 
> diff --git a/src/fdstream.c b/src/fdstream.c
> index ef118b5..d9646d0 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;
> +    VIR_TIME_WHILE_WITH_BACKOFF(timeout) {

Uff, I don't think we should go this route. Open coding the ugly macro
makes the code nicer and easier to read and understand:

    if (virTimeBackOffStart(&timeout, 1, 3*1000) < 0)
        goto error;
    while (virTimeBackOffCondition(&timeout)) {
        ...
    }

However, I'd suggest changing the name virTimeBackOffCondition to
something like virTimeBackOffWait to make it sound better when used in
the while loop:

    while (virTimeBackOffWait(&timeout))
        ...

The rest of the patch looks fine, though.

Jirka

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]