[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 05/26] tests: Mock IOMMU groups



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)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]