On 01/24/2013 11:34 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> 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(-)
>
> @@ -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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list