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

Add oVirt specific elemnt to OVF. It represents the combination of
machine type (i440fx/q35) and firmware (BIOS/UEFI).

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 v2v/create_ovf.ml                        | 20 +++++++++++++++++++-
 v2v/create_ovf.mli                       |  2 +-
 v2v/output_rhv.ml                        |  6 ++----
 v2v/output_rhv_upload.ml                 |  4 ++--
 v2v/output_vdsm.ml                       |  6 ++----
 v2v/test-v2v-o-rhv.ovf.expected          |  1 +
 v2v/test-v2v-o-vdsm-options.ovf.expected |  1 +
 7 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/v2v/create_ovf.ml b/v2v/create_ovf.ml
index 2cf610333..1cab11dfd 100644
--- a/v2v/create_ovf.ml
+++ b/v2v/create_ovf.ml
@@ -462,6 +462,22 @@ let origin_of_source_hypervisor = function
    *)
   | _ -> None
 
+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 *)
+
 (* Generate the .meta file associated with each volume. *)
 let create_meta_files output_alloc sd_uuid image_uuids targets    (* Note:
Upper case in the .meta, mixed case in the OVF. *)
@@ -506,7 +522,7 @@ let create_meta_files output_alloc sd_uuid image_uuids
targets    ) (List.combine targets image_uuids)
 
 (* Create the OVF file. *)
-let rec create_ovf source targets guestcaps inspect
+let rec create_ovf source targets guestcaps inspect target_firmware
     output_alloc sd_uuid image_uuids vol_uuids vm_uuid ovf_flavour    assert
(List.length targets = List.length vol_uuids);
 
@@ -515,6 +531,7 @@ let rec create_ovf source targets guestcaps inspect
   let vmtype = get_vmtype inspect in
   let vmtype = match vmtype with `Desktop -> "0" | `Server ->
"1" in
   let ostype = get_ostype inspect in
+  let biostype = get_ovirt_biostype guestcaps target_firmware in
 
   let ovf : doc      doc "ovf:Envelope" [
@@ -562,6 +579,7 @@ let rec create_ovf source targets guestcaps inspect
         e "VmType" [] [PCData vmtype];
         (* See https://bugzilla.redhat.com/show_bug.cgi?id=1260590#c17 *)
         e "DefaultDisplayType" [] [PCData "1"];
+        e "BiosType" [] [PCData (string_of_int biostype)];
       ] in
 
       (match source.s_cpu_model with
diff --git a/v2v/create_ovf.mli b/v2v/create_ovf.mli
index 8200b76f9..cb6c12690 100644
--- a/v2v/create_ovf.mli
+++ b/v2v/create_ovf.mli
@@ -43,7 +43,7 @@ val ovf_flavour_to_string : ovf_flavour -> string
     create OVF for another target management system then we would need
     to heavily modify or even duplicate this code. *)
 
-val create_ovf : Types.source -> Types.target list -> Types.guestcaps
-> Types.inspect -> Types.output_allocation -> string -> string list
-> string list -> string ->  ovf_flavour -> DOM.doc
+val create_ovf : Types.source -> Types.target list -> Types.guestcaps
-> Types.inspect -> Types.target_firmware  -> Types.output_allocation
-> string -> string list -> string list -> string ->  ovf_flavour
-> DOM.doc
 (** Create the OVF file.
 
     Actually a {!DOM} document is created, not a file.  It can be written
diff --git a/v2v/output_rhv.ml b/v2v/output_rhv.ml
index 5260ab030..ce57f0309 100644
--- a/v2v/output_rhv.ml
+++ b/v2v/output_rhv.ml
@@ -115,7 +115,7 @@ object
 
   method as_options = sprintf "-o rhv -os %s" os
 
-  method supported_firmware = [ TargetBIOS ]
+  method supported_firmware = [ TargetBIOS; TargetUEFI ]
 
   (* RHV doesn't support serial consoles.  This causes the conversion
    * step to remove it.
@@ -270,12 +270,10 @@ object
 
   (* This is called after conversion to write the OVF metadata. *)
   method create_metadata source targets _ guestcaps inspect target_firmware -  
(* See #supported_firmware above. *)
-    assert (target_firmware = TargetBIOS);
 
     (* Create the metadata. *)
     let ovf = Create_ovf.create_ovf source targets guestcaps inspect
-      output_alloc esd_uuid image_uuids vol_uuids vm_uuid
+      target_firmware output_alloc esd_uuid image_uuids vol_uuids vm_uuid
       Create_ovf.RHVExportStorageDomain in
 
     (* Write it to the metadata file. *)
diff --git a/v2v/output_rhv_upload.ml b/v2v/output_rhv_upload.ml
index 63fa2411a..3dcd6d952 100644
--- a/v2v/output_rhv_upload.ml
+++ b/v2v/output_rhv_upload.ml
@@ -253,7 +253,7 @@ object
     sprintf " -oc %s -op %s -os %s"
             output_conn output_password output_storage
 
-  method supported_firmware = [ TargetBIOS ]
+  method supported_firmware = [ TargetBIOS; TargetUEFI ]
 
   (* rhev-apt.exe will be installed (if available). *)
   method install_rhev_apt = true
@@ -407,7 +407,7 @@ If the messages above are not sufficient to diagnose the
problem then add the
     (* Create the metadata. *)
     let ovf        Create_ovf.create_ovf source targets guestcaps inspect
-                            output_alloc
+                            target_firmware output_alloc
                             sd_uuid image_uuids vol_uuids vm_uuid
                             OVirt in
     let ovf = DOM.doc_to_string ovf in
diff --git a/v2v/output_vdsm.ml b/v2v/output_vdsm.ml
index 9a1b748bc..f32acedae 100644
--- a/v2v/output_vdsm.ml
+++ b/v2v/output_vdsm.ml
@@ -123,7 +123,7 @@ object
        | flav -> sprintf "-oo vdsm-ovf-flavour=%s"
                          (Create_ovf.ovf_flavour_to_string flav))
 
-  method supported_firmware = [ TargetBIOS ]
+  method supported_firmware = [ TargetBIOS; TargetUEFI ]
 
   (* RHV doesn't support serial consoles.  This causes the conversion
    * step to remove it.
@@ -240,11 +240,9 @@ object
 
   (* This is called after conversion to write the OVF metadata. *)
   method create_metadata source targets _ guestcaps inspect target_firmware -  
(* See #supported_firmware above. *)
-    assert (target_firmware = TargetBIOS);
-
     (* Create the metadata. *)
     let ovf = Create_ovf.create_ovf source targets guestcaps inspect
+      target_firmware
       output_alloc dd_uuid
       vdsm_options.image_uuids
       vdsm_options.vol_uuids
diff --git a/v2v/test-v2v-o-rhv.ovf.expected b/v2v/test-v2v-o-rhv.ovf.expected
index 7bcc456c5..c6bd05c56 100644
--- a/v2v/test-v2v-o-rhv.ovf.expected
+++ b/v2v/test-v2v-o-rhv.ovf.expected
@@ -25,6 +25,7 @@
     <IsStateless>False</IsStateless>
     <VmType>0</VmType>
     <DefaultDisplayType>1</DefaultDisplayType>
+    <BiosType>0</BiosType>
     <Section ovf:id='#VM_ID#' ovf:required='false'
xsi:type='ovf:OperatingSystemSection_Type'>
       <Info>Microsoft Windows 7 Phony Edition</Info>
       <Description>Windows7</Description>
diff --git a/v2v/test-v2v-o-vdsm-options.ovf.expected
b/v2v/test-v2v-o-vdsm-options.ovf.expected
index abaf37e54..f0d418b46 100644
--- a/v2v/test-v2v-o-vdsm-options.ovf.expected
+++ b/v2v/test-v2v-o-vdsm-options.ovf.expected
@@ -25,6 +25,7 @@
     <IsStateless>False</IsStateless>
     <VmType>0</VmType>
     <DefaultDisplayType>1</DefaultDisplayType>
+    <BiosType>0</BiosType>
     <OperatingSystemSection ovf:id='VM' ovf:required='false'
ovirt:id='11'>
       <Info>Microsoft Windows 7 Phony Edition</Info>
       <Description>Windows7</Description>
-- 
2.18.0

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
    then 3  (* q35 + UEFI + secure boot *)
    else 2  (* q35 + UEFI *)

Thanks,
-- 
Pino Toscano

On Thu, Jul 19, 2018 at 03:51:04PM +0200, Tomáš Golembiovský
wrote:
> +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 *)
I think it would be better as well as simpler if this change depended
on this (not upstream) patch:

  https://www.redhat.com/archives/libguestfs/2018-June/thread.html#00085

Unfortunately that patch can't go upstream yet for a couple of reasons:

(1) It breaks a test.

(2) We've decided to get the data from osinfo instead of pivoting on a
particular year, so the patch needs to be reworked for that.

Maybe we can split that patch into the Types module change and the
other stuff.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

On Thu, Jul 19, 2018 at 03:51:04PM +0200, Tomáš Golembiovský
wrote:
> Add oVirt specific elemnt to OVF. It represents the combination of
> machine type (i440fx/q35) and firmware (BIOS/UEFI).
> 
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
>  v2v/create_ovf.ml                        | 20 +++++++++++++++++++-
>  v2v/create_ovf.mli                       |  2 +-
>  v2v/output_rhv.ml                        |  6 ++----
>  v2v/output_rhv_upload.ml                 |  4 ++--
>  v2v/output_vdsm.ml                       |  6 ++----
>  v2v/test-v2v-o-rhv.ovf.expected          |  1 +
>  v2v/test-v2v-o-vdsm-options.ovf.expected |  1 +
>  7 files changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/v2v/create_ovf.ml b/v2v/create_ovf.ml
> index 2cf610333..1cab11dfd 100644
> --- a/v2v/create_ovf.ml
> +++ b/v2v/create_ovf.ml
> @@ -462,6 +462,22 @@ let origin_of_source_hypervisor = function
>     *)
>    | _ -> None
>  
> +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 *)
> +
>  (* Generate the .meta file associated with each volume. *)
>  let create_meta_files output_alloc sd_uuid image_uuids targets >    (*
Note: Upper case in the .meta, mixed case in the OVF. *)
> @@ -506,7 +522,7 @@ let create_meta_files output_alloc sd_uuid image_uuids
targets >    ) (List.combine targets image_uuids)
>  
>  (* Create the OVF file. *)
> -let rec create_ovf source targets guestcaps inspect
> +let rec create_ovf source targets guestcaps inspect target_firmware
>      output_alloc sd_uuid image_uuids vol_uuids vm_uuid ovf_flavour >   
assert (List.length targets = List.length vol_uuids);
>  
> @@ -515,6 +531,7 @@ let rec create_ovf source targets guestcaps inspect
>    let vmtype = get_vmtype inspect in
>    let vmtype = match vmtype with `Desktop -> "0" | `Server
-> "1" in
>    let ostype = get_ostype inspect in
> +  let biostype = get_ovirt_biostype guestcaps target_firmware in
>  
>    let ovf : doc >      doc "ovf:Envelope" [
> @@ -562,6 +579,7 @@ let rec create_ovf source targets guestcaps inspect
>          e "VmType" [] [PCData vmtype];
>          (* See https://bugzilla.redhat.com/show_bug.cgi?id=1260590#c17 *)
>          e "DefaultDisplayType" [] [PCData "1"];
> +        e "BiosType" [] [PCData (string_of_int biostype)];
>        ] in
>  
>        (match source.s_cpu_model with
> diff --git a/v2v/create_ovf.mli b/v2v/create_ovf.mli
> index 8200b76f9..cb6c12690 100644
> --- a/v2v/create_ovf.mli
> +++ b/v2v/create_ovf.mli
> @@ -43,7 +43,7 @@ val ovf_flavour_to_string : ovf_flavour -> string
>      create OVF for another target management system then we would need
>      to heavily modify or even duplicate this code. *)
>  
> -val create_ovf : Types.source -> Types.target list ->
Types.guestcaps -> Types.inspect -> Types.output_allocation -> string
-> string list -> string list -> string ->  ovf_flavour ->
DOM.doc
> +val create_ovf : Types.source -> Types.target list ->
Types.guestcaps -> Types.inspect -> Types.target_firmware  ->
Types.output_allocation -> string -> string list -> string list ->
string ->  ovf_flavour -> DOM.doc
>  (** Create the OVF file.
>  
>      Actually a {!DOM} document is created, not a file.  It can be written
> diff --git a/v2v/output_rhv.ml b/v2v/output_rhv.ml
> index 5260ab030..ce57f0309 100644
> --- a/v2v/output_rhv.ml
> +++ b/v2v/output_rhv.ml
> @@ -115,7 +115,7 @@ object
>  
>    method as_options = sprintf "-o rhv -os %s" os
>  
> -  method supported_firmware = [ TargetBIOS ]
> +  method supported_firmware = [ TargetBIOS; TargetUEFI ]
>  
>    (* RHV doesn't support serial consoles.  This causes the conversion
>     * step to remove it.
> @@ -270,12 +270,10 @@ object
>  
>    (* This is called after conversion to write the OVF metadata. *)
>    method create_metadata source targets _ guestcaps inspect
target_firmware > -    (* See #supported_firmware above. *)
> -    assert (target_firmware = TargetBIOS);
>  
>      (* Create the metadata. *)
>      let ovf = Create_ovf.create_ovf source targets guestcaps inspect
> -      output_alloc esd_uuid image_uuids vol_uuids vm_uuid
> +      target_firmware output_alloc esd_uuid image_uuids vol_uuids vm_uuid
>        Create_ovf.RHVExportStorageDomain in
>  
>      (* Write it to the metadata file. *)
> diff --git a/v2v/output_rhv_upload.ml b/v2v/output_rhv_upload.ml
> index 63fa2411a..3dcd6d952 100644
> --- a/v2v/output_rhv_upload.ml
> +++ b/v2v/output_rhv_upload.ml
> @@ -253,7 +253,7 @@ object
>      sprintf " -oc %s -op %s -os %s"
>              output_conn output_password output_storage
>  
> -  method supported_firmware = [ TargetBIOS ]
> +  method supported_firmware = [ TargetBIOS; TargetUEFI ]
>  
>    (* rhev-apt.exe will be installed (if available). *)
>    method install_rhev_apt = true
> @@ -407,7 +407,7 @@ If the messages above are not sufficient to diagnose
the problem then add the
>      (* Create the metadata. *)
>      let ovf >        Create_ovf.create_ovf source targets guestcaps
inspect
> -                            output_alloc
> +                            target_firmware output_alloc
>                              sd_uuid image_uuids vol_uuids vm_uuid
>                              OVirt in
>      let ovf = DOM.doc_to_string ovf in
> diff --git a/v2v/output_vdsm.ml b/v2v/output_vdsm.ml
> index 9a1b748bc..f32acedae 100644
> --- a/v2v/output_vdsm.ml
> +++ b/v2v/output_vdsm.ml
> @@ -123,7 +123,7 @@ object
>         | flav -> sprintf "-oo vdsm-ovf-flavour=%s"
>                           (Create_ovf.ovf_flavour_to_string flav))
>  
> -  method supported_firmware = [ TargetBIOS ]
> +  method supported_firmware = [ TargetBIOS; TargetUEFI ]
>  
>    (* RHV doesn't support serial consoles.  This causes the conversion
>     * step to remove it.
> @@ -240,11 +240,9 @@ object
>  
>    (* This is called after conversion to write the OVF metadata. *)
>    method create_metadata source targets _ guestcaps inspect
target_firmware > -    (* See #supported_firmware above. *)
> -    assert (target_firmware = TargetBIOS);
> -
>      (* Create the metadata. *)
>      let ovf = Create_ovf.create_ovf source targets guestcaps inspect
> +      target_firmware
>        output_alloc dd_uuid
>        vdsm_options.image_uuids
>        vdsm_options.vol_uuids
> diff --git a/v2v/test-v2v-o-rhv.ovf.expected
b/v2v/test-v2v-o-rhv.ovf.expected
> index 7bcc456c5..c6bd05c56 100644
> --- a/v2v/test-v2v-o-rhv.ovf.expected
> +++ b/v2v/test-v2v-o-rhv.ovf.expected
> @@ -25,6 +25,7 @@
>      <IsStateless>False</IsStateless>
>      <VmType>0</VmType>
>      <DefaultDisplayType>1</DefaultDisplayType>
> +    <BiosType>0</BiosType>
>      <Section ovf:id='#VM_ID#' ovf:required='false'
xsi:type='ovf:OperatingSystemSection_Type'>
>        <Info>Microsoft Windows 7 Phony Edition</Info>
>        <Description>Windows7</Description>
> diff --git a/v2v/test-v2v-o-vdsm-options.ovf.expected
b/v2v/test-v2v-o-vdsm-options.ovf.expected
> index abaf37e54..f0d418b46 100644
> --- a/v2v/test-v2v-o-vdsm-options.ovf.expected
> +++ b/v2v/test-v2v-o-vdsm-options.ovf.expected
> @@ -25,6 +25,7 @@
>      <IsStateless>False</IsStateless>
>      <VmType>0</VmType>
>      <DefaultDisplayType>1</DefaultDisplayType>
> +    <BiosType>0</BiosType>
>      <OperatingSystemSection ovf:id='VM'
ovf:required='false' ovirt:id='11'>
>        <Info>Microsoft Windows 7 Phony Edition</Info>
>        <Description>Windows7</Description>
Could you post a second version of this patch with the changes
that Pino suggested here:

https://www.redhat.com/archives/libguestfs/2018-July/msg00047.html

Rich.

-- 
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/

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@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/

On Thu, 19 Jul 2018 16:24:57 +0200
Pino Toscano <ptoscano@redhat.com> 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?
At the time of writing I didn't test it as I was not sure how to create
q35 machine in VMware. I can give it another try one of these days.
> Does it require other changes?
AFAIK no

    Tomas
> 
> 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
>     then 3  (* q35 + UEFI + secure boot *)
>     else 2  (* q35 + UEFI *)
> 
> Thanks,
> -- 
> Pino Toscano

-- 
Tomáš Golembiovský <tgolembi@redhat.com>