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

[Michal Privoznik]

On 3/11/19 4:08 PM, Daniel P. Berrangé wrote:
> 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 at redhat.com>
>> ---
>>   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 ?

We don't need them for anything and firmware.json says that:

Management software may or may not display @description.

@tags serves development and debugging purposes only, and management 
software shall explicitly ignore it.

But I can save that for a follow up patch, if we want to parse them.

> 
>> +        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 at redhat.com>
> 

Thanks,
Michal