pseries: fix migration-test and pxe-test
diff mbox series

Message ID 20191120142539.236279-1-lvivier@redhat.com
State New
Headers show
Series
  • pseries: fix migration-test and pxe-test
Related show

Commit Message

Laurent Vivier Nov. 20, 2019, 2:25 p.m. UTC
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(-)

Comments

Thomas Huth Nov. 20, 2019, 2:34 p.m. UTC | #1
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
Juan Quintela Nov. 20, 2019, 2:34 p.m. UTC | #2
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:-)
Laurent Vivier Nov. 20, 2019, 2:38 p.m. UTC | #3
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
Greg Kurz Nov. 20, 2019, 3:03 p.m. UTC | #4
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 },
>  };
>
David Gibson Nov. 21, 2019, 1:26 a.m. UTC | #5
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 },
>  };
>

Patch
diff mbox series

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 },
 };