On Sat, 2019-07-13 at 09:29 +0200, Michal Privoznik wrote:
> When validating a domain among all the checks there are two that
> concern VIR_DOMAIN_LOADER_TYPE_PFLASH specifically. The first
> check ensures that on x86 ACPI is enabled when UEFI is requested,
> the second ensures that UEFI is used when ACPI is requested on
> aarch64. However, check for UEFI is done by plain comparison of
> def->os.loader->type which is insufficient because we have
> def->os.firmware too.
> 
> NB, this wouldn't be a problem for active domain, because on
> startup process def->os.loader->type gets filled by
> qemuFirmwareEnableFeatures(), but that's not the case for
> inactive domains.
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1729604
> 
> Signed-off-by: Michal Privoznik <[email protected]>
> ---
>  src/qemu/qemu_domain.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

The changes look good, but can you please at the same time add the
<acpi/> element under <features/> in
tests/qemuxml2argvdata/aarch64-os-firmware-efi.xml so that we gain
coverage for it?

With the above taken care of,

  Reviewed-by: Andrea Bolognani <[email protected]>

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to