[libvirt] [PATCH v2 3/3] Cache result of QEMU capabilities extraction

[Eric Blake]

On 03/10/2014 10:54 AM, Daniel P. Berrange wrote:
> Extracting capabilities from QEMU takes a notable amount of time
> when all QEMU binaries are installed. Each system emulator
> needs about 200-300ms multiplied by 26 binaries == ~5-8 seconds.
> 
> This change causes the QEMU driver to save an XML file containing
> the content of the virQEMUCaps object instance in the cache
> dir eg /var/cache/libvirt/qemu/capabilities/$SHA256(binarypath).xml
> or $HOME/.cache/libvirt/qemu/cache/capabilities/$SHA256(binarypath).xml

Good, http://www.tldp.org/LDP/Linux-Filesystem-Hierarchy/html/var.html
states that /var/cache normally persists across reboots.

> 
> We attempt to load this and only if it fails, do we fallback to
> probing the QEMU binary. The ctime of the QEMU binary and libvirtd
> are stored in the cached file and its data discarded if either
> of them change.
> 
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  src/qemu/qemu_capabilities.c | 436 ++++++++++++++++++++++++++++++++++++++++++-
>  src/qemu/qemu_capabilities.h |   2 +
>  src/qemu/qemu_driver.c       |   1 +
>  3 files changed, 431 insertions(+), 8 deletions(-)
> 
> +
> +/*
> + * Update the XML parser/formatter when adding more
> + * information to this struct so that it gets cached
> + * correctly. It does not have to be ABI-stable. A
> + * newer libvirtd will simply discard the cache if
> + * it fails to parse the XML doc and save a new one.

A newer libvirtd will discard the cache anyways, because the timestamp
differs from the cached ctime.


> +/*
> + * Parsing a doc that looks like
> + *
> + * <qemuCaps>
> + *   <usedQMP/>
> + *   <flag name='foo'/>
> + *   <flag name='bar'/>
> + *   ...
> + *   <cpu name="pentium3"/>
> + *   ...
> + *   <machine name="pc-1.0" alias="pc" maxCpus="4"/>
> + *   ...
> + * </qemuCaps>

Ought to mention the cached ctime elements.

> + */
> +static int
> +virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename,
> +                     time_t *qemuctime, time_t *selfctime)
> +{

> +    if (virXPathLongLong("string(./qemuctime)", ctxt, &l) < 0) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("missing qemuctime in QEMU capabilities XML"));

> +    qemuCaps->usedQMP = virXPathBoolean("count(.//usedQMP) > 0",

Why ./qemuctime but .//usedQMP?  What difference does // make in XPath?

> +
> +static int
> +virQEMUCapsSaveCache(virQEMUCapsPtr qemuCaps, const char *filename)
> +{
> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
> +    const char *xml = NULL;
> +    int ret = -1;
> +    size_t i;
> +
> +    virBufferAddLit(&buf, "<qemuCaps>\n");
> +
> +    virBufferAsprintf(&buf, "  <qemuctime>%llu</qemuctime>\n",

Conflicts with Laine's work to require virBufferAddIndent() rather than
hard-coding indentation.  Will be interesting to see who gets in first :)


> +static void
> +virQEMUCapsReset(virQEMUCapsPtr qemuCaps)
> +{
> +    size_t i;
> +
> +    virBitmapClearAll(qemuCaps->flags);
> +    qemuCaps->version = qemuCaps->kvmVersion = 0;
> +    qemuCaps->arch = VIR_ARCH_NONE;

Shouldn't you also reset qemuCaps->usedQMP and qemuCaps->ctime, to get
the struct back to a known-default state?  Or is this only ever going to
be used just before freeing the struct, at which point all the
reset-to-0 code is wasted CPU cycles?

-- 
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: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140310/daff09c7/attachment-0001.sig>