On Fri, Apr 15, 2016 at 10:48:30AM +0200, Jiri Denemark wrote:
> 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)) {
> ...
> }
I agree, I really don't like control loops being hidden away behind
macros as it obscures what is going on IMHO.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list