| Message ID | 1521100145-15304-3-git-send-email-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add new CD-ROM related qtests | expand |
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
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
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 ;)
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; +}
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