Re: [PATCH v6 00/11] tests/qtest: Enable running qtest on Windows

[Thomas Huth <thuth@redhat.com>]

On 28/10/2022 11.43, Bin Meng wrote:
> On Fri, Oct 28, 2022 at 5:41 PM Marc-André Lureau
> <marcandre.lureau@redhat.com> wrote:
>>
>> Hi
>>
>> On Fri, Oct 28, 2022 at 1:21 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>>>
>>> On Fri, Oct 28, 2022 at 4:09 PM Marc-André Lureau
>>> <marcandre.lureau@redhat.com> wrote:
>>>>
>>>> Hi
>>>>
>>>> On Fri, Oct 28, 2022 at 8:58 AM Bin Meng <bin.meng@windriver.com> wrote:
>>>>>
>>>>> In preparation to adding virtio-9p support on Windows, this series
>>>>> enables running qtest on Windows, so that we can run the virtio-9p
>>>>> tests on Windows to make sure it does not break accidently.
>>>>>
>>>>> Changes in v6:
>>>>> - drop patches that are already in Alex and Daniel's tree
>>>>> - remove CONFIG_POSIX from meson.build
>>>>> - include <qemu/sockets.h> in libqtest.c
>>>>> - move documentation comments of qemu_send_full() from util/osdep.c
>>>>>    to qemu/sockets.h
>>>>> - save the "exit_code" in struct QTestState
>>>>> - new patch: "tests/qtest: device-plug-test: Reverse the usage of double/single quotes"
>>>>> - new patch: "tests/qtest: Use EXIT_FAILURE instead of magic number"
>>>>> - new patch: "tests/qtest: libqtest: Introduce qtest_wait_qemu()"
>>>>> - change to use qtest_wait_qemu() API
>>>>> - new patch: "test/qtest/libqos: meson.build: Do not build virtio-9p unconditionally"
>>>>>
>>>>> Changes in v5:
>>>>> - restore to v1 version which does not touch the posix implementation
>>>>> - Drop patches that are already merged
>>>>>
>>>>> Changes in v3:
>>>>> - Add a usleep(1) in the busy wait loop
>>>>> - Drop the host test
>>>>>
>>>>> Changes in v2:
>>>>> - Introduce qemu_send_full() and use it
>>>>> - Move the enabling of building qtests on Windows to a separate
>>>>>    patch to keep bisectablity
>>>>> - Call socket_init() unconditionally
>>>>> - Add a missing CloseHandle() call
>>>>> - Change to a busy wait after migration is canceled
>>>>> - Change the timeout limit to 90 minutes
>>>>> - new patch: "tests/qtest: Enable qtest build on Windows"
>>>>>
>>>>> Bin Meng (8):
>>>>>    tests/qtest: Support libqtest to build and run on Windows
>>>>>    tests/qtest: device-plug-test: Reverse the usage of double/single
>>>>>      quotes
>>>>>    tests/qtest: Use EXIT_FAILURE instead of magic number
>>>>>    tests/qtest: libqtest: Introduce qtest_wait_qemu()
>>>>>    tests/qtest: libqos: Do not build virtio-9p unconditionally
>>>>>    tests/qtest: libqtest: Correct the timeout unit of blocking receive
>>>>>      calls for win32
>>>>>    .gitlab-ci.d/windows.yml: Increase the timeout to 90 minutes
>>>>>    tests/qtest: Enable qtest build on Windows
>>>>>
>>>>> Xuzhou Cheng (3):
>>>>>    accel/qtest: Support qtest accelerator for Windows
>>>>>    tests/qtest: Use send/recv for socket communication
>>>>>    tests/qtest: migration-test: Make sure QEMU process "to" exited after
>>>>>      migration is canceled
>>>>>
>>>>>   include/hw/core/cpu.h           |   1 +
>>>>>   include/qemu/sockets.h          |  13 +++
>>>>>   tests/qtest/libqtest.h          |   9 ++
>>>>>   accel/dummy-cpus.c              |  14 ++-
>>>>>   softmmu/cpus.c                  |   9 +-
>>>>>   tests/qtest/dbus-vmstate-test.c |   2 +-
>>>>>   tests/qtest/device-plug-test.c  |  16 ++--
>>>>>   tests/qtest/libqmp.c            |   5 +-
>>>>>   tests/qtest/libqtest.c          | 151 ++++++++++++++++++++++++++++----
>>>>>   tests/qtest/migration-test.c    |   8 +-
>>>>>   util/osdep.c                    |  22 +++++
>>>>>   .gitlab-ci.d/windows.yml        |   4 +-
>>>>>   accel/meson.build               |   2 +-
>>>>>   accel/qtest/meson.build         |   3 +-
>>>>>   tests/qtest/libqos/meson.build  |   6 +-
>>>>>   tests/qtest/meson.build         |   6 --
>>>>>   16 files changed, 221 insertions(+), 50 deletions(-)
>>>>>
>>>>> --
>>>>> 2.25.1
>>>>>
>>>>
>>>> Series looks good to me:
>>>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>>
>>>>
>>>> However, many qtests are flaky on Windows. I get a number of "broken pipe" exit code 3 & timeout. Should gitlab ignore windows test failures ? Or perhaps have a new "ignored" job for the windows qtests. What's your experience running gitlab CI with this series? Can you share results? (I kicked off one here https://gitlab.com/marcandre.lureau/qemu/-/pipelines/679511572)
>>>>
>>>
>>> This "broken pipe" error was fixed by [1] which is currently in
>>> Daniel's tree. Please apply it in your tree and it should have a 100%
>>> pass rate.
>>>
>>> [1] http://patchwork.ozlabs.org/project/qemu-devel/patch/20221006151927.2079583-17-bmeng.cn@gmail.com/
>>>
>>
>> Ok I have seen other tests randomly failing. Furthermore:
>> https://gitlab.com/marcandre.lureau/qemu/-/jobs/3241465230
>> ERROR: Job failed: execution took longer than 1h30m0s seconds
>>
>>
>> I think we should drop the last 2 patches for now, until CI testing is under control...
> 
> 2 hours is the maximum time supported by the gitlab shared runners
> which should be enough.
> 
> However people may feel that it takes too long ...

Most jobs run fine within the default timeout of 60 minutes, we've got some 
few exceptions where we bumped it to 70, 80 or even 90 minutes. But I think 
that should really be the uppermost limit - using even more than 90 minutes 
for the slow-running Windows job just feels wrong.

Aren't there other possibilities to cut the run time again? Maybe compile 
with --disable-tools? ... searching for "mingw compilation slow" in the 
internet also reveals that a lot of other people are seeing problems with 
the compilation speed of MinGW ... but there does not seem to be one 
ultimate solution that worked for everybody to speed it up...

I'll queue patches 1-9 for now so that we get the important parts in before 
the soft freeze starts ... if we finally figure out how to deal with the 
slow CI jobs best, I think it's also still OK if we enable it later before 
the hard freeze starts.

  Thomas