On 15.09.2015 10:05, Ján Tomko wrote:
> From: Pavel Fedin <p.fedin@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxxx>
> + */
> +
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list