Re: [libvirt] [PATCH v2 08/15] test: Introduce qemufirmwaretest

On Thu, Mar 07, 2019 at 10:29:18AM +0100, Michal Privoznik wrote: > Test firmware description parsing so far. > > The test files come from three locations: > 1) ovmf-sb-keys.json and ovmf-sb.json come from OVMF > package from RHEL-7 (with slight name change to reflect their > features in filename too), > > 2) bios.json and aavmf.json come form comments from s/form/from/ > firmware.json from qemu's git (3a0adfc9bf), > > 3) ovmf.json is then copied from ovmf-sb.json and stripped > of SECURE_BOOT and REQUIRES_SMM flags (plus OVMF path change). > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > tests/Makefile.am | 9 +++ > tests/qemufirmwaredata/aavmf.json | 35 +++++++++++ > tests/qemufirmwaredata/bios.json | 35 +++++++++++ > tests/qemufirmwaredata/ovmf-sb-keys.json | 36 ++++++++++++ > tests/qemufirmwaredata/ovmf-sb.json | 35 +++++++++++ > tests/qemufirmwaredata/ovmf.json | 33 +++++++++++ > tests/qemufirmwaretest.c | 75 ++++++++++++++++++++++++ > 7 files changed, 258 insertions(+) > create mode 100644 tests/qemufirmwaredata/aavmf.json > create mode 100644 tests/qemufirmwaredata/bios.json > create mode 100644 tests/qemufirmwaredata/ovmf-sb-keys.json > create mode 100644 tests/qemufirmwaredata/ovmf-sb.json > create mode 100644 tests/qemufirmwaredata/ovmf.json > create mode 100644 tests/qemufirmwaretest.c [snip] > +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) Any reason why you didn't parse them. Feels like it would have been easy enough to parse those 2 fields & avoid the special case ?

> + return -1; > + > + if (!(expected = virJSONValueToString(json, true))) > + return -1; > + > + if (!(actual = qemuFirmwareFormat(fw))) > + return -1; > + > + return virTestCompareToString(expected, actual); > +} Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt;