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@xxxxxxxxxx>
> ---
> 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.
>
> VIR_DEBUG("Returning caps %p for %s", ret, binary);
> return ret;
...
> @@ -5478,64 +5351,39 @@ virQEMUCapsCompareArch(const void *payload,
>
> virQEMUCapsPtr
> virQEMUCapsCacheLookupByArch(virCapsPtr caps,
> - virQEMUCapsCachePtr cache,
> + virFileCachePtr cache,
> virArch arch)
> {
> virQEMUCapsPtr ret = NULL;
> + virQEMUCapsCachePrivPtr priv = virFileCacheGetPriv(cache);
> virArch target;
> struct virQEMUCapsSearchData data = { .arch = arch };
>
> - virMutexLock(&cache->lock);
> - ret = virHashSearch(cache->binaries, virQEMUCapsCompareArch, &data, NULL);
> + priv->caps = caps;
> + ret = virFileCacheLookupByFunc(cache, virQEMUCapsCompareArch, &data);
> if (!ret) {
> /* If the first attempt at finding capabilities has failed, try
> * again using the QEMU target as lookup key instead */
> target = virQEMUCapsFindTarget(virArchFromHost(), data.arch);
> if (target != data.arch) {
> data.arch = target;
> - ret = virHashSearch(cache->binaries, virQEMUCapsCompareArch,
> - &data, NULL);
> + ret = virFileCacheLookupByFunc(cache, virQEMUCapsCompareArch, &data);
> }
> }
> + priv->caps = NULL;
This is unsafe too.
>
> - if (ret) {
> - char *binary;
> -
> - if (VIR_STRDUP(binary, ret->binary) < 0) {
> - ret = NULL;
> - } else {
> - virQEMUCapsCacheValidate(cache, binary, caps, &ret);
> - VIR_FREE(binary);
> - }
> - } else {
> + if (!ret) {
> virReportError(VIR_ERR_INVALID_ARG,
> _("unable to find any emulator to serve '%s' "
> "architecture"), virArchToString(arch));
> }
>
> - virObjectRef(ret);
> - virMutexUnlock(&cache->lock);
> -
> VIR_DEBUG("Returning caps %p for arch %s", ret, virArchToString(arch));
>
> return ret;
> }
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
