All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: Richard Henderson <rth@twiddle.net>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Juan Quintela <quintela@redhat.com>,
	qemu-devel@nongnu.org,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	Leonardo Bras Soares Passos <lsoaresp@redhat.com>
Subject: Re: [PATCH v2 2/2] tests: migration-test: Add dirty ring test
Date: Wed, 28 Jul 2021 16:37:59 -0400	[thread overview]
Message-ID: <YQHAJy8lX0vt+yl2@t490s> (raw)
In-Reply-To: <9b93e149-3147-3dca-83ce-474af99ccd2e@twiddle.net>

[-- Attachment #1: Type: text/plain, Size: 3101 bytes --]

On Wed, Jul 28, 2021 at 09:37:48AM -1000, Richard Henderson wrote:
> On 6/15/21 7:55 AM, Peter Xu wrote:
> > Add dirty ring test if kernel supports it.  Add the dirty ring parameter on
> > source should be mostly enough, but let's change the dest too to make them
> > match always.
> > 
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >   tests/qtest/migration-test.c | 58 ++++++++++++++++++++++++++++++++++--
> >   1 file changed, 55 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index d9225f58d4d..9ef6b471353 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -27,6 +27,10 @@
> >   #include "migration-helpers.h"
> >   #include "tests/migration/migration-test.h"
> > +#if defined(__linux__)
> > +#include "linux/kvm.h"
> > +#endif
> 
> This breaks the build for hosts that do not support kvm, e.g. sparc:
> 
> 
> [2/3] Compiling C object tests/qtest/migration-test.p/migration-test.c.o
> FAILED: tests/qtest/migration-test.p/migration-test.c.o
> cc -Itests/qtest/migration-test.p -Itests/qtest -I../qemu/tests/qtest -I.
> -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/glib-2.0
> -I/usr/lib/sparc64-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -pipe
> -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem
> /home/rth/qemu/qemu/linux-headers -isystem linux-headers -iquote . -iquote
> /home/rth/qemu/qemu -iquote /home/rth/qemu/qemu/include -iquote
> /home/rth/qemu/qemu/disas/libvixl -iquote /home/rth/qemu/qemu/tcg/sparc
> -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -m64 -mcpu=ultrasparc
> -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes
> -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes
> -fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration
> -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k
> -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs
> -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2
> -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fPIE -MD -MQ
> tests/qtest/migration-test.p/migration-test.c.o -MF
> tests/qtest/migration-test.p/migration-test.c.o.d -o
> tests/qtest/migration-test.p/migration-test.c.o -c
> ../qemu/tests/qtest/migration-test.c
> In file included from ../qemu/tests/qtest/migration-test.c:31:
> /home/rth/qemu/qemu/linux-headers/linux/kvm.h:15:10: fatal error: asm/kvm.h:
> No such file or directory
>    15 | #include <asm/kvm.h>
>       |          ^~~~~~~~~~~
> compilation terminated.

Hi, Richard,

Sorry for that.  It's very weird that linux/kvm.h exists for all archs while
it's not conditionally including asm/kvm.h only for the 5 supported archs, so
any user app trying to include linux/kvm.h will fail for the rest.

(while all references needed in this test is actually KVM_CHECK_EXTENSION,
 KVM_CAP_DIRTY_LOG_RING and both of them exist in linux/kvm.h not the asm one)

A quick fix attached; would that work for us?

Thanks,

-- 
Peter Xu

[-- Attachment #2: 0001-tests-Fix-migration-test-build-failure-for-sparc.patch --]
[-- Type: text/plain, Size: 1514 bytes --]

From 888ab46c44284738d222edc87e9fc86a49ae2f51 Mon Sep 17 00:00:00 2001
From: Peter Xu <peterx@redhat.com>
Date: Wed, 28 Jul 2021 16:32:00 -0400
Subject: [PATCH] tests: Fix migration-test build failure for sparc

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.

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


  reply	other threads:[~2021-07-28 20:39 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-15 17:55 [PATCH v2 0/2] tests: migration-test: Fix agressive test skip, add dirty ring test Peter Xu
2021-06-15 17:55 ` [PATCH v2 1/2] tests: migration-test: Still run the rest even if uffd missing Peter Xu
2021-06-16 12:49   ` Juan Quintela
2021-06-15 17:55 ` [PATCH v2 2/2] tests: migration-test: Add dirty ring test Peter Xu
2021-06-16 12:55   ` Juan Quintela
2021-06-16 13:29     ` Peter Xu
2021-07-28 19:37   ` Richard Henderson
2021-07-28 20:37     ` Peter Xu [this message]
2021-07-28 21:11       ` Richard Henderson
2021-07-28 21:39         ` Peter Xu
2021-06-30 17:01 ` [PATCH v2 0/2] tests: migration-test: Fix agressive test skip, add " Dr. David Alan Gilbert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YQHAJy8lX0vt+yl2@t490s \
    --to=peterx@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=lsoaresp@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.