On 12/20/22 12:07, Laszlo Ersek wrote: > 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 <ler...@redhat.com> >> Signed-off-by: Andrey Drobyshev <andrey.drobys...@virtuozzo.com> >> Originally-by: Denis Plotnikov <dplotni...@virtuozzo.com> >> --- >> 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.)
Oops, my bad. Will send the proper version now. > > Thanks! > Laszlo > _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs