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 <th...@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 <th...@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 :|