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
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 :|
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 ;)
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
© 2016 - 2025 Red Hat, Inc.