[libvirt] [PATCH 2/4] Implement infrastracture for mocking up QEMU capabilities cache

[Michal Privoznik]

On 15.09.2015 10:05, Ján Tomko wrote:
> From: Pavel Fedin <p.fedin at samsung.com>
> 
> The main purpose of this patch is to introduce test mode to
> virQEMUCapsCacheLookup(). This is done by adding a global variable, which
> effectively overrides binary name. This variable is supposed to be set by
> test suite.
> 
> The second addition is qemuTestCapsCacheInsert() function which allows the
> test suite to actually populate the cache.
> 
> Signed-off-by: Pavel Fedin <p.fedin at samsung.com>
> Signed-off-by: Ján Tomko <jtomko at redhat.com>
> ---
>  src/qemu/qemu_capabilities.c | 16 +++++++---------
>  src/qemu/qemu_capspriv.h     | 36 ++++++++++++++++++++++++++++++++++++
>  tests/testutilsqemu.c        | 40 ++++++++++++++++++++++++++++++++++++++++
>  tests/testutilsqemu.h        |  5 +++++
>  4 files changed, 88 insertions(+), 9 deletions(-)
>  create mode 100644 src/qemu/qemu_capspriv.h
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 4ad1bdb..9a819d2 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -42,6 +42,7 @@
>  #include "virstring.h"
>  #include "qemu_hostdev.h"
>  #include "qemu_domain.h"
> +#include "qemu_capspriv.h"
>  
>  #include <fcntl.h>
>  #include <sys/stat.h>
> @@ -331,15 +332,6 @@ struct _virQEMUCaps {
>      unsigned int *machineMaxCpus;
>  };
>  
> -struct _virQEMUCapsCache {
> -    virMutex lock;
> -    virHashTablePtr binaries;
> -    char *libDir;
> -    char *cacheDir;
> -    uid_t runUid;
> -    gid_t runGid;
> -};
> -
>  struct virQEMUCapsSearchData {
>      virArch arch;
>  };
> @@ -3718,11 +3710,17 @@ virQEMUCapsCacheNew(const char *libDir,
>      return NULL;
>  }
>  
> +const char *qemuTestCapsName;
>  
>  virQEMUCapsPtr
>  virQEMUCapsCacheLookup(virQEMUCapsCachePtr cache, const char *binary)
>  {
>      virQEMUCapsPtr ret = NULL;
> +
> +    /* This is used only by test suite!!! */
> +    if (qemuTestCapsName)
> +        binary = qemuTestCapsName;
> +
>      virMutexLock(&cache->lock);
>      ret = virHashLookup(cache->binaries, binary);
>      if (ret &&
> diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
> new file mode 100644
> index 0000000..9288dbd
> --- /dev/null
> +++ b/src/qemu/qemu_capspriv.h
> @@ -0,0 +1,36 @@
> +/*
> + * qemu_capspriv.h: private declarations for QEMU capabilities generation
> + *
> + * Copyright (C) 2015 Samsung Electronics Co. Ltd
> + * Copyright (C) 2015 Pavel Fedin
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Author: Pavel Fedin <p.fedin at samsung.com>
> + */
> +

Maybe we want to guard this file so that it cannot be included from a
random .c file? Something like we do in ./src/util/vircommandpriv.h?

> +#ifndef __QEMU_CAPSPRIV_H__
> +# define __QEMU_CAPSPRIV_H__
> +
> +struct _virQEMUCapsCache {
> +    virMutex lock;
> +    virHashTablePtr binaries;
> +    char *libDir;
> +    char *cacheDir;
> +    uid_t runUid;
> +    gid_t runGid;
> +};
> +
> +#endif
> diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
> index 84dfa75..275d22a 100644
> --- a/tests/testutilsqemu.c
> +++ b/tests/testutilsqemu.c
> @@ -8,6 +8,7 @@
>  # include "cpu_conf.h"
>  # include "qemu/qemu_driver.h"
>  # include "qemu/qemu_domain.h"
> +# include "qemu/qemu_capspriv.h"
>  # include "virstring.h"
>  
>  # define VIR_FROM_THIS VIR_FROM_QEMU
> @@ -527,6 +528,32 @@ qemuTestParseCapabilities(const char *capsFile)
>      return NULL;
>  }
>  
> +int qemuTestCapsCacheInsert(virQEMUCapsCachePtr cache, const char *binary,
> +                            virQEMUCapsPtr caps)
> +{
> +    int ret;
> +
> +    if (caps) {
> +        /* Our caps were created artificially, so we don't want
> +         * virQEMUCapsCacheFree() to attempt to deallocate them */
> +        virObjectRef(caps);
> +    } else {
> +        caps = virQEMUCapsNew();
> +        if (!caps)
> +            return -ENOMEM;
> +    }
> +
> +    /* We can have repeating names for our test data sets,
> +     * so make sure there's no old copy */
> +    virHashRemoveEntry(cache->binaries, binary);
> +
> +    ret = virHashAddEntry(cache->binaries, binary, caps);
> +    if (ret < 0)
> +        virObjectUnref(caps);

I think we want to set @qemuTestCapsName here:

if (ret < 0)
  virObjectUnref(caps)
else
  qemuTestCapsName = binary;

> +
> +    return ret;
> +}
> +
>  int qemuTestDriverInit(virQEMUDriver *driver)
>  {
>      driver->config = virQEMUDriverConfigNew(false);
> @@ -537,13 +564,24 @@ int qemuTestDriverInit(virQEMUDriver *driver)
>      if (!driver->caps)
>          goto error;
>  
> +    /* Using /dev/null for libDir and cacheDir automatically produces errors
> +     * upon attempt to use any of them */
> +    driver->qemuCapsCache = virQEMUCapsCacheNew("/dev/null", "/dev/null", 0, 0);
> +    if (!driver->qemuCapsCache)
> +        goto error;
> +
>      driver->xmlopt = virQEMUDriverCreateXMLConf(driver);
>      if (!driver->xmlopt)
>          goto error;
>  
> +    qemuTestCapsName = "empty";
> +    if (qemuTestCapsCacheInsert(driver->qemuCapsCache, "empty", NULL) < 0)
> +        goto error;
> +
>      return 0;
>  
>   error:
> +    virQEMUCapsCacheFree(driver->qemuCapsCache);
>      virObjectUnref(driver->caps);
>      virObjectUnref(driver->config);
>      virObjectUnref(driver->xmlopt);
> @@ -552,8 +590,10 @@ int qemuTestDriverInit(virQEMUDriver *driver)
>  
>  void qemuTestDriverFree(virQEMUDriver *driver)
>  {
> +    virQEMUCapsCacheFree(driver->qemuCapsCache);
>      virObjectUnref(driver->xmlopt);
>      virObjectUnref(driver->caps);
>      virObjectUnref(driver->config);
>  }
> +
>  #endif
> diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h
> index 6c2d3b5..1d5dfc5 100644
> --- a/tests/testutilsqemu.h
> +++ b/tests/testutilsqemu.h
> @@ -18,4 +18,9 @@ void testQemuCapsSetCPU(virCapsPtr caps,
>  
>  int qemuTestDriverInit(virQEMUDriver *driver);
>  void qemuTestDriverFree(virQEMUDriver *driver);
> +int qemuTestCapsCacheInsert(virQEMUCapsCachePtr cache, const char *binary,
> +                            virQEMUCapsPtr caps);
> +
> +/* This variable is actually defined in src/qemu/qemu_capabilities.c */
> +extern const char *qemuTestCapsName;
>  #endif
> 


Otherwise looking good.

Michal