Skip to contentSkip to conversation

Add new CD-ROM related qtests

PATCH Applied Thomas Huth 2 participants 6 messages Apr 27, 2018 – Jun 1, 2018

lore
Share & tools
lore
#0 lore raw

v3 cover Add new CD-ROM related qtests

With one of my clean-up patches (see commit 1454509726719e0933c800), I recently accidentially broke the "-cdrom" parameter (more precisely "-drive if=scsi") on a couple of boards, since there was no error detected during the "make check" regression testing. This is clearly an indication that we are lacking tests in this area.
So this small patch series now introduces some tests for CD-ROM drives:
The first two patches introduce the possibility to check that booting from CD-ROM drives still works fine for x86 and s390x, and the third patch adds a test that certain machines can at least still be started with the "-cdrom" parameter (i.e. that test would have catched the mistake that I did with my SCSI cleanup patch).

v3:
- Rebased to current master branch
- Add a final patch that adds an entry to the MAINTAINERS file

v2:
- Use g_spawn_sync() instead of execlp() to run genisoimage
- The "-cdrom" parameter test is now run on all architectures (with machine "none" for the machines that are not explicitly checked)
- Some rewordings and improved comments here and there

Thomas Huth (4):
tests/boot-sector: Add magic bytes to s390x boot code header tests/cdrom-test: Test booting from CD-ROM ISO image file tests/cdrom-test: Test that -cdrom parameter is working
MAINTAINERS: Add the cdrom-test to John's section

 MAINTAINERS            |   1 +
 tests/Makefile.include |   2 +
 tests/boot-sector.c    |   9 +-
 tests/cdrom-test.c     | 222 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 231 insertions(+), 3 deletions(-)
 create mode 100644 tests/cdrom-test.c
Signature · 1 line
1.8.3.1
#1 lore raw

PATCH v3 1/4 tests/boot-sector: Add magic bytes to s390x boot code header

We're going to use the s390x boot code for testing CD-ROM booting. But the ISO loader of the s390-ccw bios is a little bit more picky than the network loader and expects some magic bytes in the header of the file (see linux_s390_magic in pc-bios/s390-ccw/bootmap.c), so we've got to add them in our boot code here, too.

---
 tests/boot-sector.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
tests/boot-sector.c +6 -3
index c373f0e..7824286 100644
@@ -68,8 +68,11 @@ static uint8_t x86_boot_sector[512] = {
 };
 
 /* For s390x, use a mini "kernel" with the appropriate signature */
-static const uint8_t s390x_psw[] = {
-    0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00
+static const uint8_t s390x_psw_and_magic[] = {
+    0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00,  /* Program status word  */
+    0x02, 0x00, 0x00, 0x18, 0x60, 0x00, 0x00, 0x50,  /* Magic:               */
+    0x02, 0x00, 0x00, 0x68, 0x60, 0x00, 0x00, 0x50,  /* see linux_s390_magic */
+    0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40   /* in the s390-ccw bios */
 };
 static const uint8_t s390x_code[] = {
     0xa7, 0xf4, 0x00, 0x0a,                                /* j 0x10010 */
@@ -110,7 +113,7 @@ int boot_sector_init(char *fname)
     } else if (g_str_equal(arch, "s390x")) {
         len = 0x10000 + sizeof(s390x_code);
         boot_code = g_malloc0(len);
-        memcpy(boot_code, s390x_psw, sizeof(s390x_psw));
+        memcpy(boot_code, s390x_psw_and_magic, sizeof(s390x_psw_and_magic));
         memcpy(&boot_code[0x10000], s390x_code, sizeof(s390x_code));
     } else {
         g_assert_not_reached();
Signature · 1 line
1.8.3.1
#2 lore raw

PATCH v3 2/4 tests/cdrom-test: Test booting from CD-ROM ISO image file

In reply to Thomas Huth

We already have the code for a boot file in tests/boot-sector.c, so if the genisoimage program is available, we can easily create a bootable CD ISO image that we can use for testing whether our CD-ROM emulation and the BIOS CD-ROM boot works correctly.

---
 tests/Makefile.include |   2 +
 tests/cdrom-test.c     | 164 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 166 insertions(+)
 create mode 100644 tests/cdrom-test.c
tests/Makefile.include +2 -0
index 3b9a5e3..41abda0 100644
@@ -179,6 +179,7 @@ check-qtest-generic-y = tests/qmp-test$(EXESUF)
 gcov-files-generic-y = monitor.c qapi/qmp-dispatch.c
 check-qtest-generic-y += tests/device-introspect-test$(EXESUF)
 gcov-files-generic-y = qdev-monitor.c qmp.c
+check-qtest-generic-y += tests/cdrom-test$(EXESUF)
 
 gcov-files-ipack-y += hw/ipack/ipack.c
 check-qtest-ipack-y += tests/ipoctal232-test$(EXESUF)
@@ -835,6 +836,7 @@ tests/test-qapi-util$(EXESUF): tests/test-qapi-util.o $(test-util-obj-y)
 tests/numa-test$(EXESUF): tests/numa-test.o
 tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o tests/boot-sector.o tests/acpi-utils.o
 tests/sdhci-test$(EXESUF): tests/sdhci-test.o $(libqos-pc-obj-y)
+tests/cdrom-test$(EXESUF): tests/cdrom-test.o tests/boot-sector.o $(libqos-obj-y)
 
 tests/migration/stress$(EXESUF): tests/migration/stress.o
 	$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< ,"LINK","$(TARGET_DIR)$@")
tests/cdrom-test.c new file +164 -0

164 lines not on this page · raw message

Signature · 1 line
1.8.3.1
#3 lore raw

PATCH v3 3/4 tests/cdrom-test: Test that -cdrom parameter is working

In reply to Thomas Huth

Commit 1454509726719e0933c800 recently broke the "-cdrom" parameter on a couple of boards without us noticing it immediately. Thus let's add a test which checks that "-cdrom" can at least be used to start QEMU with certain machine types.

---
 tests/cdrom-test.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)
tests/cdrom-test.c +58 -0
index 5bbf322..7a1fce5 100644
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "libqtest.h"
 #include "boot-sector.h"
+#include "qapi/qmp/qdict.h"
 
 static char isoimage[] = "cdrom-boot-iso-XXXXXX";
 
@@ -89,6 +90,32 @@ cleanup:
     return ret;
 }
 
+/**
+ * Check that at least the -cdrom parameter is basically working, i.e. we can
+ * see the filename of the ISO image in the output of "info block" afterwards
+ */
+static void test_cdrom_param(gconstpointer data)
+{
+    QTestState *qts;
+    char *resp;
+
+    qts = qtest_startf("-M %s -cdrom %s", (const char *)data, isoimage);
+    resp = qtest_hmp(qts, "info block");
+    g_assert(strstr(resp, isoimage) != 0);
+    g_free(resp);
+    qtest_quit(qts);
+}
+
+static void add_cdrom_param_tests(const char **machines)
+{
+    while (*machines) {
+        char *testname = g_strdup_printf("cdrom/param/%s", *machines);
+        qtest_add_data_func(testname, *machines, test_cdrom_param);
+        g_free(testname);
+        machines++;
+    }
+}
+
 static void test_cdboot(gconstpointer data)
 {
     QTestState *qts;
@@ -154,6 +181,37 @@ int main(int argc, char **argv)
         add_x86_tests();
     } else if (g_str_equal(arch, "s390x")) {
         add_s390x_tests();
+    } else if (g_str_equal(arch, "ppc64")) {
+        const char *ppcmachines[] = {
+            "pseries", "mac99", "g3beige", "40p", "prep", NULL
+        };
+        add_cdrom_param_tests(ppcmachines);
+    } else if (g_str_equal(arch, "sparc")) {
+        const char *sparcmachines[] = {
+            "LX", "SPARCClassic", "SPARCbook", "SS-10", "SS-20", "SS-4",
+            "SS-5", "SS-600MP", "Voyager", "leon3_generic", NULL
+        };
+        add_cdrom_param_tests(sparcmachines);
+    } else if (g_str_equal(arch, "sparc64")) {
+        const char *sparc64machines[] = {
+            "niagara", "sun4u", "sun4v", NULL
+        };
+        add_cdrom_param_tests(sparc64machines);
+    } else if (!strncmp(arch, "mips64", 6)) {
+        const char *mips64machines[] = {
+            "magnum", "malta", "mips", "pica61", NULL
+        };
+        add_cdrom_param_tests(mips64machines);
+    } else if (g_str_equal(arch, "arm") || g_str_equal(arch, "aarch64")) {
+        const char *armmachines[] = {
+            "realview-eb", "realview-eb-mpcore", "realview-pb-a8",
+            "realview-pbx-a9", "versatileab", "versatilepb", "vexpress-a15",
+            "vexpress-a9", "virt", NULL
+        };
+        add_cdrom_param_tests(armmachines);
+    } else {
+        const char *nonemachine[] = { "none", NULL };
+        add_cdrom_param_tests(nonemachine);
     }
 
     ret = g_test_run();
Signature · 1 line
1.8.3.1
#4 lore raw

PATCH v3 4/4 MAINTAINERS: Add the cdrom-test to John's section

In reply to Thomas Huth

The cdrom-test checks various block types - IDE, SCSI and virtio, so it's a little bit hard to decide where this should belong to in the MAINTAINERS file. But John volunteered to take it, so let's put it into the IDE section for now.

---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)
MAINTAINERS +1 -0
index 24b7016..6cceb10 100644
@@ -998,6 +998,7 @@ F: hw/block/cdrom.c
 F: hw/block/hd-geometry.c
 F: tests/ide-test.c
 F: tests/ahci-test.c
+F: tests/cdrom-test.c
 F: tests/libqos/ahci*
 T: git git://github.com/jnsnow/qemu.git ide
 
Signature · 1 line
1.8.3.1
Applied
#5 lore raw
In reply to Thomas Huth
Quoted 28 lines · Thomas Huth

With one of my clean-up patches (see commit 1454509726719e0933c800), I recently accidentially broke the "-cdrom" parameter (more precisely "-drive if=scsi") on a couple of boards, since there was no error detected during the "make check" regression testing. This is clearly an indication that we are lacking tests in this area.
So this small patch series now introduces some tests for CD-ROM drives:
The first two patches introduce the possibility to check that booting from CD-ROM drives still works fine for x86 and s390x, and the third patch adds a test that certain machines can at least still be started with the "-cdrom" parameter (i.e. that test would have catched the mistake that I did with my SCSI cleanup patch).

v3:
- Rebased to current master branch
- Add a final patch that adds an entry to the MAINTAINERS file

v2:
- Use g_spawn_sync() instead of execlp() to run genisoimage
- The "-cdrom" parameter test is now run on all architectures (with machine "none" for the machines that are not explicitly checked)
- Some rewordings and improved comments here and there

Thomas Huth (4):
tests/boot-sector: Add magic bytes to s390x boot code header tests/cdrom-test: Test booting from CD-ROM ISO image file tests/cdrom-test: Test that -cdrom parameter is working
MAINTAINERS: Add the cdrom-test to John's section

Quoted 7 lines
 MAINTAINERS            |   1 +
 tests/Makefile.include |   2 +
 tests/boot-sector.c    |   9 +-
 tests/cdrom-test.c     | 222 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 231 insertions(+), 3 deletions(-)
 create mode 100644 tests/cdrom-test.c

*coughs*

Thanks, applied to my IDE tree:

https://github.com/jnsnow/qemu/commits/ide
https://github.com/jnsnow/qemu.git

--js