* [Qemu-devel] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header
2018-03-15 7:49 [Qemu-devel] [PATCH 0/3] Add new CD-ROM related qtests Thomas Huth
@ 2018-03-15 7:49 ` Thomas Huth
2018-03-15 8:10 ` [Qemu-devel] [qemu-s390x] " Christian Borntraeger
2018-03-15 11:47 ` [Qemu-devel] " Philippe Mathieu-Daudé
2018-03-15 7:49 ` [Qemu-devel] [PATCH 2/3] tests/cdboot: Test booting from CD-ROM ISO image file Thomas Huth
2018-03-15 7:49 ` [Qemu-devel] [PATCH 3/3] tests/cdrom-test: Test that -cdrom parameter is working Thomas Huth
2 siblings, 2 replies; 14+ messages in thread
From: Thomas Huth @ 2018-03-15 7:49 UTC (permalink / raw)
To: qemu-devel
Cc: Victor Kaplansky, Michael S. Tsirkin, qemu-block, qemu-s390x,
Peter Maydell, Hervé Poussineau, Mark Cave-Ayland
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.
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..04721c9 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,
+ 0x02, 0x00, 0x00, 0x18, 0x60, 0x00, 0x00, 0x50,
+ 0x02, 0x00, 0x00, 0x68, 0x60, 0x00, 0x00, 0x50,
+ 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40
};
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header
2018-03-15 7:49 ` [Qemu-devel] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header Thomas Huth
@ 2018-03-15 8:10 ` Christian Borntraeger
2018-03-15 11:47 ` [Qemu-devel] " Philippe Mathieu-Daudé
1 sibling, 0 replies; 14+ messages in thread
From: Christian Borntraeger @ 2018-03-15 8:10 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Peter Maydell, Victor Kaplansky, qemu-block, Michael S. Tsirkin,
Mark Cave-Ayland, qemu-s390x, Hervé Poussineau
On 03/15/2018 08:49 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.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.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..04721c9 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,
> + 0x02, 0x00, 0x00, 0x18, 0x60, 0x00, 0x00, 0x50,
> + 0x02, 0x00, 0x00, 0x68, 0x60, 0x00, 0x00, 0x50,
> + 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40
> };
> 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();
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header
2018-03-15 7:49 ` [Qemu-devel] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header Thomas Huth
2018-03-15 8:10 ` [Qemu-devel] [qemu-s390x] " Christian Borntraeger
@ 2018-03-15 11:47 ` Philippe Mathieu-Daudé
2018-03-15 19:32 ` Thomas Huth
2018-03-15 20:18 ` Michael S. Tsirkin
1 sibling, 2 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-03-15 11:47 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Peter Maydell, Victor Kaplansky, qemu-block, Michael S. Tsirkin,
Mark Cave-Ayland, qemu-s390x, Hervé Poussineau
On 03/15/2018 08:49 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.
>
> 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..04721c9 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[] = {
Can you add a comment such "see linux_s390_magic in
pc-bios/s390-ccw/bootmap.c"?
> + 0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00,
> + 0x02, 0x00, 0x00, 0x18, 0x60, 0x00, 0x00, 0x50,
> + 0x02, 0x00, 0x00, 0x68, 0x60, 0x00, 0x00, 0x50,
> + 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40
> };
> 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();
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header
2018-03-15 11:47 ` [Qemu-devel] " Philippe Mathieu-Daudé
@ 2018-03-15 19:32 ` Thomas Huth
2018-03-15 20:18 ` Michael S. Tsirkin
1 sibling, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2018-03-15 19:32 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Peter Maydell, Victor Kaplansky, qemu-block, Michael S. Tsirkin,
Mark Cave-Ayland, qemu-s390x, Hervé Poussineau
On 15.03.2018 12:47, Philippe Mathieu-Daudé wrote:
> On 03/15/2018 08:49 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.
>>
>> 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..04721c9 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[] = {
>
> Can you add a comment such "see linux_s390_magic in
> pc-bios/s390-ccw/bootmap.c"?
Sure, I'll add a comment.
Thomas
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header
2018-03-15 11:47 ` [Qemu-devel] " Philippe Mathieu-Daudé
2018-03-15 19:32 ` Thomas Huth
@ 2018-03-15 20:18 ` Michael S. Tsirkin
1 sibling, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2018-03-15 20:18 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Thomas Huth, qemu-devel, Peter Maydell, Victor Kaplansky,
qemu-block, Mark Cave-Ayland, qemu-s390x, Hervé Poussineau
On Thu, Mar 15, 2018 at 12:47:08PM +0100, Philippe Mathieu-Daudé wrote:
> On 03/15/2018 08:49 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.
> >
> > 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..04721c9 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[] = {
>
> Can you add a comment such "see linux_s390_magic in
> pc-bios/s390-ccw/bootmap.c"?
Or better yet, copy the code comment from there.
> > + 0x00, 0x08, 0x00, 0x00, 0x80, 0x01, 0x00, 0x00,
> > + 0x02, 0x00, 0x00, 0x18, 0x60, 0x00, 0x00, 0x50,
> > + 0x02, 0x00, 0x00, 0x68, 0x60, 0x00, 0x00, 0x50,
> > + 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40
> > };
> > 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();
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 2/3] tests/cdboot: Test booting from CD-ROM ISO image file
2018-03-15 7:49 [Qemu-devel] [PATCH 0/3] Add new CD-ROM related qtests Thomas Huth
2018-03-15 7:49 ` [Qemu-devel] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header Thomas Huth
@ 2018-03-15 7:49 ` Thomas Huth
2018-03-15 9:21 ` Daniel P. Berrangé
2018-03-15 7:49 ` [Qemu-devel] [PATCH 3/3] tests/cdrom-test: Test that -cdrom parameter is working Thomas Huth
2 siblings, 1 reply; 14+ messages in thread
From: Thomas Huth @ 2018-03-15 7:49 UTC (permalink / raw)
To: qemu-devel
Cc: Victor Kaplansky, Michael S. Tsirkin, qemu-block, qemu-s390x,
Peter Maydell, Hervé Poussineau, Mark Cave-Ayland
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 | 3 +
tests/cdrom-test.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 158 insertions(+)
create mode 100644 tests/cdrom-test.c
diff --git a/tests/Makefile.include b/tests/Makefile.include
index ef9b88c..a104222 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -304,6 +304,7 @@ check-qtest-i386-$(CONFIG_POSIX) += tests/test-filter-redirector$(EXESUF)
check-qtest-i386-y += tests/migration-test$(EXESUF)
check-qtest-i386-y += tests/test-x86-cpuid-compat$(EXESUF)
check-qtest-i386-y += tests/numa-test$(EXESUF)
+check-qtest-i386-y += tests/cdrom-test$(EXESUF)
check-qtest-x86_64-y += $(check-qtest-i386-y)
check-qtest-x86_64-y += tests/sdhci-test$(EXESUF)
gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
@@ -398,6 +399,7 @@ check-qtest-s390x-y += tests/virtio-balloon-test$(EXESUF)
check-qtest-s390x-y += tests/virtio-console-test$(EXESUF)
check-qtest-s390x-y += tests/virtio-serial-test$(EXESUF)
check-qtest-s390x-y += tests/cpu-plug-test$(EXESUF)
+check-qtest-s390x-y += tests/cdrom-test$(EXESUF)
check-qtest-generic-y += tests/qom-test$(EXESUF)
check-qtest-generic-y += tests/test-hmp$(EXESUF)
@@ -829,6 +831,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..1fc5230
--- /dev/null
+++ b/tests/cdrom-test.c
@@ -0,0 +1,155 @@
+/*
+ * 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 gen_iso(const char *fmt, ...)
+{
+ char *params, *command;
+ va_list args;
+ pid_t pid;
+ int status;
+
+ pid = fork();
+ if (pid == 0) {
+ va_start(args, fmt);
+ params = g_strdup_vprintf(fmt, args);
+ va_end(args);
+ command = g_strdup_printf("exec genisoimage %s", params);
+ g_free(params);
+ execlp("/bin/sh", "sh", "-c", command, NULL);
+ exit(1);
+ }
+ wait(&status);
+
+ return WEXITSTATUS(status);
+}
+
+static int prepare_image(const char *arch, char *isoimage)
+{
+ char srcdir[] = "cdrom-test-dir-XXXXXX";
+ char *codefile = NULL;
+ int ifh, ret = -1;
+
+ 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 {
+ g_assert_not_reached();
+ }
+
+ ret = gen_iso("-quiet -l -no-emul-boot -b %s -o %s %s",
+ strrchr(codefile, '/') + 1, isoimage, srcdir);
+ 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("cdboot/default", "-cdrom ", test_cdboot);
+ qtest_add_data_func("cdboot/virtio-scsi",
+ "-device virtio-scsi -device scsi-cd,drive=cdr "
+ "-blockdev file,node-name=cdr,filename=", test_cdboot);
+ qtest_add_data_func("cdboot/isapc", "-M isapc "
+ "-drive if=ide,media=cdrom,file=", test_cdboot);
+ qtest_add_data_func("cdboot/am53c974",
+ "-device am53c974 -device scsi-cd,drive=cd1 "
+ "-drive if=none,id=cd1,format=raw,file=", test_cdboot);
+ qtest_add_data_func("cdboot/dc390",
+ "-device dc390 -device scsi-cd,drive=cd1 "
+ "-blockdev file,node-name=cd1,filename=", test_cdboot);
+ qtest_add_data_func("cdboot/lsi53c895a",
+ "-device lsi53c895a -device scsi-cd,drive=cd1 "
+ "-blockdev file,node-name=cd1,filename=", test_cdboot);
+ qtest_add_data_func("cdboot/megasas", "-M q35 "
+ "-device megasas -device scsi-cd,drive=cd1 "
+ "-blockdev file,node-name=cd1,filename=", test_cdboot);
+ qtest_add_data_func("cdboot/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("cdboot/default", "-cdrom ", test_cdboot);
+ qtest_add_data_func("cdboot/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();
+
+ g_test_init(&argc, &argv, NULL);
+
+ if (gen_iso("--version > /dev/null")) {
+ /* 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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] tests/cdboot: Test booting from CD-ROM ISO image file
2018-03-15 7:49 ` [Qemu-devel] [PATCH 2/3] tests/cdboot: Test booting from CD-ROM ISO image file Thomas Huth
@ 2018-03-15 9:21 ` Daniel P. Berrangé
2018-03-15 10:48 ` Thomas Huth
0 siblings, 1 reply; 14+ messages in thread
From: Daniel P. Berrangé @ 2018-03-15 9:21 UTC (permalink / raw)
To: Thomas Huth
Cc: qemu-devel, Peter Maydell, Victor Kaplansky, qemu-block,
Michael S. Tsirkin, Mark Cave-Ayland, qemu-s390x,
Hervé Poussineau
On Thu, Mar 15, 2018 at 08:49:04AM +0100, 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>
> ---
> tests/Makefile.include | 3 +
> tests/cdrom-test.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 158 insertions(+)
> create mode 100644 tests/cdrom-test.c
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index ef9b88c..a104222 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -304,6 +304,7 @@ check-qtest-i386-$(CONFIG_POSIX) += tests/test-filter-redirector$(EXESUF)
> check-qtest-i386-y += tests/migration-test$(EXESUF)
> check-qtest-i386-y += tests/test-x86-cpuid-compat$(EXESUF)
> check-qtest-i386-y += tests/numa-test$(EXESUF)
> +check-qtest-i386-y += tests/cdrom-test$(EXESUF)
> check-qtest-x86_64-y += $(check-qtest-i386-y)
> check-qtest-x86_64-y += tests/sdhci-test$(EXESUF)
> gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
> @@ -398,6 +399,7 @@ check-qtest-s390x-y += tests/virtio-balloon-test$(EXESUF)
> check-qtest-s390x-y += tests/virtio-console-test$(EXESUF)
> check-qtest-s390x-y += tests/virtio-serial-test$(EXESUF)
> check-qtest-s390x-y += tests/cpu-plug-test$(EXESUF)
> +check-qtest-s390x-y += tests/cdrom-test$(EXESUF)
>
> check-qtest-generic-y += tests/qom-test$(EXESUF)
> check-qtest-generic-y += tests/test-hmp$(EXESUF)
> @@ -829,6 +831,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..1fc5230
> --- /dev/null
> +++ b/tests/cdrom-test.c
> @@ -0,0 +1,155 @@
> +/*
> + * 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 gen_iso(const char *fmt, ...)
> +{
> + char *params, *command;
> + va_list args;
> + pid_t pid;
> + int status;
> +
> + pid = fork();
> + if (pid == 0) {
> + va_start(args, fmt);
> + params = g_strdup_vprintf(fmt, args);
> + va_end(args);
> + command = g_strdup_printf("exec genisoimage %s", params);
> + g_free(params);
> + execlp("/bin/sh", "sh", "-c", command, NULL);
> + exit(1);
> + }
> + wait(&status);
IMHO this should just use g_spawn_sync(), also the use of
shell seems rather unneccessary - why not just run genisoimage
directly ?
https://developer.gnome.org/glib/stable/glib-Spawning-Processes.html#g-spawn-sync
> +
> + return WEXITSTATUS(status);
> +}
> +
> +static int prepare_image(const char *arch, char *isoimage)
> +{
> + char srcdir[] = "cdrom-test-dir-XXXXXX";
> + char *codefile = NULL;
> + int ifh, ret = -1;
> +
> + 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 {
> + g_assert_not_reached();
> + }
> +
> + ret = gen_iso("-quiet -l -no-emul-boot -b %s -o %s %s",
> + strrchr(codefile, '/') + 1, isoimage, srcdir);
It would be easy to just declare the args as an array here
char *genisoargv[] = {
"genisoimage", "-quiet", "-l", "-no-emul-boot", "-b",
strrchr(codefile, "/") + 1, "-o", isoimage, srcdir,
NULL,
};
to then pass to g_spawn_sync
> + if (ret) {
> + fprintf(stderr, "genisoimage failed: %i\n", ret);
> + }
> +
> + unlink(codefile);
> +
> +cleanup:
> + g_free(codefile);
> + rmdir(srcdir);
> + close(ifh);
> +
> + return ret;
> +}
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 :|
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] tests/cdboot: Test booting from CD-ROM ISO image file
2018-03-15 9:21 ` Daniel P. Berrangé
@ 2018-03-15 10:48 ` Thomas Huth
2018-03-15 11:57 ` Eric Blake
0 siblings, 1 reply; 14+ messages in thread
From: Thomas Huth @ 2018-03-15 10:48 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Peter Maydell, Victor Kaplansky, qemu-block,
Michael S. Tsirkin, Mark Cave-Ayland, qemu-s390x,
Hervé Poussineau
On 15.03.2018 10:21, Daniel P. Berrangé wrote:
> On Thu, Mar 15, 2018 at 08:49:04AM +0100, 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>
>> ---
>> tests/Makefile.include | 3 +
>> tests/cdrom-test.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 158 insertions(+)
>> create mode 100644 tests/cdrom-test.c
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index ef9b88c..a104222 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -304,6 +304,7 @@ check-qtest-i386-$(CONFIG_POSIX) += tests/test-filter-redirector$(EXESUF)
>> check-qtest-i386-y += tests/migration-test$(EXESUF)
>> check-qtest-i386-y += tests/test-x86-cpuid-compat$(EXESUF)
>> check-qtest-i386-y += tests/numa-test$(EXESUF)
>> +check-qtest-i386-y += tests/cdrom-test$(EXESUF)
>> check-qtest-x86_64-y += $(check-qtest-i386-y)
>> check-qtest-x86_64-y += tests/sdhci-test$(EXESUF)
>> gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
>> @@ -398,6 +399,7 @@ check-qtest-s390x-y += tests/virtio-balloon-test$(EXESUF)
>> check-qtest-s390x-y += tests/virtio-console-test$(EXESUF)
>> check-qtest-s390x-y += tests/virtio-serial-test$(EXESUF)
>> check-qtest-s390x-y += tests/cpu-plug-test$(EXESUF)
>> +check-qtest-s390x-y += tests/cdrom-test$(EXESUF)
>>
>> check-qtest-generic-y += tests/qom-test$(EXESUF)
>> check-qtest-generic-y += tests/test-hmp$(EXESUF)
>> @@ -829,6 +831,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..1fc5230
>> --- /dev/null
>> +++ b/tests/cdrom-test.c
>> @@ -0,0 +1,155 @@
>> +/*
>> + * 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 gen_iso(const char *fmt, ...)
>> +{
>> + char *params, *command;
>> + va_list args;
>> + pid_t pid;
>> + int status;
>> +
>> + pid = fork();
>> + if (pid == 0) {
>> + va_start(args, fmt);
>> + params = g_strdup_vprintf(fmt, args);
>> + va_end(args);
>> + command = g_strdup_printf("exec genisoimage %s", params);
>> + g_free(params);
>> + execlp("/bin/sh", "sh", "-c", command, NULL);
>> + exit(1);
>> + }
>> + wait(&status);
>
> IMHO this should just use g_spawn_sync(), also the use of
> shell seems rather unneccessary - why not just run genisoimage
> directly ?
That code was simply "inspired" from the execlp() code in
qtest_init_without_qmp_handshake() ... but g_spawn_sync() sounds like a
good alternative here, so I'll try to switch to that one instead.
Thanks,
Thomas
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] tests/cdboot: Test booting from CD-ROM ISO image file
2018-03-15 10:48 ` Thomas Huth
@ 2018-03-15 11:57 ` Eric Blake
0 siblings, 0 replies; 14+ messages in thread
From: Eric Blake @ 2018-03-15 11:57 UTC (permalink / raw)
To: Thomas Huth, Daniel P. Berrangé
Cc: Peter Maydell, Victor Kaplansky, qemu-block, Michael S. Tsirkin,
Mark Cave-Ayland, qemu-devel, qemu-s390x, Hervé Poussineau
On 03/15/2018 05:48 AM, Thomas Huth wrote:
>>> + pid = fork();
>>> + if (pid == 0) {
>>> + va_start(args, fmt);
>>> + params = g_strdup_vprintf(fmt, args);
>>> + va_end(args);
>>> + command = g_strdup_printf("exec genisoimage %s", params);
>>> + g_free(params);
>>> + execlp("/bin/sh", "sh", "-c", command, NULL);
>>> + exit(1);
>>> + }
>>> + wait(&status);
>>
>> IMHO this should just use g_spawn_sync(), also the use of
>> shell seems rather unneccessary
and potentially dangerous - if we aren't absolutely positive that we
aren't going to improperly expand shell metacharacters embedded in params.
>> - why not just run genisoimage
>> directly ?
>
> That code was simply "inspired" from the execlp() code in
> qtest_init_without_qmp_handshake()
Sounds like a good idea for a future cleanup patch ;)
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH 3/3] tests/cdrom-test: Test that -cdrom parameter is working
2018-03-15 7:49 [Qemu-devel] [PATCH 0/3] Add new CD-ROM related qtests Thomas Huth
2018-03-15 7:49 ` [Qemu-devel] [PATCH 1/3] tests/boot-sector: Add magic bytes to s390x boot code header Thomas Huth
2018-03-15 7:49 ` [Qemu-devel] [PATCH 2/3] tests/cdboot: Test booting from CD-ROM ISO image file Thomas Huth
@ 2018-03-15 7:49 ` Thomas Huth
2018-03-15 11:42 ` Philippe Mathieu-Daudé
2018-03-15 11:58 ` Eric Blake
2 siblings, 2 replies; 14+ messages in thread
From: Thomas Huth @ 2018-03-15 7:49 UTC (permalink / raw)
To: qemu-devel
Cc: Victor Kaplansky, Michael S. Tsirkin, qemu-block, qemu-s390x,
Peter Maydell, Hervé Poussineau, Mark Cave-Ayland
Commit 1454509726719e0933c800 recently broke the "-cdrom" parameter
on a couple of boards without that we noticed it immediately. Thus
add a test which checks that "-cdrom" can at least be used to start
QEMU with certain machine types.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/Makefile.include | 7 +++++-
tests/cdrom-test.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 68 insertions(+), 2 deletions(-)
diff --git a/tests/Makefile.include b/tests/Makefile.include
index a104222..b744fea 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -321,8 +321,9 @@ check-qtest-microblaze-y = tests/boot-serial-test$(EXESUF)
check-qtest-mips-y = tests/endianness-test$(EXESUF)
check-qtest-mips64-y = tests/endianness-test$(EXESUF)
+check-qtest-mips64-y += tests/cdrom-test$(EXESUF)
-check-qtest-mips64el-y = tests/endianness-test$(EXESUF)
+check-qtest-mips64el-y = $(check-qtest-mips64-y)
check-qtest-moxie-y = tests/boot-serial-test$(EXESUF)
@@ -356,6 +357,7 @@ check-qtest-ppc64-y += tests/display-vga-test$(EXESUF)
check-qtest-ppc64-y += tests/numa-test$(EXESUF)
check-qtest-ppc64-$(CONFIG_IVSHMEM) += tests/ivshmem-test$(EXESUF)
check-qtest-ppc64-y += tests/cpu-plug-test$(EXESUF)
+check-qtest-ppc64-y += tests/cdrom-test$(EXESUF)
check-qtest-sh4-y = tests/endianness-test$(EXESUF)
@@ -365,10 +367,12 @@ check-qtest-sparc-y = tests/prom-env-test$(EXESUF)
check-qtest-sparc-y += tests/m48t59-test$(EXESUF)
gcov-files-sparc-y = hw/timer/m48t59.c
check-qtest-sparc-y += tests/boot-serial-test$(EXESUF)
+check-qtest-sparc-y += tests/cdrom-test$(EXESUF)
check-qtest-sparc64-y = tests/endianness-test$(EXESUF)
check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF)
+check-qtest-sparc64-y += tests/cdrom-test$(EXESUF)
check-qtest-arm-y = tests/tmp105-test$(EXESUF)
check-qtest-arm-y += tests/ds1338-test$(EXESUF)
@@ -384,6 +388,7 @@ check-qtest-arm-y += tests/sdhci-test$(EXESUF)
check-qtest-aarch64-y = tests/numa-test$(EXESUF)
check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF)
+check-qtest-aarch64-y += tests/cdrom-test$(EXESUF)
check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
index 1fc5230..5e516dd 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";
@@ -62,7 +63,13 @@ static int prepare_image(const char *arch, char *isoimage)
goto cleanup;
}
} else {
- g_assert_not_reached();
+ /* 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;
+ }
}
ret = gen_iso("-quiet -l -no-emul-boot -b %s -o %s %s",
@@ -81,6 +88,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;
@@ -145,6 +178,34 @@ 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", "mipssim", "pica61", NULL
+ };
+ add_cdrom_param_tests(mips64machines);
+ } else if (g_str_equal(arch, "aarch64")) {
+ const char *aarch64machines[] = {
+ "realview-eb", "realview-eb-mpcore", "realview-pb-a8",
+ "realview-pbx-a9", "versatileab", "versatilepb", "vexpress-a15",
+ "vexpress-a9", "virt", NULL
+ };
+ add_cdrom_param_tests(aarch64machines);
}
ret = g_test_run();
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] tests/cdrom-test: Test that -cdrom parameter is working
2018-03-15 7:49 ` [Qemu-devel] [PATCH 3/3] tests/cdrom-test: Test that -cdrom parameter is working Thomas Huth
@ 2018-03-15 11:42 ` Philippe Mathieu-Daudé
2018-03-15 19:34 ` Thomas Huth
2018-03-15 11:58 ` Eric Blake
1 sibling, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-03-15 11:42 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Peter Maydell, Victor Kaplansky, qemu-block, Michael S. Tsirkin,
Mark Cave-Ayland, qemu-s390x, Hervé Poussineau
Hi Thomas,
On 03/15/2018 08:49 AM, Thomas Huth wrote:
> Commit 1454509726719e0933c800 recently broke the "-cdrom" parameter
> on a couple of boards without that we noticed it immediately. Thus
> add a test which checks that "-cdrom" can at least be used to start
> QEMU with certain machine types.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> tests/Makefile.include | 7 +++++-
> tests/cdrom-test.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 68 insertions(+), 2 deletions(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index a104222..b744fea 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -321,8 +321,9 @@ check-qtest-microblaze-y = tests/boot-serial-test$(EXESUF)
> check-qtest-mips-y = tests/endianness-test$(EXESUF)
>
> check-qtest-mips64-y = tests/endianness-test$(EXESUF)
> +check-qtest-mips64-y += tests/cdrom-test$(EXESUF)
>
> -check-qtest-mips64el-y = tests/endianness-test$(EXESUF)
> +check-qtest-mips64el-y = $(check-qtest-mips64-y)
>
> check-qtest-moxie-y = tests/boot-serial-test$(EXESUF)
>
> @@ -356,6 +357,7 @@ check-qtest-ppc64-y += tests/display-vga-test$(EXESUF)
> check-qtest-ppc64-y += tests/numa-test$(EXESUF)
> check-qtest-ppc64-$(CONFIG_IVSHMEM) += tests/ivshmem-test$(EXESUF)
> check-qtest-ppc64-y += tests/cpu-plug-test$(EXESUF)
> +check-qtest-ppc64-y += tests/cdrom-test$(EXESUF)
>
> check-qtest-sh4-y = tests/endianness-test$(EXESUF)
>
> @@ -365,10 +367,12 @@ check-qtest-sparc-y = tests/prom-env-test$(EXESUF)
> check-qtest-sparc-y += tests/m48t59-test$(EXESUF)
> gcov-files-sparc-y = hw/timer/m48t59.c
> check-qtest-sparc-y += tests/boot-serial-test$(EXESUF)
> +check-qtest-sparc-y += tests/cdrom-test$(EXESUF)
>
> check-qtest-sparc64-y = tests/endianness-test$(EXESUF)
> check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
> check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF)
> +check-qtest-sparc64-y += tests/cdrom-test$(EXESUF)
>
> check-qtest-arm-y = tests/tmp105-test$(EXESUF)
> check-qtest-arm-y += tests/ds1338-test$(EXESUF)
> @@ -384,6 +388,7 @@ check-qtest-arm-y += tests/sdhci-test$(EXESUF)
> check-qtest-aarch64-y = tests/numa-test$(EXESUF)
> check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
> check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF)
> +check-qtest-aarch64-y += tests/cdrom-test$(EXESUF)
Since you use g_str_equal(arch,), maybe this test can be added
regardless the arch in check-qtest-generic.
>
> check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
>
> diff --git a/tests/cdrom-test.c b/tests/cdrom-test.c
> index 1fc5230..5e516dd 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";
>
> @@ -62,7 +63,13 @@ static int prepare_image(const char *arch, char *isoimage)
> goto cleanup;
> }
> } else {
> - g_assert_not_reached();
> + /* 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;
> + }
> }
>
> ret = gen_iso("-quiet -l -no-emul-boot -b %s -o %s %s",
> @@ -81,6 +88,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;
> @@ -145,6 +178,34 @@ 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", "mipssim", "pica61", NULL
> + };
> + add_cdrom_param_tests(mips64machines);
> + } else if (g_str_equal(arch, "aarch64")) {
> + const char *aarch64machines[] = {
> + "realview-eb", "realview-eb-mpcore", "realview-pb-a8",
> + "realview-pbx-a9", "versatileab", "versatilepb", "vexpress-a15",
> + "vexpress-a9", "virt", NULL
> + };
> + add_cdrom_param_tests(aarch64machines);
> }
>
> ret = g_test_run();
>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] tests/cdrom-test: Test that -cdrom parameter is working
2018-03-15 11:42 ` Philippe Mathieu-Daudé
@ 2018-03-15 19:34 ` Thomas Huth
0 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2018-03-15 19:34 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Peter Maydell, Victor Kaplansky, qemu-block, Michael S. Tsirkin,
Mark Cave-Ayland, qemu-s390x, Hervé Poussineau
On 15.03.2018 12:42, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
>
> On 03/15/2018 08:49 AM, Thomas Huth wrote:
>> Commit 1454509726719e0933c800 recently broke the "-cdrom" parameter
>> on a couple of boards without that we noticed it immediately. Thus
>> add a test which checks that "-cdrom" can at least be used to start
>> QEMU with certain machine types.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> tests/Makefile.include | 7 +++++-
>> tests/cdrom-test.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 68 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index a104222..b744fea 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -321,8 +321,9 @@ check-qtest-microblaze-y = tests/boot-serial-test$(EXESUF)
>> check-qtest-mips-y = tests/endianness-test$(EXESUF)
>>
>> check-qtest-mips64-y = tests/endianness-test$(EXESUF)
>> +check-qtest-mips64-y += tests/cdrom-test$(EXESUF)
>>
>> -check-qtest-mips64el-y = tests/endianness-test$(EXESUF)
>> +check-qtest-mips64el-y = $(check-qtest-mips64-y)
>>
>> check-qtest-moxie-y = tests/boot-serial-test$(EXESUF)
>>
>> @@ -356,6 +357,7 @@ check-qtest-ppc64-y += tests/display-vga-test$(EXESUF)
>> check-qtest-ppc64-y += tests/numa-test$(EXESUF)
>> check-qtest-ppc64-$(CONFIG_IVSHMEM) += tests/ivshmem-test$(EXESUF)
>> check-qtest-ppc64-y += tests/cpu-plug-test$(EXESUF)
>> +check-qtest-ppc64-y += tests/cdrom-test$(EXESUF)
>>
>> check-qtest-sh4-y = tests/endianness-test$(EXESUF)
>>
>> @@ -365,10 +367,12 @@ check-qtest-sparc-y = tests/prom-env-test$(EXESUF)
>> check-qtest-sparc-y += tests/m48t59-test$(EXESUF)
>> gcov-files-sparc-y = hw/timer/m48t59.c
>> check-qtest-sparc-y += tests/boot-serial-test$(EXESUF)
>> +check-qtest-sparc-y += tests/cdrom-test$(EXESUF)
>>
>> check-qtest-sparc64-y = tests/endianness-test$(EXESUF)
>> check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
>> check-qtest-sparc64-y += tests/boot-serial-test$(EXESUF)
>> +check-qtest-sparc64-y += tests/cdrom-test$(EXESUF)
>>
>> check-qtest-arm-y = tests/tmp105-test$(EXESUF)
>> check-qtest-arm-y += tests/ds1338-test$(EXESUF)
>> @@ -384,6 +388,7 @@ check-qtest-arm-y += tests/sdhci-test$(EXESUF)
>> check-qtest-aarch64-y = tests/numa-test$(EXESUF)
>> check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
>> check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF)
>> +check-qtest-aarch64-y += tests/cdrom-test$(EXESUF)
>
> Since you use g_str_equal(arch,), maybe this test can be added
> regardless the arch in check-qtest-generic.
Yes, sounds like a good idea. I think the test can even be run on other
architectures by using the default machine or "-M none" ... I'll give it
a try ...
>> ret = g_test_run();
>>
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Thanks!
Thomas
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] tests/cdrom-test: Test that -cdrom parameter is working
2018-03-15 7:49 ` [Qemu-devel] [PATCH 3/3] tests/cdrom-test: Test that -cdrom parameter is working Thomas Huth
2018-03-15 11:42 ` Philippe Mathieu-Daudé
@ 2018-03-15 11:58 ` Eric Blake
1 sibling, 0 replies; 14+ messages in thread
From: Eric Blake @ 2018-03-15 11:58 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: Peter Maydell, Victor Kaplansky, qemu-block, Michael S. Tsirkin,
Mark Cave-Ayland, qemu-s390x, Hervé Poussineau
On 03/15/2018 02:49 AM, Thomas Huth wrote:
> Commit 1454509726719e0933c800 recently broke the "-cdrom" parameter
> on a couple of boards without that we noticed it immediately. Thus
s/without that we noticed/without us noticing/
> add a test which checks that "-cdrom" can at least be used to start
> QEMU with certain machine types.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> tests/Makefile.include | 7 +++++-
> tests/cdrom-test.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 68 insertions(+), 2 deletions(-)
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 14+ messages in thread