Re: [PATCH v3 2/2] tests/9pfs: fix test dir for parallel tests

[Greg Kurz]

On Sun, 1 Nov 2020 15:37:12 +0100
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> Use mkdtemp() to generate a unique directory for the 9p 'local' tests.
> 
> This fixes occasional 9p test failures when running 'make check -jN' if
> QEMU was compiled for multiple target architectures, because the individual
> architecture's test suites would run in parallel and interfere with each
> other's data as the test directory was previously hard coded and hence the
> same directory was used by all of them simultaniously.
> 
> This also requires a change how the test directory is created and deleted:
> As the test path is now randomized and virtio_9p_register_nodes() being
> called in a somewhat undeterministic way, that's no longer an appropriate
> place to create and remove the test directory. Use a constructor and
> destructor function for creating and removing the test directory instead.
> Unfortunately libqos currently does not support setup/teardown callbacks
> to handle this more cleanly.
> 
> The constructor functions needs to be in virtio-9p-test.c, not in
> virtio-9p.c, because in the latter location it would cause all apps that
> link to libqos (i.e. entirely unrelated test suites) to create a 9pfs
> test directory as well, which would even break other test suites.
> 
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

I could run 'make check -j' with 4 archs (ppc64, x86_64, aarch64, s390x)
on a POWER9 system with 128 cpus, for ~1 hour without seeing any failure.

Tested-by: Greg Kurz <groug@kaod.org>

>  tests/qtest/libqos/virtio-9p.c | 14 ++++++++------
>  tests/qtest/virtio-9p-test.c   | 12 ++++++++++++
>  2 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qtest/libqos/virtio-9p.c b/tests/qtest/libqos/virtio-9p.c
> index 2736e9ae2a..586e700b24 100644
> --- a/tests/qtest/libqos/virtio-9p.c
> +++ b/tests/qtest/libqos/virtio-9p.c
> @@ -35,7 +35,12 @@ static char *concat_path(const char* a, const char* b)
>  static void init_local_test_path(void)
>  {
>      char *pwd = g_get_current_dir();
> -    local_test_path = concat_path(pwd, "qtest-9p-local");
> +    char *template = concat_path(pwd, "qtest-9p-local-XXXXXX");
> +    local_test_path = mkdtemp(template);
> +    if (!local_test_path) {
> +        g_test_message("mkdtemp('%s') failed: %s", template, 
> strerror(errno));
> +    }
> +    g_assert(local_test_path);
>      g_free(pwd);
>  }
>  
> @@ -43,6 +48,8 @@ void virtio_9p_create_local_test_dir(void)
>  {
>      struct stat st;
>  
> +    init_local_test_path();
> +
>      g_assert(local_test_path != NULL);
>      mkdir(local_test_path, 0777);
>  
> @@ -244,11 +251,6 @@ static void virtio_9p_register_nodes(void)
>      const char *str_simple = "fsdev=fsdev0,mount_tag=" MOUNT_TAG;
>      const char *str_addr = "fsdev=fsdev0,addr=04.0,mount_tag=" MOUNT_TAG;
>  
> -    /* make sure test dir for the 'local' tests exists and is clean */
> -    init_local_test_path();
> -    virtio_9p_remove_local_test_dir();
> -    virtio_9p_create_local_test_dir();
> -
>      QPCIAddress addr = {
>          .devfn = QPCI_DEVFN(4, 0),
>      };
> diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> index c15908f27b..6401d4f564 100644
> --- a/tests/qtest/virtio-9p-test.c
> +++ b/tests/qtest/virtio-9p-test.c
> @@ -1076,3 +1076,15 @@ static void register_virtio_9p_test(void)
>  }
>  
>  libqos_init(register_virtio_9p_test);
> +
> +static void __attribute__((constructor)) construct_9p_test(void)
> +{
> +    /* make sure test dir for the 'local' tests exists */
> +    virtio_9p_create_local_test_dir();
> +}
> +
> +static void __attribute__((destructor)) destruct_9p_test(void)
> +{
> +    /* remove previously created test dir when test suite completed */
> +    virtio_9p_remove_local_test_dir();
> +}