Re: [libvirt] [PATCH 10/11] qemu: switch QEMU capabilities to use virFileCache

On Mon, Jul 10, 2017 at 14:46:49 +0200, Pavel Hrdina wrote: > The switch contains considerable amount of changes: > > virQEMUCapsRememberCached() is removed because this is now handled > by virFileCacheSave(). > > virQEMUCapsInitCached() is removed because this is now handled by > virFileCacheLoad(). > > virQEMUCapsNewForBinary() is split into two functions, > virQEMUCapsNewData() which creates new data if there is nothing > cached and virQEMUCapsLoadFile() which loads the cached data. > This is now handled by virFileCacheNewData(). > > virQEMUCapsCacheValidate() is removed because this is now handled by > virFileCacheValidate(). > > virQEMUCapsCacheFree() is removed because it's no longer required. > > Add virCapsPtr into virQEMUCapsCachePriv because for each call of > virFileCacheLookup*() we need to use current virCapsPtr. > > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > src/qemu/qemu_capabilities.c | 322 ++++++++++++------------------------------- > src/qemu/qemu_capabilities.h | 17 +-- > src/qemu/qemu_capspriv.h | 8 +- > src/qemu/qemu_conf.h | 3 +- > src/qemu/qemu_driver.c | 2 +- > tests/testutilsqemu.c | 9 +- > tests/testutilsqemu.h | 3 +- > 7 files changed, 102 insertions(+), 262 deletions(-) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index e190cfa8b1..c3e09616de 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c ... > virQEMUCapsPtr > virQEMUCapsCacheLookup(virCapsPtr caps, > - virQEMUCapsCachePtr cache, > + virFileCachePtr cache, > const char *binary) > { > virQEMUCapsPtr ret = NULL; > + virQEMUCapsCachePrivPtr priv = virFileCacheGetPriv(cache); > > - virMutexLock(&cache->lock); > - > - ret = virHashLookup(cache->binaries, binary); > - virQEMUCapsCacheValidate(cache, binary, caps, &ret); > - virObjectRef(ret); > - > - virMutexUnlock(&cache->lock); > + priv->caps = caps; > + ret = virFileCacheLookup(cache, binary); > + priv->caps = NULL; This is not safe anymore since the cache is locked only inside virFileCacheLookup. I think you will need to use a new parameter for passing the capabilities to virFileCacheLookup.