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