On 06/02/2017 12:07 PM, Andrea Bolognani wrote:
> Later on we're going to need access to information about IOMMU
> groups for host devices. Implement the support in virpcimock,
> and start using that mock library in a few QEMU test cases.
> ---
> tests/qemumemlocktest.c | 21 ++++++++++++++++++++-
> tests/qemuxml2argvtest.c | 25 ++++++++++++++++++++++---
> tests/qemuxml2xmltest.c | 22 +++++++++++++++++++++-
> tests/virpcimock.c | 43 +++++++++++++++++++++++++++++++++++++------
> 4 files changed, 100 insertions(+), 11 deletions(-)
>
> diff --git a/tests/qemumemlocktest.c b/tests/qemumemlocktest.c
> index 6cf17a4..c0f1dc3 100644
> --- a/tests/qemumemlocktest.c
> +++ b/tests/qemumemlocktest.c
> @@ -56,11 +56,25 @@ testCompareMemLock(const void *data)
> return ret;
> }
>
> +# define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XXXXXX"
>
> static int
> mymain(void)
> {
> int ret = 0;
> + char *fakerootdir;
> +
> + if (VIR_STRDUP_QUIET(fakerootdir, FAKEROOTDIRTEMPLATE) < 0) {
> + fprintf(stderr, "Out of memory\n");
> + abort();
> + }
> +
> + if (!mkdtemp(fakerootdir)) {
> + fprintf(stderr, "Cannot create fakerootdir");
> + abort();
> + }
> +
> + setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, 1);
I see all this repetition of boilerplate code and wonder if maybe it
should be done in one common place. Not a topic for this series though.
>
> abs_top_srcdir = getenv("abs_top_srcdir");
> if (!abs_top_srcdir)
> @@ -124,12 +138,17 @@ mymain(void)
> DO_TEST("pseries-hardlimit+locked+hostdev", 2147483648);
> DO_TEST("pseries-locked+hostdev", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED);
>
> + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
> + virFileDeleteTree(fakerootdir);
> +
> qemuTestDriverFree(&driver);
> + VIR_FREE(fakerootdir);
>
> return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
> }
>
> -VIR_TEST_MAIN(mymain)
> +VIR_TEST_MAIN_PRELOAD(mymain,
> + abs_builddir "/.libs/virpcimock.so")
>
> #else
>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index b360185..cc6a8a8 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -534,13 +534,27 @@ testCompareXMLToArgv(const void *data)
> return ret;
> }
>
> +# define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XXXXXX"
>
> static int
> mymain(void)
> {
> int ret = 0;
> + char *fakerootdir;
> bool skipLegacyCPUs = false;
>
> + if (VIR_STRDUP_QUIET(fakerootdir, FAKEROOTDIRTEMPLATE) < 0) {
> + fprintf(stderr, "Out of memory\n");
> + abort();
Any particular reason (other than "that's what everybody else is doing")
that you use a combination of fprintf+abort here, but the ABORT macro in
other places?
> + }
> +
> + if (!mkdtemp(fakerootdir)) {
> + fprintf(stderr, "Cannot create fakerootdir");
> + abort();
> + }
> +
> + setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, 1);
> +
> abs_top_srcdir = getenv("abs_top_srcdir");
> if (!abs_top_srcdir)
> abs_top_srcdir = abs_srcdir "/..";
> @@ -2567,15 +2581,20 @@ mymain(void)
> DO_TEST_PARSE_ERROR("cpu-cache-passthrough3", QEMU_CAPS_KVM);
> DO_TEST_PARSE_ERROR("cpu-cache-passthrough-l3", QEMU_CAPS_KVM);
>
> + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
> + virFileDeleteTree(fakerootdir);
> +
> qemuTestDriverFree(&driver);
> + VIR_FREE(fakerootdir);
>
> return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
> }
>
> VIR_TEST_MAIN_PRELOAD(mymain,
> - abs_builddir "/.libs/qemuxml2argvmock.so",
> - abs_builddir "/.libs/virrandommock.so",
> - abs_builddir "/.libs/qemucpumock.so")
> + abs_builddir "/.libs/qemuxml2argvmock.so",
> + abs_builddir "/.libs/virrandommock.so",
> + abs_builddir "/.libs/qemucpumock.so",
> + abs_builddir "/.libs/virpcimock.so")
>
> #else
>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index fff13e2..3c8dbc4 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -303,14 +303,28 @@ testInfoSet(struct testInfo *info,
> return -1;
> }
>
> +# define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XXXXXX"
>
> static int
> mymain(void)
> {
> int ret = 0;
> + char *fakerootdir;
> struct testInfo info;
> virQEMUDriverConfigPtr cfg = NULL;
>
> + if (VIR_STRDUP_QUIET(fakerootdir, FAKEROOTDIRTEMPLATE) < 0) {
> + fprintf(stderr, "Out of memory\n");
> + abort();
> + }
> +
> + if (!mkdtemp(fakerootdir)) {
> + fprintf(stderr, "Cannot create fakerootdir");
> + abort();
> + }
> +
> + setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, 1);
> +
> memset(&info, 0, sizeof(info));
>
> if (qemuTestDriverInit(&driver) < 0)
> @@ -1137,12 +1151,18 @@ mymain(void)
> DO_TEST("cpu-check-default-partial", NONE);
> DO_TEST("cpu-check-default-partial2", NONE);
>
> + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
> + virFileDeleteTree(fakerootdir);
> +
> qemuTestDriverFree(&driver);
> + VIR_FREE(fakerootdir);
>
> return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
> }
>
> -VIR_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/qemuxml2xmlmock.so")
> +VIR_TEST_MAIN_PRELOAD(mymain,
> + abs_builddir "/.libs/qemuxml2xmlmock.so",
> + abs_builddir "/.libs/virpcimock.so")
>
> #else
>
> diff --git a/tests/virpcimock.c b/tests/virpcimock.c
> index e9408aa..19b10f4 100644
> --- a/tests/virpcimock.c
> +++ b/tests/virpcimock.c
> @@ -127,6 +127,7 @@ struct pciDevice {
> int vendor;
> int device;
> int class;
> + int iommuGroup;
> struct pciDriver *driver; /* Driver attached. NULL if attached to no driver */
> };
>
> @@ -190,6 +191,22 @@ make_file(const char *path,
> VIR_FREE(filepath);
> }
>
> +static void
> +make_symlink(const char *path,
> + const char *name,
> + const char *target)
> +{
> + char *filepath = NULL;
> +
> + if (virAsprintfQuiet(&filepath, "%s/%s", path, name) < 0)
> + ABORT_OOM();
> +
> + if (symlink(target, filepath) < 0)
> + ABORT("Unable to create symlink filepath -> target");
> +
> + VIR_FREE(filepath);
> +}
> +
> static int
> pci_read_file(const char *path,
> char *buf,
> @@ -322,7 +339,7 @@ pci_device_new_from_stub(const struct pciDevice *data)
> char *id;
> char *c;
> char *configSrc;
> - char tmp[32];
> + char tmp[256];
> struct stat sb;
>
> if (VIR_STRDUP_QUIET(id, data->id) < 0)
> @@ -386,6 +403,20 @@ pci_device_new_from_stub(const struct pciDevice *data)
> ABORT("@tmp overflow");
> make_file(devpath, "class", tmp, -1);
>
> + if (snprintf(tmp, sizeof(tmp),
> + "%s/../../../kernel/iommu_groups/%d",
> + devpath, dev->iommuGroup) < 0) {
> + ABORT("@tmp overflow");
> + }
> + if (virFileMakePath(tmp) < 0)
> + ABORT("Unable to create %s", tmp);
> +
> + if (snprintf(tmp, sizeof(tmp),
> + "../../../kernel/iommu_groups/%d", dev->iommuGroup) < 0) {
> + ABORT("@tmp overflow");
> + }
> + make_symlink(devpath, "iommu_group", tmp);
> +
> if (pci_device_autobind(dev) < 0)
> ABORT("Unable to bind: %s", data->id);
>
> @@ -821,12 +852,12 @@ init_env(void)
> MAKE_PCI_DEVICE("0000:00:02.0", 0x8086, 0x0046);
> MAKE_PCI_DEVICE("0000:00:03.0", 0x8086, 0x0048);
> MAKE_PCI_DEVICE("0001:00:00.0", 0x1014, 0x03b9, .class = 0x060400);
> - MAKE_PCI_DEVICE("0001:01:00.0", 0x8086, 0x105e);
> - MAKE_PCI_DEVICE("0001:01:00.1", 0x8086, 0x105e);
> + MAKE_PCI_DEVICE("0001:01:00.0", 0x8086, 0x105e, .iommuGroup = 2);
> + MAKE_PCI_DEVICE("0001:01:00.1", 0x8086, 0x105e, .iommuGroup = 2);
> MAKE_PCI_DEVICE("0005:80:00.0", 0x10b5, 0x8112, .class = 0x060400);
> - MAKE_PCI_DEVICE("0005:90:01.0", 0x1033, 0x0035);
> - MAKE_PCI_DEVICE("0005:90:01.1", 0x1033, 0x0035);
> - MAKE_PCI_DEVICE("0005:90:01.2", 0x1033, 0x00e0);
> + MAKE_PCI_DEVICE("0005:90:01.0", 0x1033, 0x0035, .iommuGroup = 3);
> + MAKE_PCI_DEVICE("0005:90:01.1", 0x1033, 0x0035, .iommuGroup = 3);
> + MAKE_PCI_DEVICE("0005:90:01.2", 0x1033, 0x00e0, .iommuGroup = 3);
> MAKE_PCI_DEVICE("0000:0a:01.0", 0x8086, 0x0047);
> MAKE_PCI_DEVICE("0000:0a:02.0", 0x8286, 0x0048);
> MAKE_PCI_DEVICE("0000:0a:03.0", 0x8386, 0x0048);
>
Reviewed-by: Laine Stump <laine@laine.org>
(feel free to include in the commit log or not :-P)
(also, I'm assuming that you've run make check && make syntax-check on
each patch individually. Otherwise all ACKs are null and void)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list