On Thu, Jan 24, 2013 at 02:59:49PM -0500, Laine Stump wrote:
> On 01/24/2013 01:34 PM, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> >
> > This previous commit
> >
> > commit 1a50ba2cb07d8bb2aa724062889deb9efd7ad9e9
> > Author: Viktor Mihajlovski <mihajlov@xxxxxxxxxxxxxxxxxx>
> > Date: Mon Nov 26 15:17:13 2012 +0100
> >
> > qemu: Fix QMP Capabability Probing Failure
> >
> > which attempted to make sure the QEMU process used for probing
> > ran as the right user id, caused serious performance regression
> > and unreliability in probing. The -daemonize switch in QEMU
> > guarantees that the monitor socket is present before the parent
> > process exits. This means libvirtd is guaranteed to be able to
> > connect immediately. By switching from -daemonize to the
> > virCommandDaemonize API libvirtd was no longer synchronized with
> > QEMU's startup process. The result was that the QEMU monitor
> > failed to open and went into its 200ms sleep loop. This happened
> > for all 25 binaries resulting in 5 seconds worth of sleeping
> > at libvirtd startup. In addition sometimes when libvirt connected,
> > QEMU would be partially initialized and crash causing total
> > failure to probe that binary.
> >
> > This commit reverts the previous change, ensuring we do use the
> > -daemonize flag to QEMU. Startup delay is cut from 7 seconds
> > to 2 seconds on my machine, which is on a par with what it was
> > prior to the capabilities rewrite.
> >
> > To deal with the fact that QEMU needs to be able to create the
> > pidfile, we switch pidfile location fron runDir to libDir, which
> > QEMU is guaranteed to be able to write to.
> >
> > Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> > ---
> > src/qemu/qemu_capabilities.c | 50 ++++++++++++++++++++++++++++++--------------
> > src/qemu/qemu_capabilities.h | 3 +--
> > 2 files changed, 35 insertions(+), 18 deletions(-)
> >
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index 95fa3da..703179d 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -38,6 +38,7 @@
> > #include "virbitmap.h"
> > #include "virnodesuspend.h"
> > #include "qemu_monitor.h"
> > +#include "virtime.h"
> >
> > #include <sys/stat.h>
> > #include <unistd.h>
> > @@ -916,11 +917,19 @@ virCapsPtr qemuCapsInit(qemuCapsCachePtr cache)
> > * so just probe for them all - we gracefully fail
> > * if a qemu-system-$ARCH binary can't be found
> > */
> > - for (i = 0 ; i < VIR_ARCH_LAST ; i++)
> > + unsigned long long a, b;
> > + for (i = 0 ; i < VIR_ARCH_LAST ; i++) {
> > + unsigned long long start, end;
> > + if (virTimeMillisNow(&start) < 0)
> > + goto error;
> > if (qemuCapsInitGuest(caps, cache,
> > virArchFromHost(),
> > i) < 0)
> > goto error;
> > + if (virTimeMillisNow(&end) < 0)
> > + goto error;
> > + VIR_DEBUG("Probed %s in %llums", virArchToString(i), end-start);
> > + }
>
> Did you intend to leave in this debug code? (if you did, you need to
> move the definition of a & b up to the top of the block, and maybe
> rename them to something more descriptive)
No, it should have been removed.
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