[Libguestfs] [V2V PATCH v4] parse_libvirt_xml: look for manual firmware in "/domain/os/loader/@type"

[Andrey Drobyshev]

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 <lersek at redhat.com>
>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev at virtuozzo.com>
>> Originally-by: Denis Plotnikov <dplotnikov at 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
>