5:42 p.m.
On 02/27/19 11:04, Michal Privoznik wrote:
Test firmware description parsing so far.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/Makefile.am | 9 ++++
tests/qemufirmwaredata/40-bios.json | 35 +++++++++++++
tests/qemufirmwaredata/50-ovmf-sb.json | 36 +++++++++++++
tests/qemufirmwaredata/60-ovmf.json | 35 +++++++++++++
tests/qemufirmwaredata/70-aavmf.json | 35 +++++++++++++
tests/qemufirmwaretest.c | 70 ++++++++++++++++++++++++++
6 files changed, 220 insertions(+)
create mode 100644 tests/qemufirmwaredata/40-bios.json
create mode 100644 tests/qemufirmwaredata/50-ovmf-sb.json
create mode 100644 tests/qemufirmwaredata/60-ovmf.json
create mode 100644 tests/qemufirmwaredata/70-aavmf.json
create mode 100644 tests/qemufirmwaretest.c
If the test data files added in this patch are from the examples in
QEMU's "docs/interop/firmware.json", I suggest stating so in the commit
message, also identifying the QEMU commit that added those examples.
In addition, I wonder if the filenames should carry the double-digit
priority prefixes at once in this patch.
Even if they do, I'm thinking that aavmf shouldn't be ordered (by
priority prefix) behind ovmf, given that the qemu targets / machine
types they are suitable for are mutually exclusive.
Other than that, I have two superficial comments (questions) below,
because my knowledge of the libvirt test infrastructure is nonexistent
to minimal:
diff --git a/tests/Makefile.am b/tests/Makefile.am
index c3f633cee0..d23a8c2812 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -132,6 +132,7 @@ EXTRA_DIST = \
qemuxml2xmloutdata \
qemustatusxml2xmldata \
qemumemlockdata \
+ qemufirmwaredata \
secretxml2xmlin \
securityselinuxhelperdata \
securityselinuxlabeldata \
@@ -291,6 +292,7 @@ test_programs += qemuxml2argvtest qemuxml2xmltest \
qemublocktest \
qemumigparamstest \
qemusecuritytest \
+ qemufirmwaretest \
$(NULL)
test_helpers += qemucapsprobe
test_libraries += libqemumonitortestutils.la \
@@ -698,6 +700,12 @@ qemusecuritytest_SOURCES = \
testutilsqemu.h testutilsqemu.c
qemusecuritytest_LDADD = $(qemu_LDADDS) $(LDADDS)
+qemufirmwaretest_SOURCES = \
+ qemufirmwaretest.c \
+ testutils.h testutils.c \
+ $(NULL)
+qemufirmwaretest_LDADD = $(qemu_LDADDS) $(LDADDS)
+
else ! WITH_QEMU
EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \
domainsnapshotxml2xmltest.c \
@@ -711,6 +719,7 @@ EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c
\
qemumigparamstest.c \
qemusecuritytest.c qemusecuritytest.h \
qemusecuritymock.c \
+ qemufirmwaretest.c \
$(QEMUMONITORTESTUTILS_SOURCES)
endif ! WITH_QEMU
diff --git a/tests/qemufirmwaredata/40-bios.json b/tests/qemufirmwaredata/40-bios.json
new file mode 100644
index 0000000000..137ff70779
--- /dev/null
+++ b/tests/qemufirmwaredata/40-bios.json
@@ -0,0 +1,35 @@
+{
+ "description": "SeaBIOS",
+ "interface-types": [
+ "bios"
+ ],
+ "mapping": {
+ "device": "memory",
+ "filename": "/usr/share/seabios/bios-256k.bin"
+ },
+ "targets": [
+ {
+ "architecture": "i386",
+ "machines": [
+ "pc-i440fx-*",
+ "pc-q35-*"
+ ]
+ },
+ {
+ "architecture": "x86_64",
+ "machines": [
+ "pc-i440fx-*",
+ "pc-q35-*"
+ ]
+ }
+ ],
+ "features": [
+ "acpi-s3",
+ "acpi-s4"
+ ],
+ "tags": [
+ "CONFIG_BOOTSPLASH=n",
+ "CONFIG_ROM_SIZE=256",
+ "CONFIG_USE_SMM=n"
+ ]
+}
diff --git a/tests/qemufirmwaredata/50-ovmf-sb.json
b/tests/qemufirmwaredata/50-ovmf-sb.json
new file mode 100644
index 0000000000..c804ac1038
--- /dev/null
+++ b/tests/qemufirmwaredata/50-ovmf-sb.json
@@ -0,0 +1,36 @@
+{
+ "description": "OVMF with SB+SMM, SB enabled, MS certs
enrolled",
+ "interface-types": [
+ "uefi"
+ ],
+ "mapping": {
+ "device": "flash",
+ "executable": {
+ "filename": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
+ "format": "raw"
+ },
+ "nvram-template": {
+ "filename": "/usr/share/OVMF/OVMF_VARS.secboot.fd",
+ "format": "raw"
+ }
+ },
+ "targets": [
+ {
+ "architecture": "x86_64",
+ "machines": [
+ "pc-q35-*"
+ ]
+ }
+ ],
+ "features": [
+ "acpi-s3",
+ "amd-sev",
+ "enrolled-keys",
+ "requires-smm",
+ "secure-boot",
+ "verbose-dynamic"
+ ],
+ "tags": [
+
+ ]
+}
diff --git a/tests/qemufirmwaredata/60-ovmf.json b/tests/qemufirmwaredata/60-ovmf.json
new file mode 100644
index 0000000000..5e8a94ae78
--- /dev/null
+++ b/tests/qemufirmwaredata/60-ovmf.json
@@ -0,0 +1,35 @@
+{
+ "description": "OVMF with SB+SMM, empty varstore",
+ "interface-types": [
+ "uefi"
+ ],
+ "mapping": {
+ "device": "flash",
+ "executable": {
+ "filename": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
+ "format": "raw"
+ },
+ "nvram-template": {
+ "filename": "/usr/share/OVMF/OVMF_VARS.fd",
+ "format": "raw"
+ }
+ },
+ "targets": [
+ {
+ "architecture": "x86_64",
+ "machines": [
+ "pc-q35-*"
+ ]
+ }
+ ],
+ "features": [
+ "acpi-s3",
+ "amd-sev",
+ "requires-smm",
+ "secure-boot",
+ "verbose-dynamic"
+ ],
+ "tags": [
+
+ ]
+}
diff --git a/tests/qemufirmwaredata/70-aavmf.json b/tests/qemufirmwaredata/70-aavmf.json
new file mode 100644
index 0000000000..114d1475a2
--- /dev/null
+++ b/tests/qemufirmwaredata/70-aavmf.json
@@ -0,0 +1,35 @@
+{
+ "description": "UEFI firmware for ARM64 virtual machines",
+ "interface-types": [
+ "uefi"
+ ],
+ "mapping": {
+ "device": "flash",
+ "executable": {
+ "filename": "/usr/share/AAVMF/AAVMF_CODE.fd",
+ "format": "raw"
+ },
+ "nvram-template": {
+ "filename": "/usr/share/AAVMF/AAVMF_VARS.fd",
+ "format": "raw"
+ }
+ },
+ "targets": [
+ {
+ "architecture": "aarch64",
+ "machines": [
+ "virt-*"
+ ]
+ }
+ ],
+ "features": [
+
+ ],
+ "tags": [
+ "-a AARCH64",
+ "-p ArmVirtPkg/ArmVirtQemu.dsc",
+ "-t GCC48",
+ "-b DEBUG",
+ "-D DEBUG_PRINT_ERROR_LEVEL=0x80000000"
+ ]
+}
diff --git a/tests/qemufirmwaretest.c b/tests/qemufirmwaretest.c
new file mode 100644
index 0000000000..0c5fb1e55a
--- /dev/null
+++ b/tests/qemufirmwaretest.c
@@ -0,0 +1,70 @@
+#include <config.h>
+
+#include "testutils.h"
+#include "qemu/qemu_firmware.h"
+
+#define VIR_FROM_THIS VIR_FROM_QEMU
+
+static int
+testParseFormatFW(const void *opaque)
+{
+ const char *filename = opaque;
+ VIR_AUTOFREE(char *) path = NULL;
+ VIR_AUTOPTR(qemuFirmware) fw = NULL;
+ VIR_AUTOFREE(char *) buf = NULL;
+ VIR_AUTOPTR(virJSONValue) json = NULL;
+ VIR_AUTOFREE(char *) expected = NULL;
+ VIR_AUTOFREE(char *) actual = NULL;
+
+ if (virAsprintf(&path, "%s/qemufirmwaredata/%s",
+ abs_srcdir, filename) < 0)
+ return -1;
+
+ if (!(fw = qemuFirmwareParse(path)))
+ return -1;
+
+ if (virFileReadAll(path,
+ 1024 * 1024, /* 1MiB */
+ &buf) < 0)
+ return -1;
+
+ if (!(json = virJSONValueFromString(buf)))
+ return -1;
+
+ /* Description and tags are not parsed. */
+ if (virJSONValueObjectRemoveKey(json, "description", NULL) < 0 ||
+ virJSONValueObjectRemoveKey(json, "tags", NULL) < 0)
+ return -1;
+
+ if (!(expected = virJSONValueToString(json, true)))
+ return -1;
+
+ if (!(actual = qemuFirmwareFormat(fw)))
+ return -1;
+
+ return virTestCompareToString(expected, actual);
+}
Can you add a comment to the function about the general idea? Or is this
approach common for libvirt tests? I mean, to make heads or tails of
this function, I have to decipher what each step does, and what the
final comparison compares. It would be easier to read if you explained
the steps in a comment, and then we'd only have to verify the steps
against that documentation.
My other question: when does virJSONValueObjectRemoveKey() fail? I
assume it doesn't fail when the key isn't present.
Thanks,
Laszlo
+
+
+static int
+mymain(void)
+{
+ int ret = 0;
+
+#define DO_PARSE_TEST(filename) \
+ do { \
+ if (virTestRun("QEMU FW " filename, \
+ testParseFormatFW, filename) < 0) \
+ ret = -1; \
+ } while (0)
+
+ DO_PARSE_TEST("40-bios.json");
+ DO_PARSE_TEST("50-ovmf-sb.json");
+ DO_PARSE_TEST("60-ovmf.json");
+ DO_PARSE_TEST("70-aavmf.json");
+
+ return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+
+VIR_TEST_MAIN(mymain);