According to [1], there're different ways to specify which firmware is to be used by a libvirt-driven VM. Namely, there's an automatic firmware selection, e.g.: ... <os firmware='(bios|efi)'> ... and a manual one, e.g.: ... <os> <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> ... </os> ... with the latter being a way to specify UEFI firmware. So let's add this search path as well when parsing source VM's libvirt xml. [1] https://libvirt.org/formatdomain.html#bios-bootloader Co-authored-by: Laszlo Ersek <lersek(a)redhat.com&gt; Signed-off-by: Andrey Drobyshev <andrey.drobyshev(a)virtuozzo.com&gt; Originally-by: Denis Plotnikov <dplotnikov(a)virtuozzo.com&gt; --- input/parse_libvirt_xml.ml | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/input/parse_libvirt_xml.ml b/input/parse_libvirt_xml.ml index 56ce1c22..ab72c0ce 100644 --- a/input/parse_libvirt_xml.ml +++ b/input/parse_libvirt_xml.ml @@ -446,12 +446,23 @@ let parse_libvirt_xml ?conn xml = done; List.rev !nics in - (* Firmware. *) + (* Firmware. + * If "/domain/os" node doesn't contain "firmware" attribute (automatic + * firmware), we look for the presence of "pflash" in + * "/domain/os/loader/@type" attribute (manual firmware), with the latter + * indicating the UEFI firmware. + * See https://libvirt.org/formatdomain.html#bios-bootloader + *) let firmware = match xpath_string "/domain/os/@firmware" with | Some "bios" -> BIOS | Some "efi" -> UEFI - | None | Some _ -> UnknownFirmware in + | Some _ -> UnknownFirmware + | None -> ( + match xpath_string "/domain/os/loader/@type" with + | Some "pflash" -> UEFI + | _ -> UnknownFirmware + ) in (* Fallback to BIOS if we haven't found explicitly specified firmware. * This is VZ-specific since we're either using "/domain/os/loader" node

Hi Andrey, On 12/19/22 19:59, Andrey Drobyshev wrote: > According to [1], there're different ways to specify which firmware is > to be used by a libvirt-driven VM. Namely, there's an automatic > firmware selection, e.g.: > > ... > <os firmware='(bios|efi)'> > ... > > and a manual one, e.g.: > > ... > <os> > <loader readonly='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> > ... > </os> > ... > > with the latter being a way to specify UEFI firmware. So let's add this > search path as well when parsing source VM's libvirt xml. > > [1] https://libvirt.org/formatdomain.html#bios-bootloader > > Co-authored-by: Laszlo Ersek <lersek(a)redhat.com&gt; > Signed-off-by: Andrey Drobyshev <andrey.drobyshev(a)virtuozzo.com&gt; > Originally-by: Denis Plotnikov <dplotnikov(a)virtuozzo.com&gt; > --- > input/parse_libvirt_xml.ml | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/input/parse_libvirt_xml.ml b/input/parse_libvirt_xml.ml > index 56ce1c22..ab72c0ce 100644 > --- a/input/parse_libvirt_xml.ml > +++ b/input/parse_libvirt_xml.ml > @@ -446,12 +446,23 @@ let parse_libvirt_xml ?conn xml = > done; > List.rev !nics in > > - (* Firmware. *) > + (* Firmware. > + * If "/domain/os" node doesn't contain "firmware" attribute (automatic > + * firmware), we look for the presence of "pflash" in > + * "/domain/os/loader/@type" attribute (manual firmware), with the latter > + * indicating the UEFI firmware. > + * See https://libvirt.org/formatdomain.html#bios-bootloader > + *) > let firmware = > match xpath_string "/domain/os/@firmware" with > | Some "bios" -> BIOS > | Some "efi" -> UEFI > - | None | Some _ -> UnknownFirmware in > + | Some _ -> UnknownFirmware > + | None -> ( > + match xpath_string "/domain/os/loader/@type" with > + | Some "pflash" -> UEFI > + | _ -> UnknownFirmware > + ) in > > (* Fallback to BIOS if we haven't found explicitly specified firmware. > * This is VZ-specific since we're either using "/domain/os/loader" node I'm OK with this patch. The only reason I can't give R-b for it is that you noted me as co-author on the patch, and I can't review my own (or co-authored) patches. But that's not a problem; I actually tried to apply (and then push) this patch, without any R-b's (with Rich being on PTO). However, the patch does not apply to master @ 1c8ff404582f. The conflict is in the trailing context of the patch: the trailing comment in v4 introduces a VZ-specific code section, whereas on the master branch, we have: (* Check for hostdev devices. (RHBZ#1472719) *) let () = Can you please rebase to the master branch and repost? (Quickly checking versions 1 through 3 of the patch, those were all based on the master branch; I think it is only in v4 where you have based the patch on a downstream-only branch. That's totally fine of course, but please send the upstream version to the list.)

Thanks! Laszlo