[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] tests/qtest/migration: Restore include for postcopy
From: |
Fabiano Rosas |
Subject: |
Re: [PATCH] tests/qtest/migration: Restore include for postcopy |
Date: |
Wed, 18 Dec 2024 12:12:11 -0300 |
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Tue, Dec 17, 2024 at 04:47:39PM -0500, Peter Xu wrote:
>> On Tue, Dec 17, 2024 at 06:22:01PM -0300, Fabiano Rosas wrote:
>> > Commit 124a3c58b8 ("tests/qtest/migration: Move ufd_version_check to
>> > utils") moved the ufd_version_check() function to another file but
>> > failed to bring along the <sys/syscall> include, which is necessary to
>> > pull in <asm/unistd.h> for __NR_userfaultd.
>> >
>> > Restore the missing include.
>>
>> Ohhhhhhh.. so postcopy tests will always be skipped as of now? Maybe worth
>> explicit mention that in the commit message if so, only when you merge.
>>
>> >
>> > While here, remove the ifdef __linux__ that's redundant and fix a
>> > couple of typos.
>> >
>> > Fixes: 124a3c58b8 ("tests/qtest/migration: Move ufd_version_check to
>> > utils")
>> > Signed-off-by: Fabiano Rosas <farosas@suse.de>
>>
>> Reviewed-by: Peter Xu <peterx@redhat.com>
>>
>> Maybe we don't need to be as careful on old kernels anymore especially in
>> tests, because userfaultfd syscall existed for ~10 years. So if we want we
>> can start requiring __NR_userfaultfd present for __linux__, then no way to
>> miss such spot next time.
>
> Yes, I think that check is obsolete, based on our supported platforms
> list. It would suffice to just check __linux__.
This breaks the cross builds. It seems the __NR_userfaultfd was actually
stopping several archs from reaching ufd_version_check(). Since
<sys/ioctl.h> is under HOST_X86_64, these new instances now fail to find
the 'ioctl' symbol:
https://gitlab.com/farosas/qemu/-/pipelines/1594332399
Of course I could just include <sys/ioctl.h> unconditionally, but the
fact that new code is not being built means the assumption that we can
imply __NR_userfaultfd from __linux__ alone is not correct.