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

[Eric Blake]

On 01/24/2013 11:34 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>

> 
> 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 at redhat.com>
> ---
>  src/qemu/qemu_capabilities.c | 50 ++++++++++++++++++++++++++++++--------------
>  src/qemu/qemu_capabilities.h |  3 +--
>  2 files changed, 35 insertions(+), 18 deletions(-)
> 
> @@ -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?

> +    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 '-'

> +    }
>  
>      /* 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?


> +    /*
> +     * We explicitly need to use -daemonize here, rather than
> +     * virCommandDaemonize, because we need to synchronize
> +     * with QEMU creating its monitor socket API. Using
> +     * daemonize guarnatees control won't return to libvirt

s/guarnatees/guarantees/

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130124/1b35f320/attachment-0001.sig>