Re: [PATCH v4 00/54] tests/qtest: Enable running qtest on Windows

2022-10-03 Thread Bin Meng
Hi Marc-André,

On Mon, Oct 3, 2022 at 5:26 PM Marc-André Lureau
 wrote:
>
> Hi Bin
>
> On Tue, Sep 27, 2022 at 3:18 PM Bin Meng  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 v4:
>> - Do not use g_autofree and g_steal_pointer
>> - Update the error reporting by using the GError "error" argument
>>   of g_dir_make_tmp()
>> - Remove the const from tmpfs declaration
>> - Replace the whole block with a g_assert_no_error()
>> - Replace the error reporting with g_assert_no_error()
>> - Update error reporting
>> - Move the new text section after the "QTest" section instead
>> - Use plural in both cases: "on POSIX hosts as well as Windows hosts"
>> - Use "The following list shows some best practices"
>> - Fix typo of delimiter
>> - New patch: "tests/qtest: boot-serial-test: Close the serial file before 
>> starting QEMU"
>> - Drop patch: "chardev/char-file: Add FILE_SHARE_WRITE when openning the 
>> file for win32"
>>
>
> Could you post a v5 rebased on the current master? thanks
>

Sure, will do.

> (I think most of the remaining patches are simple enough that I could take 
> them in a misc PR if they are not picked by subsystem maintainers)

Thank you.

Regards,
Bin



Re: [PATCH v4 00/54] tests/qtest: Enable running qtest on Windows

2022-10-03 Thread Marc-André Lureau
Hi Bin

On Tue, Sep 27, 2022 at 3:18 PM Bin Meng  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 v4:
> - Do not use g_autofree and g_steal_pointer
> - Update the error reporting by using the GError "error" argument
>   of g_dir_make_tmp()
> - Remove the const from tmpfs declaration
> - Replace the whole block with a g_assert_no_error()
> - Replace the error reporting with g_assert_no_error()
> - Update error reporting
> - Move the new text section after the "QTest" section instead
> - Use plural in both cases: "on POSIX hosts as well as Windows hosts"
> - Use "The following list shows some best practices"
> - Fix typo of delimiter
> - New patch: "tests/qtest: boot-serial-test: Close the serial file before
> starting QEMU"
> - Drop patch: "chardev/char-file: Add FILE_SHARE_WRITE when openning the
> file for win32"
>
>
Could you post a v5 rebased on the current master? thanks

(I think most of the remaining patches are simple enough that I could take
them in a misc PR if they are not picked by subsystem maintainers)

Changes in v3:
> - Remove unnecessary "error = NULL" statements
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Split to a separate patch
> - Split to a separate patch
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Use g_steal_pointer() in create_test_img()
> - Split to a separate patch
> - Split to a separate patch
> - Split to a separate patch
> - Split to a separate patch
> - Split to a separate patch
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Split to a separate patch
> - Split to a separate patch
> - Add a usleep(1) in the busy wait loop
> - Drop the host test
> - Drop patch: "tests: Change to use g_mkdir()"
> - Drop patch: "block: Unify the get_tmp_filename() implementation",
>   and send it as a separate patch
>
> Changes in v2:
> - new patch: "tests/qtest: i440fx-test: Rewrite create_blob_file() to be
> portable"
> - Use g_autofree to declare the variable
> - Change to use g_mkdir()
> - Change to use g_mkdir()
> - Change to use g_mkdir()
> - Change to skip only part of the virtio-net-test cases that require
>   socketpair() intead of disabling all of them
> - Introduce a new variable qtests_filter and add that to the
>   qtests_ARCH variables
> - Add a comment in the code to explain why test_qmp_oob test case
>   is skipped on win32
> - Replace signal by the semaphore on posix too
> - Use __declspec(selectany) for the common weak symbol on Windows
> - 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
> - Drop ahci-test.c changes that are no longer needed
> - Change the place that sets IO redirection in the command line
> - Change to a busy wait after migration is canceled
> - new patch: "io/channel-watch: Drop the unnecessary cast"
> - Change the timeout limit to 90 minutes
> - new patch: Display meson test logs in the Windows CI
> - new patch: "tests/qtest: Enable qtest build on Windows"
> - Minor wording changes
> - Drop patches that were already applied in the mainline
> - Drop patch: "qga/commands-posix-ssh: Use g_mkdir_with_parents()"
> - Drop patch: "tests: Skip iotests and qtest when
> '--without-default-devices'"
> - Drop patch: "tests/qtest: Fix ERROR_SHARING_VIOLATION for win32"
>
> Bin Meng (48):
>   tests/qtest: i440fx-test: Rewrite create_blob_file() to be portable
>   semihosting/arm-compat-semi: Avoid using hardcoded /tmp
>   tcg: Avoid using hardcoded /tmp
>   util/qemu-sockets: Use g_get_tmp_dir() to get the directory for
> temporary files
>   tests/qtest: ahci-test: Avoid using hardcoded /tmp
>   tests/qtest: aspeed_smc-test: Avoid using hardcoded /tmp
>   tests/qtest: boot-serial-test: Avoid using hardcoded /tmp
>   tests/qtest: cxl-test: Avoid using hardcoded /tmp
>   tests/qtest: fdc-test: Avoid using hardcoded /tmp
>   tests/qtest: generic_fuzz: Avoid using hardcoded /tmp
>   tests/qtest: virtio_blk_fuzz: Avoid using hardcoded /tmp
>   tests/qtest: hd-geo-test: Avoid using hardcoded /tmp
>   tests/qtest: ide-test: Avoid using hardcoded /tmp
>   tests/qtest: migration-test: Avoid using hardcoded /tmp
>   tests/qtest: 

Re: [PATCH v4 00/54] tests/qtest: Enable running qtest on Windows

2022-09-28 Thread Daniel P . Berrangé
On Wed, Sep 28, 2022 at 05:24:56PM +0200, Thomas Huth wrote:
> On 28/09/2022 12.31, Thomas Huth wrote:
> > On 27/09/2022 13.05, Bin Meng 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.
> > 
> > Thanks for your patches - I've picked many of them for my pull request
> > that I sent out earlier today, so you don't have to carry them along
> > anymore once the PR got merged.
> > 
> > For the patches that are not directly related to tests/ ... could you
> > maybe ask the corresponding maintainers to pick those up? I'm not sure
> > whether they should go through my testing branch, too...
> > 
> > Anyway, there seems to be one more issue: The migration test sometimes
> > seems to be failing on aarch64 with all your patches applied:
> > 
> > 87/470 qemu:qtest+qtest-aarch64 / qtest-aarch64/migration-test
> > ERROR  261.71s   killed by signal 6 SIGABRT
> >  >>> MALLOC_PERTURB_=171 QTEST_QEMU_BINARY=./qemu-system-aarch64
> > QTEST_QEMU_IMG=./qemu-img
> > G_TEST_DBUS_DAEMON=/home/thuth/devel/qemu/tests/dbus-vmstate-daemon.sh
> > QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon
> > /home/thuth/tmp/qemu-build/tests/qtest/migration-test --tap -k
> > ―――
> > stderr:
> > **
> > ERROR:../../devel/qemu/tests/qtest/migration-helpers.c:205:wait_for_migration_status:
> > assertion failed: (g_test_timer_elapsed() <
> > MIGRATION_STATUS_WAIT_TIMEOUT)
> > ../../devel/qemu/tests/qtest/libqtest.c:201: kill_qemu() tried to
> > terminate QEMU process but encountered exit status 1 (expected 0)
> > 
> > (test program exited with status code -6)
> > ―
> > 
> > Not sure whether it's really related to your patches or whether it's
> > something else that has been merged recently, I'm having problems to
> > reproduce it reliably, but it's definitely something we should keep an
> > eye on...
> 
> Seems like somebody also ran into this issue with a vanilla QEMU:
> 
>  https://gitlab.com/qemu-project/qemu/-/issues/1230
> 
> So it's not related to your patch series.

This status timeout was something we merged in last cycle. We've
long had wierd hangs in the migration tests and this timeout change
was an attempt to turn the test suite hangs into explicit failures
for greater visibility. I guess this is working as intended, but
we're not really closer to understanding what the root problem is
we're hitting.

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




Re: [PATCH v4 00/54] tests/qtest: Enable running qtest on Windows

2022-09-28 Thread Thomas Huth

On 28/09/2022 12.31, Thomas Huth wrote:

On 27/09/2022 13.05, Bin Meng 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.


Thanks for your patches - I've picked many of them for my pull request that 
I sent out earlier today, so you don't have to carry them along anymore once 
the PR got merged.


For the patches that are not directly related to tests/ ... could you maybe 
ask the corresponding maintainers to pick those up? I'm not sure whether 
they should go through my testing branch, too...


Anyway, there seems to be one more issue: The migration test sometimes seems 
to be failing on aarch64 with all your patches applied:


87/470 qemu:qtest+qtest-aarch64 / qtest-aarch64/migration-test 
ERROR  261.71s   killed by signal 6 SIGABRT
 >>> MALLOC_PERTURB_=171 QTEST_QEMU_BINARY=./qemu-system-aarch64 
QTEST_QEMU_IMG=./qemu-img 
G_TEST_DBUS_DAEMON=/home/thuth/devel/qemu/tests/dbus-vmstate-daemon.sh 
QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon 
/home/thuth/tmp/qemu-build/tests/qtest/migration-test --tap -k

―――
stderr:
**
ERROR:../../devel/qemu/tests/qtest/migration-helpers.c:205:wait_for_migration_status: 
assertion failed: (g_test_timer_elapsed() < MIGRATION_STATUS_WAIT_TIMEOUT)
../../devel/qemu/tests/qtest/libqtest.c:201: kill_qemu() tried to terminate 
QEMU process but encountered exit status 1 (expected 0)


(test program exited with status code -6)
―

Not sure whether it's really related to your patches or whether it's 
something else that has been merged recently, I'm having problems to 
reproduce it reliably, but it's definitely something we should keep an eye 
on...


Seems like somebody also ran into this issue with a vanilla QEMU:

 https://gitlab.com/qemu-project/qemu/-/issues/1230

So it's not related to your patch series.

 Thomas





Re: [PATCH v4 00/54] tests/qtest: Enable running qtest on Windows

2022-09-28 Thread Thomas Huth

On 27/09/2022 13.05, Bin Meng 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.


Thanks for your patches - I've picked many of them for my pull request that 
I sent out earlier today, so you don't have to carry them along anymore once 
the PR got merged.


For the patches that are not directly related to tests/ ... could you maybe 
ask the corresponding maintainers to pick those up? I'm not sure whether 
they should go through my testing branch, too...


Anyway, there seems to be one more issue: The migration test sometimes seems 
to be failing on aarch64 with all your patches applied:


87/470 qemu:qtest+qtest-aarch64 / qtest-aarch64/migration-test 
ERROR  261.71s   killed by signal 6 SIGABRT
>>> MALLOC_PERTURB_=171 QTEST_QEMU_BINARY=./qemu-system-aarch64 
QTEST_QEMU_IMG=./qemu-img 
G_TEST_DBUS_DAEMON=/home/thuth/devel/qemu/tests/dbus-vmstate-daemon.sh 
QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon 
/home/thuth/tmp/qemu-build/tests/qtest/migration-test --tap -k

―――
stderr:
**
ERROR:../../devel/qemu/tests/qtest/migration-helpers.c:205:wait_for_migration_status: 
assertion failed: (g_test_timer_elapsed() < MIGRATION_STATUS_WAIT_TIMEOUT)
../../devel/qemu/tests/qtest/libqtest.c:201: kill_qemu() tried to terminate 
QEMU process but encountered exit status 1 (expected 0)


(test program exited with status code -6)
―

Not sure whether it's really related to your patches or whether it's 
something else that has been merged recently, I'm having problems to 
reproduce it reliably, but it's definitely something we should keep an eye on...


 Thomas