On Mon, Mar 10, 2014 at 12:50:20PM -0600, Eric Blake wrote:
> 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
> > + */
> > +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?
That's a bug, but thanks to xpath it just happens to still work.
./foo means match '<foo>' as an immediate child of current node.
.//foo means match '<foo>' as an arbitrarily deep child of the
current node.
> > +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 :)
Well it doesn't conflict so much as mean Laine has more work todo :-P
> > +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?
It should reset usedQMP, but not ctime - the ctime value always
matches the current QEMU binary and we don't overwrite that when
loading the XML.
Regards,
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