[PATCH v3 03/38] qemu_firmware: Improve matching when loader.type is absent

Andrea Bolognani via Devel posted 38 patches 1 week, 5 days ago
There is a newer version of this series
[PATCH v3 03/38] qemu_firmware: Improve matching when loader.type is absent
Posted by Andrea Bolognani via Devel 1 week, 5 days ago
Right now we don't allow any match in that scenario, but really
if a specific type hasn't been requested it means that any type
is considered acceptable and we should allow matching against
both UEFI and BIOS.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/qemu/qemu_firmware.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 6a074055ca..9f7c981c25 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1104,16 +1104,17 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
          * not present, we can still infer this information from
          * other factors. Specifically, the pflash loader type is
          * only used for UEFI, while the rom loader type can be used
-         * both for UEFI and BIOS */
+         * both for UEFI and BIOS. If the loader type has not been
+         * specified, we keep our options open */
         switch (loader->type) {
         case VIR_DOMAIN_LOADER_TYPE_PFLASH:
             wantUEFI = true;
             break;
         case VIR_DOMAIN_LOADER_TYPE_ROM:
+        case VIR_DOMAIN_LOADER_TYPE_NONE:
             wantUEFI = true;
             wantBIOS = true;
             break;
-        case VIR_DOMAIN_LOADER_TYPE_NONE:
         case VIR_DOMAIN_LOADER_TYPE_LAST:
         default:
             break;
-- 
2.53.0
Re: [PATCH v3 03/38] qemu_firmware: Improve matching when loader.type is absent
Posted by Daniel P. Berrangé via Devel 1 week ago
On Wed, Feb 18, 2026 at 01:05:26PM +0100, Andrea Bolognani via Devel wrote:
> Right now we don't allow any match in that scenario, but really
> if a specific type hasn't been requested it means that any type
> is considered acceptable and we should allow matching against
> both UEFI and BIOS.

AFAICT, there is nothing in the callpath that would limit this to only
VMs of a certain architecture or machine.  This new codepath only works
for architectures which are capable of supporting UEFI and that's only
x86, arm (virt machine), riscv (virt machine).   For any other arch
or machine type we don't want TYPE_NONE to match this way.

AFAICS, requiring an explicit "type" attribute for any firmware file
matching was part of thue mechanism to avoid impacting back compat for
existing XML configurations none of which will have set 'type'.

> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  src/qemu/qemu_firmware.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
> index 6a074055ca..9f7c981c25 100644
> --- a/src/qemu/qemu_firmware.c
> +++ b/src/qemu/qemu_firmware.c
> @@ -1104,16 +1104,17 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
>           * not present, we can still infer this information from
>           * other factors. Specifically, the pflash loader type is
>           * only used for UEFI, while the rom loader type can be used
> -         * both for UEFI and BIOS */
> +         * both for UEFI and BIOS. If the loader type has not been
> +         * specified, we keep our options open */
>          switch (loader->type) {
>          case VIR_DOMAIN_LOADER_TYPE_PFLASH:
>              wantUEFI = true;
>              break;
>          case VIR_DOMAIN_LOADER_TYPE_ROM:
> +        case VIR_DOMAIN_LOADER_TYPE_NONE:
>              wantUEFI = true;
>              wantBIOS = true;
>              break;
> -        case VIR_DOMAIN_LOADER_TYPE_NONE:
>          case VIR_DOMAIN_LOADER_TYPE_LAST:
>          default:
>              break;
> -- 
> 2.53.0
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|
Re: [PATCH v3 03/38] qemu_firmware: Improve matching when loader.type is absent
Posted by Andrea Bolognani via Devel 1 week ago
On Mon, Feb 23, 2026 at 11:47:47AM +0000, Daniel P. Berrangé wrote:
> On Wed, Feb 18, 2026 at 01:05:26PM +0100, Andrea Bolognani via Devel wrote:
> > Right now we don't allow any match in that scenario, but really
> > if a specific type hasn't been requested it means that any type
> > is considered acceptable and we should allow matching against
> > both UEFI and BIOS.
>
> AFAICT, there is nothing in the callpath that would limit this to only
> VMs of a certain architecture or machine.  This new codepath only works
> for architectures which are capable of supporting UEFI and that's only
> x86, arm (virt machine), riscv (virt machine).

Or loongarch64 :)

> For any other arch
> or machine type we don't want TYPE_NONE to match this way.
>
> AFAICS, requiring an explicit "type" attribute for any firmware file
> matching was part of thue mechanism to avoid impacting back compat for
> existing XML configurations none of which will have set 'type'.

Note that this is just *one* of the many criteria that need to be
satisfied before declaring a match.

If there is no JSON firmware descriptor, there will be no match. If
there is no firmware binary that will work for the architecture or
machine type available on the system, there will be no match. If the
domain XML contains something extremely silly such as

  <loader type='pflash'>/usr/share/seabios/bios.bin</loader>

there will be no match. So incorrect matches are not really a
concern.

On the other hand, right now when libvirt is presented with

  <loader>/usr/share/edk2/ovmf/OVMF_CODE.fd</loader>

it is unable to fill in all the additional information that it
already knows about, and that's quite unhelpful.

But your comments made me realize that the way we're matching things
is still incorrect/suboptimal even after these changes. I'll rework
things based on that realization, and we can continue the discussion
once that's done.

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH v3 03/38] qemu_firmware: Improve matching when loader.type is absent
Posted by Andrea Bolognani via Devel 1 week ago
On Mon, Feb 23, 2026 at 11:22:41AM -0500, Andrea Bolognani wrote:
> On Mon, Feb 23, 2026 at 11:47:47AM +0000, Daniel P. Berrangé wrote:
> > On Wed, Feb 18, 2026 at 01:05:26PM +0100, Andrea Bolognani via Devel wrote:
> > > Right now we don't allow any match in that scenario, but really
> > > if a specific type hasn't been requested it means that any type
> > > is considered acceptable and we should allow matching against
> > > both UEFI and BIOS.
> >
> > AFAICT, there is nothing in the callpath that would limit this to only
> > VMs of a certain architecture or machine.  This new codepath only works
> > for architectures which are capable of supporting UEFI and that's only
> > x86, arm (virt machine), riscv (virt machine).
>
> Or loongarch64 :)
>
> > For any other arch
> > or machine type we don't want TYPE_NONE to match this way.
> >
> > AFAICS, requiring an explicit "type" attribute for any firmware file
> > matching was part of thue mechanism to avoid impacting back compat for
> > existing XML configurations none of which will have set 'type'.
>
> Note that this is just *one* of the many criteria that need to be
> satisfied before declaring a match.
>
> If there is no JSON firmware descriptor, there will be no match. If
> there is no firmware binary that will work for the architecture or
> machine type available on the system, there will be no match. If the
> domain XML contains something extremely silly such as
>
>   <loader type='pflash'>/usr/share/seabios/bios.bin</loader>
>
> there will be no match. So incorrect matches are not really a
> concern.
>
> On the other hand, right now when libvirt is presented with
>
>   <loader>/usr/share/edk2/ovmf/OVMF_CODE.fd</loader>
>
> it is unable to fill in all the additional information that it
> already knows about, and that's quite unhelpful.
>
> But your comments made me realize that the way we're matching things
> is still incorrect/suboptimal even after these changes. I'll rework
> things based on that realization, and we can continue the discussion
> once that's done.

This is proving to be more of a challenge than I initially
anticipated.

Since these changes are not really prerequisites for introducing
varstore support and simply ended up in the pile because I made them
while working on that, I will pull them out of the series for now and
come back with a new version once I've had the chance to spend some
more time thinking things through.

-- 
Andrea Bolognani / Red Hat / Virtualization