[PATCH] migration-test: ppc64: fix FORTH test program

Laurent Vivier posted 1 patch 4 years, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200107163437.52139-1-lvivier@redhat.com
Test asan failed
Test checkpatch passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu failed
Test docker-quick@centos7 passed
Maintainers: Laurent Vivier <lvivier@redhat.com>, Thomas Huth <thuth@redhat.com>, Juan Quintela <quintela@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>
tests/migration-test.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] migration-test: ppc64: fix FORTH test program
Posted by Laurent Vivier 4 years, 3 months ago
Commit e51e711b1bef has moved the initialization of start_address and
end_address after the definition of the command line argument,
where the nvramrc is initialized, and thus the loop is between 0 and 0
rather than 1 MiB and 100 MiB.

It doesn't affect the result of the test if all the tests are run in
sequence because the two first tests don't run the loop, so the
values are correctly initialized when we actually need them.

But it hangs when we ask to run only one test, for instance:

    QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 \
    tests/migration-test -m=quick -p /ppc64/migration/validate_uuid_error

Fixes: e51e711b1bef ("tests/migration: Add migration-test header file")
Cc: wei@redhat.com
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 tests/migration-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 53afec439522..341d19092214 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -480,14 +480,14 @@ static int test_migrate_start(QTestState **from, QTestState **to,
     } else if (strcmp(arch, "ppc64") == 0) {
         machine_opts = "vsmt=8";
         memory_size = "256M";
+        start_address = PPC_TEST_MEM_START;
+        end_address = PPC_TEST_MEM_END;
         arch_source = g_strdup_printf("-nodefaults "
                                       "-prom-env 'use-nvramrc?=true' -prom-env "
                                       "'nvramrc=hex .\" _\" begin %x %x "
                                       "do i c@ 1 + i c! 1000 +loop .\" B\" 0 "
                                       "until'", end_address, start_address);
         arch_target = g_strdup("");
-        start_address = PPC_TEST_MEM_START;
-        end_address = PPC_TEST_MEM_END;
     } else if (strcmp(arch, "aarch64") == 0) {
         init_bootfile(bootpath, aarch64_kernel, sizeof(aarch64_kernel));
         machine_opts = "virt,gic-version=max";
-- 
2.23.0


Re: [PATCH] migration-test: ppc64: fix FORTH test program
Posted by Dr. David Alan Gilbert 4 years, 3 months ago
* Laurent Vivier (lvivier@redhat.com) wrote:
> Commit e51e711b1bef has moved the initialization of start_address and
> end_address after the definition of the command line argument,
> where the nvramrc is initialized, and thus the loop is between 0 and 0
> rather than 1 MiB and 100 MiB.
> 
> It doesn't affect the result of the test if all the tests are run in
> sequence because the two first tests don't run the loop, so the
> values are correctly initialized when we actually need them.
> 
> But it hangs when we ask to run only one test, for instance:
> 
>     QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 \
>     tests/migration-test -m=quick -p /ppc64/migration/validate_uuid_error
> 
> Fixes: e51e711b1bef ("tests/migration: Add migration-test header file")
> Cc: wei@redhat.com
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  tests/migration-test.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 53afec439522..341d19092214 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -480,14 +480,14 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>      } else if (strcmp(arch, "ppc64") == 0) {
>          machine_opts = "vsmt=8";
>          memory_size = "256M";
> +        start_address = PPC_TEST_MEM_START;
> +        end_address = PPC_TEST_MEM_END;
>          arch_source = g_strdup_printf("-nodefaults "
>                                        "-prom-env 'use-nvramrc?=true' -prom-env "
>                                        "'nvramrc=hex .\" _\" begin %x %x "
>                                        "do i c@ 1 + i c! 1000 +loop .\" B\" 0 "
>                                        "until'", end_address, start_address);
>          arch_target = g_strdup("");
> -        start_address = PPC_TEST_MEM_START;
> -        end_address = PPC_TEST_MEM_END;
>      } else if (strcmp(arch, "aarch64") == 0) {
>          init_bootfile(bootpath, aarch64_kernel, sizeof(aarch64_kernel));
>          machine_opts = "virt,gic-version=max";
> -- 
> 2.23.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


Re: [PATCH] migration-test: ppc64: fix FORTH test program
Posted by Juan Quintela 4 years, 3 months ago
Laurent Vivier <lvivier@redhat.com> wrote:
> Commit e51e711b1bef has moved the initialization of start_address and
> end_address after the definition of the command line argument,
> where the nvramrc is initialized, and thus the loop is between 0 and 0
> rather than 1 MiB and 100 MiB.
>
> It doesn't affect the result of the test if all the tests are run in
> sequence because the two first tests don't run the loop, so the
> values are correctly initialized when we actually need them.
>
> But it hangs when we ask to run only one test, for instance:
>
>     QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 \
>     tests/migration-test -m=quick -p /ppc64/migration/validate_uuid_error
>
> Fixes: e51e711b1bef ("tests/migration: Add migration-test header file")
> Cc: wei@redhat.com
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

queued.


Re: [PATCH] migration-test: ppc64: fix FORTH test program
Posted by David Gibson 4 years, 3 months ago
On Tue, Jan 07, 2020 at 05:34:37PM +0100, Laurent Vivier wrote:
> Commit e51e711b1bef has moved the initialization of start_address and
> end_address after the definition of the command line argument,
> where the nvramrc is initialized, and thus the loop is between 0 and 0
> rather than 1 MiB and 100 MiB.
> 
> It doesn't affect the result of the test if all the tests are run in
> sequence because the two first tests don't run the loop, so the
> values are correctly initialized when we actually need them.
> 
> But it hangs when we ask to run only one test, for instance:
> 
>     QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 \
>     tests/migration-test -m=quick -p /ppc64/migration/validate_uuid_error
> 
> Fixes: e51e711b1bef ("tests/migration: Add migration-test header file")
> Cc: wei@redhat.com
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  tests/migration-test.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 53afec439522..341d19092214 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -480,14 +480,14 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>      } else if (strcmp(arch, "ppc64") == 0) {
>          machine_opts = "vsmt=8";
>          memory_size = "256M";
> +        start_address = PPC_TEST_MEM_START;
> +        end_address = PPC_TEST_MEM_END;
>          arch_source = g_strdup_printf("-nodefaults "
>                                        "-prom-env 'use-nvramrc?=true' -prom-env "
>                                        "'nvramrc=hex .\" _\" begin %x %x "
>                                        "do i c@ 1 + i c! 1000 +loop .\" B\" 0 "
>                                        "until'", end_address, start_address);
>          arch_target = g_strdup("");
> -        start_address = PPC_TEST_MEM_START;
> -        end_address = PPC_TEST_MEM_END;
>      } else if (strcmp(arch, "aarch64") == 0) {
>          init_bootfile(bootpath, aarch64_kernel, sizeof(aarch64_kernel));
>          machine_opts = "virt,gic-version=max";

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson