1. Patchew
  2. QEMU
  3. View series

[PATCH] pseries: fix migration-test and pxe-test

Laurent Vivier posted 1 patch 4 years, 5 months ago
Test asan passed
Test checkpatch passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191120142539.236279-1-lvivier@redhat.com
Maintainers: Thomas Huth <thuth@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Juan Quintela <quintela@redhat.com>
tests/migration-test.c | 4 ++--
tests/pxe-test.c       | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
[PATCH] pseries: fix migration-test and pxe-test
Posted by Laurent Vivier 4 years, 5 months ago 
Commit 29cb4187497d ("spapr: Set VSMT to smp_threads by default")
has introduced a new default value for VSMT that is not supported
by old kernels (before 4.13 kernel) and this breaks "make check"
on these kernels.

To fix that, explicitly set in the involved tests the value that was
used as the default value before the change.

Cc: Greg Kurz <groug@kaod.org>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 tests/migration-test.c | 4 ++--
 tests/pxe-test.c       | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index ac780dffdaad..ebd77a581aff 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -614,7 +614,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
         end_address = S390_TEST_MEM_END;
     } else if (strcmp(arch, "ppc64") == 0) {
         extra_opts = use_shmem ? get_shmem_opts("256M", shmem_path) : NULL;
-        cmd_src = g_strdup_printf("-machine accel=%s -m 256M -nodefaults"
+        cmd_src = g_strdup_printf("-machine accel=%s,vsmt=8 -m 256M -nodefaults"
                                   " -name source,debug-threads=on"
                                   " -serial file:%s/src_serial"
                                   " -prom-env 'use-nvramrc?=true' -prom-env "
@@ -623,7 +623,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
                                   "until' %s %s",  accel, tmpfs, end_address,
                                   start_address, extra_opts ? extra_opts : "",
                                   opts_src);
-        cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
+        cmd_dst = g_strdup_printf("-machine accel=%s,vsmt=8 -m 256M"
                                   " -name target,debug-threads=on"
                                   " -serial file:%s/dest_serial"
                                   " -incoming %s %s %s",
diff --git a/tests/pxe-test.c b/tests/pxe-test.c
index 948b0fbdc727..aaae54f7550d 100644
--- a/tests/pxe-test.c
+++ b/tests/pxe-test.c
@@ -46,15 +46,15 @@ static testdef_t x86_tests_slow[] = {
 
 static testdef_t ppc64_tests[] = {
     { "pseries", "spapr-vlan",
-      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken" },
+      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,vsmt=8" },
     { "pseries", "virtio-net-pci",
-      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken" },
+      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,vsmt=8" },
     { NULL },
 };
 
 static testdef_t ppc64_tests_slow[] = {
     { "pseries", "e1000",
-      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken" },
+      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,vsmt=8" },
     { NULL },
 };
 
-- 
2.23.0
Re: [PATCH] pseries: fix migration-test and pxe-test
Posted by Thomas Huth 4 years, 5 months ago 
On 20/11/2019 15.25, Laurent Vivier wrote:
> Commit 29cb4187497d ("spapr: Set VSMT to smp_threads by default")
> has introduced a new default value for VSMT that is not supported
> by old kernels (before 4.13 kernel) and this breaks "make check"
> on these kernels.
> 
> To fix that, explicitly set in the involved tests the value that was
> used as the default value before the change.
> 
> Cc: Greg Kurz <groug@kaod.org>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  tests/migration-test.c | 4 ++--
>  tests/pxe-test.c       | 6 +++---
>  2 files changed, 5 insertions(+), 5 deletions(-)

Acked-by: Thomas Huth <thuth@redhat.com>

I assume this will go via the ppc tree?

 Thomas
Re: [PATCH] pseries: fix migration-test and pxe-test
Posted by Juan Quintela 4 years, 5 months ago 
Laurent Vivier <lvivier@redhat.com> wrote:
> Commit 29cb4187497d ("spapr: Set VSMT to smp_threads by default")
> has introduced a new default value for VSMT that is not supported
> by old kernels (before 4.13 kernel) and this breaks "make check"
> on these kernels.
>
> To fix that, explicitly set in the involved tests the value that was
> used as the default value before the change.
>
> Cc: Greg Kurz <groug@kaod.org>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

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

A comment telling that never their use could help for future readers
O:-)
Re: [PATCH] pseries: fix migration-test and pxe-test
Posted by Laurent Vivier 4 years, 5 months ago 
On 20/11/2019 15:34, Juan Quintela wrote:
> Laurent Vivier <lvivier@redhat.com> wrote:
>> Commit 29cb4187497d ("spapr: Set VSMT to smp_threads by default")
>> has introduced a new default value for VSMT that is not supported
>> by old kernels (before 4.13 kernel) and this breaks "make check"
>> on these kernels.
>>
>> To fix that, explicitly set in the involved tests the value that was
>> used as the default value before the change.
>>
>> Cc: Greg Kurz <groug@kaod.org>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> 
> A comment telling that never their use could help for future readers
> O:-)

I assumed that "git blame" would be enough for a reader that wants more
details :)

Thanks,
Laurent
Re: [PATCH] pseries: fix migration-test and pxe-test
Posted by Greg Kurz 4 years, 5 months ago 
On Wed, 20 Nov 2019 15:25:39 +0100
Laurent Vivier <lvivier@redhat.com> wrote:

> Commit 29cb4187497d ("spapr: Set VSMT to smp_threads by default")
> has introduced a new default value for VSMT that is not supported
> by old kernels (before 4.13 kernel) and this breaks "make check"
> on these kernels.
> 
> To fix that, explicitly set in the involved tests the value that was
> used as the default value before the change.
> 

Some more details: the 'make check' failure was observed on a POWER8 host
with an old kernel RHEL7 kernel without VSMT support. Passing vsmt=8 to QEMU
causes spapr_set_vsmt_mode() to simply not even try to change the VSMT in KVM
since POWER8 hosts are SMT8 by default, and the VM starts just fine.

> Cc: Greg Kurz <groug@kaod.org>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---

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

I could also check this is doesn't break the following setups:
- KVM HV on POWER9 host
- KVM PR on POWER8 host (just spits a non-fatal warning)
- TCG

Note that migration-test explicitly requires KVM HV. It simply
exits otherwise.

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

>  tests/migration-test.c | 4 ++--
>  tests/pxe-test.c       | 6 +++---
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index ac780dffdaad..ebd77a581aff 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -614,7 +614,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>          end_address = S390_TEST_MEM_END;
>      } else if (strcmp(arch, "ppc64") == 0) {
>          extra_opts = use_shmem ? get_shmem_opts("256M", shmem_path) : NULL;
> -        cmd_src = g_strdup_printf("-machine accel=%s -m 256M -nodefaults"
> +        cmd_src = g_strdup_printf("-machine accel=%s,vsmt=8 -m 256M -nodefaults"
>                                    " -name source,debug-threads=on"
>                                    " -serial file:%s/src_serial"
>                                    " -prom-env 'use-nvramrc?=true' -prom-env "
> @@ -623,7 +623,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>                                    "until' %s %s",  accel, tmpfs, end_address,
>                                    start_address, extra_opts ? extra_opts : "",
>                                    opts_src);
> -        cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
> +        cmd_dst = g_strdup_printf("-machine accel=%s,vsmt=8 -m 256M"
>                                    " -name target,debug-threads=on"
>                                    " -serial file:%s/dest_serial"
>                                    " -incoming %s %s %s",
> diff --git a/tests/pxe-test.c b/tests/pxe-test.c
> index 948b0fbdc727..aaae54f7550d 100644
> --- a/tests/pxe-test.c
> +++ b/tests/pxe-test.c
> @@ -46,15 +46,15 @@ static testdef_t x86_tests_slow[] = {
>  
>  static testdef_t ppc64_tests[] = {
>      { "pseries", "spapr-vlan",
> -      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken" },
> +      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,vsmt=8" },
>      { "pseries", "virtio-net-pci",
> -      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken" },
> +      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,vsmt=8" },
>      { NULL },
>  };
>  
>  static testdef_t ppc64_tests_slow[] = {
>      { "pseries", "e1000",
> -      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken" },
> +      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,vsmt=8" },
>      { NULL },
>  };
>
Re: [PATCH] pseries: fix migration-test and pxe-test
Posted by David Gibson 4 years, 5 months ago 
On Wed, Nov 20, 2019 at 03:25:39PM +0100, Laurent Vivier wrote:
> Commit 29cb4187497d ("spapr: Set VSMT to smp_threads by default")
> has introduced a new default value for VSMT that is not supported
> by old kernels (before 4.13 kernel) and this breaks "make check"
> on these kernels.
> 
> To fix that, explicitly set in the involved tests the value that was
> used as the default value before the change.
> 
> Cc: Greg Kurz <groug@kaod.org>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Applied to ppc-for-4.2, thanks.

> ---
>  tests/migration-test.c | 4 ++--
>  tests/pxe-test.c       | 6 +++---
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index ac780dffdaad..ebd77a581aff 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -614,7 +614,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>          end_address = S390_TEST_MEM_END;
>      } else if (strcmp(arch, "ppc64") == 0) {
>          extra_opts = use_shmem ? get_shmem_opts("256M", shmem_path) : NULL;
> -        cmd_src = g_strdup_printf("-machine accel=%s -m 256M -nodefaults"
> +        cmd_src = g_strdup_printf("-machine accel=%s,vsmt=8 -m 256M -nodefaults"
>                                    " -name source,debug-threads=on"
>                                    " -serial file:%s/src_serial"
>                                    " -prom-env 'use-nvramrc?=true' -prom-env "
> @@ -623,7 +623,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>                                    "until' %s %s",  accel, tmpfs, end_address,
>                                    start_address, extra_opts ? extra_opts : "",
>                                    opts_src);
> -        cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
> +        cmd_dst = g_strdup_printf("-machine accel=%s,vsmt=8 -m 256M"
>                                    " -name target,debug-threads=on"
>                                    " -serial file:%s/dest_serial"
>                                    " -incoming %s %s %s",
> diff --git a/tests/pxe-test.c b/tests/pxe-test.c
> index 948b0fbdc727..aaae54f7550d 100644
> --- a/tests/pxe-test.c
> +++ b/tests/pxe-test.c
> @@ -46,15 +46,15 @@ static testdef_t x86_tests_slow[] = {
>  
>  static testdef_t ppc64_tests[] = {
>      { "pseries", "spapr-vlan",
> -      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken" },
> +      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,vsmt=8" },
>      { "pseries", "virtio-net-pci",
> -      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken" },
> +      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,vsmt=8" },
>      { NULL },
>  };
>  
>  static testdef_t ppc64_tests_slow[] = {
>      { "pseries", "e1000",
> -      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken" },
> +      "-machine cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken,vsmt=8" },
>      { NULL },
>  };
>  

-- 
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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.