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

The test cases share one QEMU instance and, with it, the state of the
floppy controller.  Several of them therefore only pass in the order they
happen to be registered in: test_read_no_dma_1, _18 and _19 read the medium
that test_media_insert leaves in the drive, so running any of them on its
own fails.  fuzz-registers is worse -- it writes 1000 random values to the
eight I/O ports at FLOPPY_BASE and restores none of them, which is harmless
only because it is the last test case to run in the shared instance; the
two registered after it start one of their own.  Moving it anywhere else
leaves an unacknowledged interrupt behind and the next test dies in
send_seek().

Register the test cases from a table instead, through a wrapper that starts
a QEMU instance, runs one test and stops it again.  The test functions are
unchanged: they reach the current instance through global_qtest, which
qtest_start() reassigns for every test.  The three read_no_dma tests now
insert their own medium, and test_read_id no longer ejects at the end -- it
did that only to leave the drive as the next test expected to find it.

The two CVE test cases keep qtest_add_func(); they already run a QEMU
instance of their own and do not use global_qtest.

The same pattern is used by tests/qtest/boot-serial-test.c.

Every test case now passes on its own, and the suite passes with the
registration order reversed.

Signed-off-by: Christian Quante <[email protected]>
---
 tests/qtest/fdc-test.c | 71 ++++++++++++++++++++++++++++++------------
 1 file changed, 51 insertions(+), 20 deletions(-)

diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c
index 1e1dd8659d..fd38475bf7 100644
--- a/tests/qtest/fdc-test.c
+++ b/tests/qtest/fdc-test.c
@@ -510,9 +510,6 @@ static void test_read_id(void)
     g_assert_cmpint(cyl, ==, 8);
     g_assert_cmpint(head, ==, 1);
     g_assert_cmpint(st0, ==, head << 2);
-
-    /* Leave the drive empty, the way the machine starts up. */
-    media_eject();
 }
 
 /*
@@ -552,6 +549,7 @@ static void test_read_no_dma_1(void)
 {
     uint8_t ret;
 
+    media_insert();
     outb(FLOPPY_BASE + reg_dor, inb(FLOPPY_BASE + reg_dor) & ~0x08);
     send_seek(0);
     ret = send_read_no_dma_command(1, 0x04);
@@ -562,6 +560,7 @@ static void test_read_no_dma_18(void)
 {
     uint8_t ret;
 
+    media_insert();
     outb(FLOPPY_BASE + reg_dor, inb(FLOPPY_BASE + reg_dor) & ~0x08);
     send_seek(0);
     ret = send_read_no_dma_command(18, 0x04);
@@ -572,6 +571,7 @@ static void test_read_no_dma_19(void)
 {
     uint8_t ret;
 
+    media_insert();
     outb(FLOPPY_BASE + reg_dor, inb(FLOPPY_BASE + reg_dor) & ~0x08);
     send_seek(0);
     ret = send_read_no_dma_command(19, 0x20);
@@ -656,10 +656,53 @@ static void test_cve_2021_3507(void)
     qtest_quit(s);
 }
 
+/*
+ * Each test gets its own QEMU instance, so that none of them depends on the
+ * state another one left behind.  Without this, several tests only pass in
+ * the order they happen to be registered in: the read_no_dma tests need the
+ * medium that test_media_insert leaves in the drive, and fuzz-registers
+ * writes random values to the eight I/O ports at FLOPPY_BASE and restores
+ * none of them.
+ */
+typedef struct {
+    const char *path;
+    void (*fn)(void);
+} FDCTest;
+
+static const FDCTest fdc_tests[] = {
+    { "/fdc/cmos",              test_cmos },
+    { "/fdc/no_media_on_start", test_no_media_on_start },
+    { "/fdc/read_without_media", test_read_without_media },
+    { "/fdc/media_change",      test_media_change },
+    { "/fdc/sense_interrupt",   test_sense_interrupt },
+    { "/fdc/relative_seek",     test_relative_seek },
+    { "/fdc/read_id",           test_read_id },
+    { "/fdc/read_id_no_media",  test_read_id_no_media },
+    { "/fdc/verify",            test_verify },
+    { "/fdc/media_insert",      test_media_insert },
+    { "/fdc/read_no_dma_1",     test_read_no_dma_1 },
+    { "/fdc/read_no_dma_18",    test_read_no_dma_18 },
+    { "/fdc/read_no_dma_19",    test_read_no_dma_19 },
+    { "/fdc/fuzz-registers",    fuzz_registers },
+};
+
+static void run_isolated(const void *data)
+{
+    const FDCTest *test = data;
+
+    qtest_start("-machine pc -device floppy,id=floppy0");
+    qtest_irq_intercept_in(global_qtest, "ioapic");
+
+    test->fn();
+
+    qtest_end();
+}
+
 int main(int argc, char **argv)
 {
     int fd;
     int ret;
+    size_t i;
 
     /* Create a temporary raw image */
     fd = g_file_open_tmp("qtest.XXXXXX", &test_image, NULL);
@@ -671,29 +714,17 @@ int main(int argc, char **argv)
     /* Run the tests */
     g_test_init(&argc, &argv, NULL);
 
-    qtest_start("-machine pc -device floppy,id=floppy0");
-    qtest_irq_intercept_in(global_qtest, "ioapic");
-    qtest_add_func("/fdc/cmos", test_cmos);
-    qtest_add_func("/fdc/no_media_on_start", test_no_media_on_start);
-    qtest_add_func("/fdc/read_without_media", test_read_without_media);
-    qtest_add_func("/fdc/media_change", test_media_change);
-    qtest_add_func("/fdc/sense_interrupt", test_sense_interrupt);
-    qtest_add_func("/fdc/relative_seek", test_relative_seek);
-    qtest_add_func("/fdc/read_id", test_read_id);
-    qtest_add_func("/fdc/read_id_no_media", test_read_id_no_media);
-    qtest_add_func("/fdc/verify", test_verify);
-    qtest_add_func("/fdc/media_insert", test_media_insert);
-    qtest_add_func("/fdc/read_no_dma_1", test_read_no_dma_1);
-    qtest_add_func("/fdc/read_no_dma_18", test_read_no_dma_18);
-    qtest_add_func("/fdc/read_no_dma_19", test_read_no_dma_19);
-    qtest_add_func("/fdc/fuzz-registers", fuzz_registers);
+    for (i = 0; i < ARRAY_SIZE(fdc_tests); i++) {
+        qtest_add_data_func(fdc_tests[i].path, &fdc_tests[i], run_isolated);
+    }
+
+    /* These use a QTestState of their own instead of global_qtest */
     qtest_add_func("/fdc/fuzz/cve_2021_20196", test_cve_2021_20196);
     qtest_add_func("/fdc/fuzz/cve_2021_3507", test_cve_2021_3507);
 
     ret = g_test_run();
 
     /* Cleanup */
-    qtest_end();
     unlink(test_image);
     g_free(test_image);
 
-- 
2.53.0