[Libguestfs] [PATCH] v2v: ovf: add firmware and machine type element

[Richard W.M. Jones]

On Thu, Jul 19, 2018 at 04:24:57PM +0200, Pino Toscano wrote:
> On Thursday, 19 July 2018 15:51:04 CEST Tomáš Golembiovský wrote:
> > Add oVirt specific elemnt to OVF. It represents the combination of
> > machine type (i440fx/q35) and firmware (BIOS/UEFI).
> 
> Other than adding a new element in the OVF, this also enables the
> conversion of UEFI guests to oVirt. It is tested? Does it require other
> changes?
> 
> I'd split the addition of <BiosType> to the OVF in an own patch, and
> the enablement of UEFI for oVirt in a different one.
> 
> > +let get_ovirt_biostype guestcaps target_firmware  =
> > +  let uefi_firmware =
> > +    match target_firmware with
> > +    | TargetBIOS -> None
> > +    | TargetUEFI -> Some (find_uefi_firmware guestcaps.gcaps_arch) in
> > +  let secure_boot_required =
> > +    match uefi_firmware with
> > +    | Some { Uefi.flags = flags }
> > +         when List.mem Uefi.UEFI_FLAG_SECURE_BOOT_REQUIRED flags -> true
> > +    | _ -> false in
> > +  match target_firmware, secure_boot_required with
> > +  | TargetUEFI, true -> 3 (* q35 + UEFI + secure boot *)
> > +  | TargetUEFI, _ -> 2 (* q35 + UEFI *)
> > +  (* 1 is q35 + SeaBIOS *)
> > +  | _, _ -> 0 (* i440fx + SeaBIOS *)
> 
> This function seems a bit to convoluted -- what about something like
> (untested):
> 
> let get_ovirt_biostype guestcaps = function
>   | TargetBIOS -> 0  (* i440fx + SeaBIOS *)
>   | TargetUEFI ->
>     let caps = find_uefi_firmware guestcaps.gcaps_arch in
>     if List.mem Uefi.UEFI_FLAG_SECURE_BOOT_REQUIRED caps.flags

These two lines aren't right, because they test if secure boot
is available on the UEFI firmware installed on the conversion
appliance.

In fact I don't think there is a way to detect if secure boot
is required/supported by a guest (probably a missing feature)
so you can just leave out the "3" case entirely.

Rich.

>     then 3  (* q35 + UEFI + secure boot *)
>     else 2  (* q35 + UEFI *)
> 
> Thanks,
> -- 
> Pino Toscano



> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs


-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/