[Qemu-devel] [PATCH v2 0/3] Add new CD-ROM related qtests

[Qemu-devel] [PATCH v2 0/3] Add new CD-ROM related qtests

[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).

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 (3):
  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

 tests/Makefile.include |   2 +
 tests/boot-sector.c    |   9 +-
 tests/cdrom-test.c     | 222 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 230 insertions(+), 3 deletions(-)
 create mode 100644 tests/cdrom-test.c

-- 
1.8.3.1

[Qemu-devel] [PATCH v2 1/3] tests/boot-sector: Add magic bytes to s390x boot code header

[Thomas Huth]

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.

Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/boot-sector.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tests/boot-sector.c b/tests/boot-sector.c
index c373f0e..7824286 100644
--- a/tests/boot-sector.c
+++ b/tests/boot-sector.c
@@ -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();
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 2/3] tests/cdrom-test: Test booting from CD-ROM ISO image file

[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.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include |   2 +
 tests/cdrom-test.c     | 164 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 166 insertions(+)
 create mode 100644 tests/cdrom-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index ef9b88c..37ca258 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -178,6 +178,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)
@@ -829,6 +830,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)$@")
diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
new file mode 100644
index 0000000..5bbf322
--- /dev/null
+++ b/tests/cdrom-test.c
@@ -0,0 +1,164 @@
+/*
+ * Various tests for emulated CD-ROM drives.
+ *
+ * Copyright (c) 2018 Red Hat Inc.
+ *
+ * Author:
+ *    Thomas Huth <thuth@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2
+ * or later. See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "boot-sector.h"
+
+static char isoimage[] = "cdrom-boot-iso-XXXXXX";
+
+static int exec_genisoimg(const char **args)
+{
+    gchar *out_err = NULL;
+    gint exit_status = -1;
+    bool success;
+
+    success = g_spawn_sync(NULL, (gchar **)args, NULL,
+                           G_SPAWN_SEARCH_PATH | G_SPAWN_STDOUT_TO_DEV_NULL,
+                           NULL, NULL, NULL, &out_err, &exit_status, NULL);
+    if (!success) {
+        return -ENOENT;
+    }
+    if (out_err) {
+        fputs(out_err, stderr);
+        g_free(out_err);
+    }
+
+    return exit_status;
+}
+
+static int prepare_image(const char *arch, char *isoimage)
+{
+    char srcdir[] = "cdrom-test-dir-XXXXXX";
+    char *codefile = NULL;
+    int ifh, ret = -1;
+    const char *args[] = {
+        "genisoimage", "-quiet", "-l", "-no-emul-boot",
+        "-b", NULL, "-o", isoimage, srcdir, NULL
+    };
+
+    ifh = mkstemp(isoimage);
+    if (ifh < 0) {
+        perror("Error creating temporary iso image file");
+        return -1;
+    }
+    if (!mkdtemp(srcdir)) {
+        perror("Error creating temporary directory");
+        goto cleanup;
+    }
+
+    if (g_str_equal(arch, "i386") || g_str_equal(arch, "x86_64") ||
+        g_str_equal(arch, "s390x")) {
+        codefile = g_strdup_printf("%s/bootcode-XXXXXX", srcdir);
+        ret = boot_sector_init(codefile);
+        if (ret) {
+            goto cleanup;
+        }
+    } else {
+        /* Just create a dummy file */
+        char txt[] = "empty disc";
+        codefile = g_strdup_printf("%s/readme.txt", srcdir);
+        if (!g_file_set_contents(codefile, txt, sizeof(txt) - 1, NULL)) {
+            fprintf(stderr, "Failed to create '%s'\n", codefile);
+            goto cleanup;
+        }
+    }
+
+    args[5] = strchr(codefile, '/') + 1;
+    ret = exec_genisoimg(args);
+    if (ret) {
+        fprintf(stderr, "genisoimage failed: %i\n", ret);
+    }
+
+    unlink(codefile);
+
+cleanup:
+    g_free(codefile);
+    rmdir(srcdir);
+    close(ifh);
+
+    return ret;
+}
+
+static void test_cdboot(gconstpointer data)
+{
+    QTestState *qts;
+
+    qts = qtest_startf("-accel kvm:tcg -no-shutdown %s%s", (const char *)data,
+                       isoimage);
+    boot_sector_test(qts);
+    qtest_quit(qts);
+}
+
+static void add_x86_tests(void)
+{
+    qtest_add_data_func("cdrom/boot/default", "-cdrom ", test_cdboot);
+    qtest_add_data_func("cdrom/boot/virtio-scsi",
+                        "-device virtio-scsi -device scsi-cd,drive=cdr "
+                        "-blockdev file,node-name=cdr,filename=", test_cdboot);
+    qtest_add_data_func("cdrom/boot/isapc", "-M isapc "
+                        "-drive if=ide,media=cdrom,file=", test_cdboot);
+    qtest_add_data_func("cdrom/boot/am53c974",
+                        "-device am53c974 -device scsi-cd,drive=cd1 "
+                        "-drive if=none,id=cd1,format=raw,file=", test_cdboot);
+    qtest_add_data_func("cdrom/boot/dc390",
+                        "-device dc390 -device scsi-cd,drive=cd1 "
+                        "-blockdev file,node-name=cd1,filename=", test_cdboot);
+    qtest_add_data_func("cdrom/boot/lsi53c895a",
+                        "-device lsi53c895a -device scsi-cd,drive=cd1 "
+                        "-blockdev file,node-name=cd1,filename=", test_cdboot);
+    qtest_add_data_func("cdrom/boot/megasas", "-M q35 "
+                        "-device megasas -device scsi-cd,drive=cd1 "
+                        "-blockdev file,node-name=cd1,filename=", test_cdboot);
+    qtest_add_data_func("cdrom/boot/megasas-gen2", "-M q35 "
+                        "-device megasas-gen2 -device scsi-cd,drive=cd1 "
+                        "-blockdev file,node-name=cd1,filename=", test_cdboot);
+}
+
+static void add_s390x_tests(void)
+{
+    qtest_add_data_func("cdrom/boot/default", "-cdrom ", test_cdboot);
+    qtest_add_data_func("cdrom/boot/virtio-scsi",
+                        "-device virtio-scsi -device scsi-cd,drive=cdr "
+                        "-blockdev file,node-name=cdr,filename=", test_cdboot);
+}
+
+int main(int argc, char **argv)
+{
+    int ret;
+    const char *arch = qtest_get_arch();
+    const char *genisocheck[] = { "genisoimage", "-version", NULL };
+
+    g_test_init(&argc, &argv, NULL);
+
+    if (exec_genisoimg(genisocheck)) {
+        /* genisoimage not available - so can't run tests */
+        return 0;
+    }
+
+    ret = prepare_image(arch, isoimage);
+    if (ret) {
+        return ret;
+    }
+
+    if (g_str_equal(arch, "i386") || g_str_equal(arch, "x86_64")) {
+        add_x86_tests();
+    } else if (g_str_equal(arch, "s390x")) {
+        add_s390x_tests();
+    }
+
+    ret = g_test_run();
+
+    unlink(isoimage);
+
+    return ret;
+}
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 3/3] tests/cdrom-test: Test that -cdrom parameter is working

[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.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/cdrom-test.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
index 5bbf322..7a1fce5 100644
--- a/tests/cdrom-test.c
+++ b/tests/cdrom-test.c
@@ -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();
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 0/3] Add new CD-ROM related qtests

[Hervé Poussineau]

Le 16/03/2018 à 06:39, Thomas Huth a écrit :
> 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).

Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>

> 
> 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 (3):
>    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
> 
>   tests/Makefile.include |   2 +
>   tests/boot-sector.c    |   9 +-
>   tests/cdrom-test.c     | 222 +++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 230 insertions(+), 3 deletions(-)
>   create mode 100644 tests/cdrom-test.c
> 

Re: [Qemu-devel] [PATCH v2 0/3] Add new CD-ROM related qtests

[Mark Cave-Ayland]

On 16/03/18 05:39, Thomas Huth wrote:

> 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).
> 
> 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 (3):
>    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
> 
>   tests/Makefile.include |   2 +
>   tests/boot-sector.c    |   9 +-
>   tests/cdrom-test.c     | 222 +++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 230 insertions(+), 3 deletions(-)
>   create mode 100644 tests/cdrom-test.c

Hi Thomas,

Nice work - looks like a good, comprehensive test of the -cdrom option.

Acked-By: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

Re: [Qemu-devel] [PATCH v2 2/3] tests/cdrom-test: Test booting from CD-ROM ISO image file

[Philippe Mathieu-Daudé]

On 03/16/2018 06:39 AM, Thomas Huth wrote:
> 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.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  tests/Makefile.include |   2 +
>  tests/cdrom-test.c     | 164 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 166 insertions(+)
>  create mode 100644 tests/cdrom-test.c
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index ef9b88c..37ca258 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -178,6 +178,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)
> @@ -829,6 +830,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)$@")
> diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
> new file mode 100644
> index 0000000..5bbf322
> --- /dev/null
> +++ b/tests/cdrom-test.c
> @@ -0,0 +1,164 @@
> +/*
> + * Various tests for emulated CD-ROM drives.
> + *
> + * Copyright (c) 2018 Red Hat Inc.
> + *
> + * Author:
> + *    Thomas Huth <thuth@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2
> + * or later. See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqtest.h"
> +#include "boot-sector.h"
> +
> +static char isoimage[] = "cdrom-boot-iso-XXXXXX";
> +
> +static int exec_genisoimg(const char **args)
> +{
> +    gchar *out_err = NULL;
> +    gint exit_status = -1;
> +    bool success;
> +
> +    success = g_spawn_sync(NULL, (gchar **)args, NULL,
> +                           G_SPAWN_SEARCH_PATH | G_SPAWN_STDOUT_TO_DEV_NULL,
> +                           NULL, NULL, NULL, &out_err, &exit_status, NULL);
> +    if (!success) {
> +        return -ENOENT;
> +    }
> +    if (out_err) {
> +        fputs(out_err, stderr);
> +        g_free(out_err);
> +    }
> +
> +    return exit_status;
> +}
> +
> +static int prepare_image(const char *arch, char *isoimage)
> +{
> +    char srcdir[] = "cdrom-test-dir-XXXXXX";
> +    char *codefile = NULL;
> +    int ifh, ret = -1;
> +    const char *args[] = {
> +        "genisoimage", "-quiet", "-l", "-no-emul-boot",
> +        "-b", NULL, "-o", isoimage, srcdir, NULL
> +    };
> +
> +    ifh = mkstemp(isoimage);
> +    if (ifh < 0) {
> +        perror("Error creating temporary iso image file");
> +        return -1;
> +    }
> +    if (!mkdtemp(srcdir)) {
> +        perror("Error creating temporary directory");
> +        goto cleanup;
> +    }
> +
> +    if (g_str_equal(arch, "i386") || g_str_equal(arch, "x86_64") ||
> +        g_str_equal(arch, "s390x")) {
> +        codefile = g_strdup_printf("%s/bootcode-XXXXXX", srcdir);
> +        ret = boot_sector_init(codefile);
> +        if (ret) {
> +            goto cleanup;
> +        }
> +    } else {
> +        /* Just create a dummy file */
> +        char txt[] = "empty disc";
> +        codefile = g_strdup_printf("%s/readme.txt", srcdir);
> +        if (!g_file_set_contents(codefile, txt, sizeof(txt) - 1, NULL)) {
> +            fprintf(stderr, "Failed to create '%s'\n", codefile);
> +            goto cleanup;
> +        }
> +    }
> +
> +    args[5] = strchr(codefile, '/') + 1;
> +    ret = exec_genisoimg(args);
> +    if (ret) {
> +        fprintf(stderr, "genisoimage failed: %i\n", ret);
> +    }
> +
> +    unlink(codefile);
> +
> +cleanup:
> +    g_free(codefile);
> +    rmdir(srcdir);
> +    close(ifh);
> +
> +    return ret;
> +}
> +
> +static void test_cdboot(gconstpointer data)
> +{
> +    QTestState *qts;
> +
> +    qts = qtest_startf("-accel kvm:tcg -no-shutdown %s%s", (const char *)data,
> +                       isoimage);
> +    boot_sector_test(qts);
> +    qtest_quit(qts);
> +}
> +
> +static void add_x86_tests(void)
> +{
> +    qtest_add_data_func("cdrom/boot/default", "-cdrom ", test_cdboot);
> +    qtest_add_data_func("cdrom/boot/virtio-scsi",
> +                        "-device virtio-scsi -device scsi-cd,drive=cdr "
> +                        "-blockdev file,node-name=cdr,filename=", test_cdboot);
> +    qtest_add_data_func("cdrom/boot/isapc", "-M isapc "
> +                        "-drive if=ide,media=cdrom,file=", test_cdboot);
> +    qtest_add_data_func("cdrom/boot/am53c974",
> +                        "-device am53c974 -device scsi-cd,drive=cd1 "
> +                        "-drive if=none,id=cd1,format=raw,file=", test_cdboot);
> +    qtest_add_data_func("cdrom/boot/dc390",
> +                        "-device dc390 -device scsi-cd,drive=cd1 "
> +                        "-blockdev file,node-name=cd1,filename=", test_cdboot);
> +    qtest_add_data_func("cdrom/boot/lsi53c895a",
> +                        "-device lsi53c895a -device scsi-cd,drive=cd1 "
> +                        "-blockdev file,node-name=cd1,filename=", test_cdboot);
> +    qtest_add_data_func("cdrom/boot/megasas", "-M q35 "
> +                        "-device megasas -device scsi-cd,drive=cd1 "
> +                        "-blockdev file,node-name=cd1,filename=", test_cdboot);
> +    qtest_add_data_func("cdrom/boot/megasas-gen2", "-M q35 "
> +                        "-device megasas-gen2 -device scsi-cd,drive=cd1 "
> +                        "-blockdev file,node-name=cd1,filename=", test_cdboot);
> +}
> +
> +static void add_s390x_tests(void)
> +{
> +    qtest_add_data_func("cdrom/boot/default", "-cdrom ", test_cdboot);
> +    qtest_add_data_func("cdrom/boot/virtio-scsi",
> +                        "-device virtio-scsi -device scsi-cd,drive=cdr "
> +                        "-blockdev file,node-name=cdr,filename=", test_cdboot);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    int ret;
> +    const char *arch = qtest_get_arch();
> +    const char *genisocheck[] = { "genisoimage", "-version", NULL };
> +
> +    g_test_init(&argc, &argv, NULL);
> +
> +    if (exec_genisoimg(genisocheck)) {
> +        /* genisoimage not available - so can't run tests */
> +        return 0;
> +    }
> +
> +    ret = prepare_image(arch, isoimage);
> +    if (ret) {
> +        return ret;
> +    }
> +
> +    if (g_str_equal(arch, "i386") || g_str_equal(arch, "x86_64")) {
> +        add_x86_tests();
> +    } else if (g_str_equal(arch, "s390x")) {
> +        add_s390x_tests();
> +    }
> +
> +    ret = g_test_run();
> +
> +    unlink(isoimage);
> +
> +    return ret;
> +}
> 

Re: [Qemu-devel] [PATCH v2 1/3] tests/boot-sector: Add magic bytes to s390x boot code header

[Philippe Mathieu-Daudé]

On 03/16/2018 06:39 AM, Thomas Huth wrote:
> 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.
> 
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/boot-sector.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/boot-sector.c b/tests/boot-sector.c
> index c373f0e..7824286 100644
> --- a/tests/boot-sector.c
> +++ b/tests/boot-sector.c
> @@ -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();
> 

Re: [Qemu-devel] [PATCH v2 0/3] Add new CD-ROM related qtests

[Michael S. Tsirkin]

On Fri, Mar 16, 2018 at 06:39:52AM +0100, Thomas Huth wrote:
> 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).


Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> 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 (3):
>   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
> 
>  tests/Makefile.include |   2 +
>  tests/boot-sector.c    |   9 +-
>  tests/cdrom-test.c     | 222 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 230 insertions(+), 3 deletions(-)
>  create mode 100644 tests/cdrom-test.c
> 
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH v2 0/3] Add new CD-ROM related qtests

[John Snow]

On 03/16/2018 01:39 AM, Thomas Huth wrote:
> 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).
> 
> 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 (3):
>   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
> 
>  tests/Makefile.include |   2 +
>  tests/boot-sector.c    |   9 +-
>  tests/cdrom-test.c     | 222 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 230 insertions(+), 3 deletions(-)
>  create mode 100644 tests/cdrom-test.c
> 

New file, but no edit to MAINTAINERS.

Re: [Qemu-devel] [PATCH v2 0/3] Add new CD-ROM related qtests

[Thomas Huth]

On 29.03.2018 20:28, John Snow wrote:
> 
> 
> On 03/16/2018 01:39 AM, Thomas Huth wrote:
>> 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).
>>
>> 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 (3):
>>   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
>>
>>  tests/Makefile.include |   2 +
>>  tests/boot-sector.c    |   9 +-
>>  tests/cdrom-test.c     | 222 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 230 insertions(+), 3 deletions(-)
>>  create mode 100644 tests/cdrom-test.c
>>
> 
> New file, but no edit to MAINTAINERS.

Which section do you suggest? It tests IDE CD-ROMs, SCSI CD-ROMs, and
even virtio-block (on s390x) ... so I have a hard time to decide where
this should belong to...

 Thomas

Re: [Qemu-devel] [PATCH v2 0/3] Add new CD-ROM related qtests

[John Snow]

On 04/02/2018 02:39 PM, Thomas Huth wrote:
> On 29.03.2018 20:28, John Snow wrote:
>>
>>
>> On 03/16/2018 01:39 AM, Thomas Huth wrote:
>>> 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).
>>>
>>> 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 (3):
>>>   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
>>>
>>>  tests/Makefile.include |   2 +
>>>  tests/boot-sector.c    |   9 +-
>>>  tests/cdrom-test.c     | 222 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 230 insertions(+), 3 deletions(-)
>>>  create mode 100644 tests/cdrom-test.c
>>>
>>
>> New file, but no edit to MAINTAINERS.
> 
> Which section do you suggest? It tests IDE CD-ROMs, SCSI CD-ROMs, and
> even virtio-block (on s390x) ... so I have a hard time to decide where
> this should belong to...
> 
>  Thomas
> 

I was hoping you'd figure it out, but fine :D

You can stick it under my section if you want, but I'll probably defer
to you if the s390x parts break.

--js

Re: [Qemu-devel] [PATCH v2 0/3] Add new CD-ROM related qtests

[Thomas Huth]

On 02.04.2018 21:34, John Snow wrote:
> 
> 
> On 04/02/2018 02:39 PM, Thomas Huth wrote:
>> On 29.03.2018 20:28, John Snow wrote:
>>>
>>>
>>> On 03/16/2018 01:39 AM, Thomas Huth wrote:
>>>> 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).
>>>>
>>>> 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 (3):
>>>>   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
>>>>
>>>>  tests/Makefile.include |   2 +
>>>>  tests/boot-sector.c    |   9 +-
>>>>  tests/cdrom-test.c     | 222 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 230 insertions(+), 3 deletions(-)
>>>>  create mode 100644 tests/cdrom-test.c
>>>>
>>>
>>> New file, but no edit to MAINTAINERS.
>>
>> Which section do you suggest? It tests IDE CD-ROMs, SCSI CD-ROMs, and
>> even virtio-block (on s390x) ... so I have a hard time to decide where
>> this should belong to...
>>
>>  Thomas
>>
> 
> I was hoping you'd figure it out, but fine :D
> 
> You can stick it under my section if you want, but I'll probably defer
> to you if the s390x parts break.

Ok, thanks. But in the long run, we might even need a generic "QTESTS"
section in the MAINTAINERS file, since most of the qtests are currently
not listed there.
And while we're at it, we should maybe also move the qtests to a
separate folder tests/qtests/ or so, so that it is clearer which of the
tests is a qtest and which are something different.
I'll try to come up with some patches once the hard freeze is over...

 Thomas