[PATCH] tests: Fix migration-test build failure for sparc

[PATCH] tests: Fix migration-test build failure for sparc

[Peter Xu]

Even if <linux/kvm.h> seems to exist for all archs on linux, however including
it with __linux__ defined seems to be not working yet as it'll try to include
asm/kvm.h and that can be missing for archs that do not support kvm.

To fix this (instead of any attempt to fix linux headers..), we can mark the
header to be x86_64 only, because it's so far only service for adding the kvm
dirty ring test.

No need to have "Fixes" as the issue is just introduced very recently.

Reported-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tests/qtest/migration-test.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 1e8b7784ef..cc5e83d98a 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -27,7 +27,8 @@
 #include "migration-helpers.h"
 #include "tests/migration/migration-test.h"
 
-#if defined(__linux__)
+/* For dirty ring test; so far only x86_64 is supported */
+#if defined(__linux__) && defined(HOST_X86_64)
 #include "linux/kvm.h"
 #endif
 
@@ -1395,7 +1396,7 @@ static void test_multifd_tcp_cancel(void)
 
 static bool kvm_dirty_ring_supported(void)
 {
-#if defined(__linux__)
+#if defined(__linux__) && defined(HOST_X86_64)
     int ret, kvm_fd = open("/dev/kvm", O_RDONLY);
 
     if (kvm_fd < 0) {
-- 
2.31.1

Re: [PATCH] tests: Fix migration-test build failure for sparc

[Peter Xu]

On Wed, Jul 28, 2021 at 05:41:28PM -0400, Peter Xu wrote:
> No need to have "Fixes" as the issue is just introduced very recently.

And.. This is only true if this patch can be merged in 6.1...

I should have added "for 6.1" in the subject but I forgot.  Sorry.

-- 
Peter Xu

Re: [PATCH] tests: Fix migration-test build failure for sparc

[Richard Henderson]

On 7/28/21 11:41 AM, Peter Xu wrote:
> Even if <linux/kvm.h> seems to exist for all archs on linux, however including
> it with __linux__ defined seems to be not working yet as it'll try to include
> asm/kvm.h and that can be missing for archs that do not support kvm.
> 
> To fix this (instead of any attempt to fix linux headers..), we can mark the
> header to be x86_64 only, because it's so far only service for adding the kvm
> dirty ring test.
> 
> No need to have "Fixes" as the issue is just introduced very recently.

What an odd thing to say.  How do I know that without the link?
Fixes: 1f546b709d61

Anyway,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

> 
> Reported-by: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   tests/qtest/migration-test.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 1e8b7784ef..cc5e83d98a 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -27,7 +27,8 @@
>   #include "migration-helpers.h"
>   #include "tests/migration/migration-test.h"
>   
> -#if defined(__linux__)
> +/* For dirty ring test; so far only x86_64 is supported */
> +#if defined(__linux__) && defined(HOST_X86_64)
>   #include "linux/kvm.h"
>   #endif
>   
> @@ -1395,7 +1396,7 @@ static void test_multifd_tcp_cancel(void)
>   
>   static bool kvm_dirty_ring_supported(void)
>   {
> -#if defined(__linux__)
> +#if defined(__linux__) && defined(HOST_X86_64)
>       int ret, kvm_fd = open("/dev/kvm", O_RDONLY);
>   
>       if (kvm_fd < 0) {
> 

Re: [PATCH] tests: Fix migration-test build failure for sparc

[Philippe Mathieu-Daudé]

On 7/28/21 11:41 PM, Peter Xu wrote:
> Even if <linux/kvm.h> seems to exist for all archs on linux, however including
> it with __linux__ defined seems to be not working yet as it'll try to include
> asm/kvm.h and that can be missing for archs that do not support kvm.
> 
> To fix this (instead of any attempt to fix linux headers..), we can mark the
> header to be x86_64 only, because it's so far only service for adding the kvm
> dirty ring test.
> 
> No need to have "Fixes" as the issue is just introduced very recently.

Personally I find it very useful to navigate in gitk without having
to use git-blame.

Fixes: 1f546b709d6 ("tests: migration-test: Add dirty ring test")
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> 
> Reported-by: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  tests/qtest/migration-test.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 1e8b7784ef..cc5e83d98a 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -27,7 +27,8 @@
>  #include "migration-helpers.h"
>  #include "tests/migration/migration-test.h"
>  
> -#if defined(__linux__)
> +/* For dirty ring test; so far only x86_64 is supported */
> +#if defined(__linux__) && defined(HOST_X86_64)
>  #include "linux/kvm.h"
>  #endif
>  
> @@ -1395,7 +1396,7 @@ static void test_multifd_tcp_cancel(void)
>  
>  static bool kvm_dirty_ring_supported(void)
>  {
> -#if defined(__linux__)
> +#if defined(__linux__) && defined(HOST_X86_64)
>      int ret, kvm_fd = open("/dev/kvm", O_RDONLY);
>  
>      if (kvm_fd < 0) {
> 

Re: [PATCH] tests: Fix migration-test build failure for sparc

[Peter Xu]

On Thu, Jul 29, 2021 at 01:48:57AM +0200, Philippe Mathieu-Daudé wrote:
> On 7/28/21 11:41 PM, Peter Xu wrote:
> > Even if <linux/kvm.h> seems to exist for all archs on linux, however including
> > it with __linux__ defined seems to be not working yet as it'll try to include
> > asm/kvm.h and that can be missing for archs that do not support kvm.
> > 
> > To fix this (instead of any attempt to fix linux headers..), we can mark the
> > header to be x86_64 only, because it's so far only service for adding the kvm
> > dirty ring test.
> > 
> > No need to have "Fixes" as the issue is just introduced very recently.
> 
> Personally I find it very useful to navigate in gitk without having
> to use git-blame.

Makes sense.

> 
> Fixes: 1f546b709d6 ("tests: migration-test: Add dirty ring test")
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks all!

-- 
Peter Xu

Re: [PATCH] tests: Fix migration-test build failure for sparc

[Thomas Huth]

On 28/07/2021 23.41, Peter Xu wrote:
> Even if <linux/kvm.h> seems to exist for all archs on linux, however including
> it with __linux__ defined seems to be not working yet as it'll try to include
> asm/kvm.h and that can be missing for archs that do not support kvm.
> 
> To fix this (instead of any attempt to fix linux headers..), we can mark the
> header to be x86_64 only, because it's so far only service for adding the kvm
> dirty ring test.
> 
> No need to have "Fixes" as the issue is just introduced very recently.
> 
> Reported-by: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   tests/qtest/migration-test.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 1e8b7784ef..cc5e83d98a 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -27,7 +27,8 @@
>   #include "migration-helpers.h"
>   #include "tests/migration/migration-test.h"
>   
> -#if defined(__linux__)
> +/* For dirty ring test; so far only x86_64 is supported */
> +#if defined(__linux__) && defined(HOST_X86_64)
>   #include "linux/kvm.h"
>   #endif
>   
> @@ -1395,7 +1396,7 @@ static void test_multifd_tcp_cancel(void)
>   
>   static bool kvm_dirty_ring_supported(void)
>   {
> -#if defined(__linux__)
> +#if defined(__linux__) && defined(HOST_X86_64)
>       int ret, kvm_fd = open("/dev/kvm", O_RDONLY);
>   
>       if (kvm_fd < 0) {
> 

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

Juan, Dave, if you don't mind I can take this through my testing branch - 
I'm planning to send a pull request today anyway.

  Thomas

Re: [PATCH] tests: Fix migration-test build failure for sparc

[Dr. David Alan Gilbert]

* Thomas Huth (thuth@redhat.com) wrote:
> On 28/07/2021 23.41, Peter Xu wrote:
> > Even if <linux/kvm.h> seems to exist for all archs on linux, however including
> > it with __linux__ defined seems to be not working yet as it'll try to include
> > asm/kvm.h and that can be missing for archs that do not support kvm.
> > 
> > To fix this (instead of any attempt to fix linux headers..), we can mark the
> > header to be x86_64 only, because it's so far only service for adding the kvm
> > dirty ring test.
> > 
> > No need to have "Fixes" as the issue is just introduced very recently.
> > 
> > Reported-by: Richard Henderson <rth@twiddle.net>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >   tests/qtest/migration-test.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index 1e8b7784ef..cc5e83d98a 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -27,7 +27,8 @@
> >   #include "migration-helpers.h"
> >   #include "tests/migration/migration-test.h"
> > -#if defined(__linux__)
> > +/* For dirty ring test; so far only x86_64 is supported */
> > +#if defined(__linux__) && defined(HOST_X86_64)
> >   #include "linux/kvm.h"
> >   #endif
> > @@ -1395,7 +1396,7 @@ static void test_multifd_tcp_cancel(void)
> >   static bool kvm_dirty_ring_supported(void)
> >   {
> > -#if defined(__linux__)
> > +#if defined(__linux__) && defined(HOST_X86_64)
> >       int ret, kvm_fd = open("/dev/kvm", O_RDONLY);
> >       if (kvm_fd < 0) {
> > 
> 
> Acked-by: Thomas Huth <thuth@redhat.com>
> 
> Juan, Dave, if you don't mind I can take this through my testing branch -
> I'm planning to send a pull request today anyway.

Please do; thanks!

Dave

>  Thomas
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK