[libvirt] [PATCH] Fix performance & reliabilty of QMP probing

[Daniel P. Berrange]

On Thu, Jan 24, 2013 at 12:57:48PM -0700, Eric Blake wrote:
> On 01/24/2013 11:34 AM, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange at redhat.com>
> > @@ -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;
> 
> What are 'a' and 'b' for?

Left over debug code.

> 
> > +    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);
> 
> spaces around '-'

Opps, all this left over debug code should have been removed.

> 
> > +    }
> >  
> >      /* QEMU Requires an emulator in the XML */
> >      virCapabilitiesSetEmulatorRequired(caps);
> > @@ -2291,7 +2300,6 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps)
> >  static int
> >  qemuCapsInitQMP(qemuCapsPtr caps,
> >                  const char *libDir,
> > -                const char *runDir,
> >                  uid_t runUid,
> >                  gid_t runGid)
> >  {
> > @@ -2324,8 +2332,11 @@ qemuCapsInitQMP(qemuCapsPtr caps,
> >  
> >      /* ".pidfile" suffix is used rather than ".pid" to avoid a possible clash
> >       * with a qemu domain called "capabilities"
> > +     * Normally we'd use runDir for pid files, but because we're using
> > +     * -daemonize we need QEMU to be allowed to create them, rather
> > +     * than libvirtd. So we're using libDir which QEMU can write to
> >       */
> > -    if (virAsprintf(&pidfile, "%s/%s", runDir, "capabilities.pidfile") < 0) {
> > +    if (virAsprintf(&pidfile, "%s/%s", libDir, "capabilities.pidfile") < 0) {
> 
> Will this change work across an upgrade?  That is, if I have a qemu
> already running under libvirtd with the pid file in the old location,
> and then restart to a newer libvirtd, do we ever try to read the pidfile
> again, and fail because it is not in the right location?

This code is only executed while probing capabilities, so the QEMU
is only running for a fraction of the second. The actual QEMU VM
behaviour is unchanged, so there's no upgrade issue.

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 :|