[PATCH 0/3] tests/qtest/fdc-test: give each test case its own instance

Christian Quante posted 3 patches 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260822104329.42121-1-christian@quante.one
Maintainers: John Snow <jsnow@redhat.com>, Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
tests/qtest/fdc-test.c | 127 ++++++++++++++++++++++++++++++++++-------
1 file changed, 107 insertions(+), 20 deletions(-)
[PATCH 0/3] tests/qtest/fdc-test: give each test case its own instance
Posted by Christian Quante 1 month ago
The test cases in fdc-test share one QEMU instance and, with it, the
state of the floppy controller.  Several of them only pass in the order
they happen to be registered in: read_no_dma_1, _18 and _19 fail when
run on their own, and fuzz-registers leaves an unacknowledged interrupt
behind, so it works only where it sits today.

This takes up Kevin Wolf's invitation from the review of d00567f7fb and
431f59a34d: "If you feel like addressing the wider ordering problem in
the tests, too, you're welcome to do so."

Patch 1 registers the test cases from a table and runs each one in a
QEMU instance of its own.  No test function changes what it tests; the
three read_no_dma cases insert their own medium instead of inheriting
one, and test_read_id stops ejecting at the end.

Patch 2 skips the test cases when the pc machine is not part of the
build, after 4e37f749dd, 6500a6abe6 and bb98963d6d did the same for
drive_del-test, device-plug-test and pxe-test.  Patch 1 routes every
test case through a single function, so one check covers all of them.

Patch 3 adds a test case that keeps d00567f7fb ("hw/block/fdc: select
the drive named by the READ ID command") from being lost again -- with
one drive, answering for the wrong one cannot be told from answering
for the right one.  It needs a machine with two drives, which is what
the table's args field is for.

Measured on this series:

  * every test case passes on its own; on master three of them fail
  * the suite passes with the registration order reversed; on master it
    aborts in send_seek()
  * removing SET_CUR_DRV() from fdctrl_handle_readid() again turns
    exactly one test case red -- the new one -- and leaves the other
    sixteen green
  * each of the three commits builds and passes on its own

The price is wall-clock time: the fifteen test cases in the table now
start an instance each instead of sharing one, which takes the suite
from 0.78s to 2.8s.  The qtest timeout is 60s.

The pattern is not new here.  Of the 195 files in tests/qtest, 57
register their test cases with qtest_add_data_func() and none uses glib
fixtures; boot-serial-test.c is the closest model, with a table of test
definitions and a shared runner that starts one instance per entry.  72
files still include libqtest-single.h, which is why the test functions
keep reaching their instance through global_qtest instead of carrying a
QTestState around -- that would have touched every one of them.

Christian Quante (3):
  tests/qtest/fdc-test: give each test case its own QEMU instance
  tests/qtest/fdc-test: skip test cases when the 'pc' machine is missing
  tests/qtest/fdc-test: check that READ ID answers for the drive it
    names

 tests/qtest/fdc-test.c | 127 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 107 insertions(+), 20 deletions(-)


base-commit: eea8fe61b8be8f3016e522e6af24924a0266ca95
-- 
2.53.0
Re: [PATCH 0/3] tests/qtest/fdc-test: give each test case its own instance
Posted by Marc-André Lureau 1 month ago
Hi

On Sat, Aug 22, 2026 at 2:45 PM Christian Quante <christian@quante.one> wrote:
>
> The test cases in fdc-test share one QEMU instance and, with it, the
> state of the floppy controller.  Several of them only pass in the order
> they happen to be registered in: read_no_dma_1, _18 and _19 fail when
> run on their own, and fuzz-registers leaves an unacknowledged interrupt
> behind, so it works only where it sits today.
>
> This takes up Kevin Wolf's invitation from the review of d00567f7fb and
> 431f59a34d: "If you feel like addressing the wider ordering problem in
> the tests, too, you're welcome to do so."
>
> Patch 1 registers the test cases from a table and runs each one in a
> QEMU instance of its own.  No test function changes what it tests; the
> three read_no_dma cases insert their own medium instead of inheriting
> one, and test_read_id stops ejecting at the end.
>
> Patch 2 skips the test cases when the pc machine is not part of the
> build, after 4e37f749dd, 6500a6abe6 and bb98963d6d did the same for
> drive_del-test, device-plug-test and pxe-test.  Patch 1 routes every
> test case through a single function, so one check covers all of them.
>
> Patch 3 adds a test case that keeps d00567f7fb ("hw/block/fdc: select
> the drive named by the READ ID command") from being lost again -- with
> one drive, answering for the wrong one cannot be told from answering
> for the right one.  It needs a machine with two drives, which is what
> the table's args field is for.
>
> Measured on this series:
>
>   * every test case passes on its own; on master three of them fail
>   * the suite passes with the registration order reversed; on master it
>     aborts in send_seek()
>   * removing SET_CUR_DRV() from fdctrl_handle_readid() again turns
>     exactly one test case red -- the new one -- and leaves the other
>     sixteen green
>   * each of the three commits builds and passes on its own
>
> The price is wall-clock time: the fifteen test cases in the table now
> start an instance each instead of sharing one, which takes the suite
> from 0.78s to 2.8s.  The qtest timeout is 60s.
>
> The pattern is not new here.  Of the 195 files in tests/qtest, 57
> register their test cases with qtest_add_data_func() and none uses glib
> fixtures; boot-serial-test.c is the closest model, with a table of test
> definitions and a shared runner that starts one instance per entry.  72
> files still include libqtest-single.h, which is why the test functions
> keep reaching their instance through global_qtest instead of carrying a
> QTestState around -- that would have touched every one of them.
>
> Christian Quante (3):
>   tests/qtest/fdc-test: give each test case its own QEMU instance
>   tests/qtest/fdc-test: skip test cases when the 'pc' machine is missing
>   tests/qtest/fdc-test: check that READ ID answers for the drive it
>     names
>

Series lgtm, however it takes ~10s more now.
Tested-by: Marc-André Lureau <marcandre.lureau@redhat.com>


I guess I should submit my old patch
https://patchew.org/QEMU/20260512065633.3542562-1-marcandre.lureau@redhat.com/

and perhaps add fdc to the slow_qtests dictionary..
Re: [PATCH 0/3] tests/qtest/fdc-test: give each test case its own instance
Posted by Christian Quante 1 month ago
Hi Marc-André,

thanks for testing it.

> Series lgtm, however it takes ~10s more now.

I can't reproduce that here: fdc-test goes from 0.8s to 2.6s, so
+1.8s. With an --enable-asan build it is 3.4s, still nowhere near
+10s. The series takes the test from 2 QEMU instances to 16, at about
128ms each.

What are you measuring, and on what kind of build -- fdc-test on its
own?

Christian
Re: [PATCH 0/3] tests/qtest/fdc-test: give each test case its own instance
Posted by Fabiano Rosas 3 weeks, 2 days ago
Christian Quante <christian@quante.one> writes:

> Hi Marc-André,
>
> thanks for testing it.
>
>> Series lgtm, however it takes ~10s more now.
>
> I can't reproduce that here: fdc-test goes from 0.8s to 2.6s, so
> +1.8s. With an --enable-asan build it is 3.4s, still nowhere near
> +10s. The series takes the test from 2 QEMU instances to 16, at about
> 128ms each.
>
> What are you measuring, and on what kind of build -- fdc-test on its
> own?
>
> Christian

The timing looks ok on my side as well. I'll queue this series for the
moment. @Marc-André, feel free to object.

Thanks!
Re: [PATCH 0/3] tests/qtest/fdc-test: give each test case its own instance
Posted by Marc-André Lureau 3 weeks, 2 days ago
Hi

On Thu, Sep 3, 2026 at 5:56 PM Fabiano Rosas <farosas@suse.de> wrote:
>
> Christian Quante <christian@quante.one> writes:
>
> > Hi Marc-André,
> >
> > thanks for testing it.
> >
> >> Series lgtm, however it takes ~10s more now.
> >
> > I can't reproduce that here: fdc-test goes from 0.8s to 2.6s, so
> > +1.8s. With an --enable-asan build it is 3.4s, still nowhere near
> > +10s. The series takes the test from 2 QEMU instances to 16, at about
> > 128ms each.
> >
> > What are you measuring, and on what kind of build -- fdc-test on its
> > own?
> >
> > Christian
>
> The timing looks ok on my side as well. I'll queue this series for the
> moment. @Marc-André, feel free to object.
>

No problem, I couldn't immediately reproduce it. Yes, I was running
with ASAN, and another without it, and noticed those +10s. But I was
probably doing something heavy at the same time.