Libguestfs - Jan 2022 - [v2v PATCH 6/7] convert/libosinfo_utils: extract "best_driver" from "windows_virtio.ml"

The "copy_from_libosinfo" function in "windows_virtio.ml"
filters and
sorts the driver list from libosinfo in order to find the best driver.
Move this logic to a separate function (called "best_driver") in the
Libosinfo_utils module.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2043333
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 convert/libosinfo_utils.mli | 12 ++++++++++
 convert/libosinfo_utils.ml  | 35 +++++++++++++++++++++++++++++
 convert/windows_virtio.ml   | 44 +------------------------------------
 3 files changed, 48 insertions(+), 43 deletions(-)

diff --git a/convert/libosinfo_utils.mli b/convert/libosinfo_utils.mli
index 67be16c491a4..14991bc2d782 100644
--- a/convert/libosinfo_utils.mli
+++ b/convert/libosinfo_utils.mli
@@ -28,12 +28,24 @@ val get_os_by_short_id : string -> Libosinfo.osinfo_os
 val string_of_osinfo_device_list : Libosinfo.osinfo_device list -> string
 (** Convert an [osinfo_device] list to a printable string for debugging. *)
 
 val string_of_osinfo_device_driver : Libosinfo.osinfo_device_driver ->
string
 (** Convert a [osinfo_device_driver] to a printable string for debugging. *)
 
+val best_driver : Libosinfo.osinfo_device_driver list ->
+                  string ->
+                  Libosinfo.osinfo_device_driver
+(** [best_driver drivers arch] picks the best driver from [drivers] as follows:
+    - filters out drivers that:
+      - target a different architecture,
+      - are not pre-installable,
+      - have an invalid or non-local URL;
+    - sorts the remaining drivers by priority, like libosinfo does;
+    - picks the top driver of the sorted list.
+    Raises Not_found if no driver in [drivers] survives filtering. *)
+
 type os_support = {
   q35 : bool;
   vio10 : bool;
 }
 (** Tell whether the operating system supports the Q35 board type and/or
     non-transitional (virtio-1.0-only) virtio devices. (Internally, the
diff --git a/convert/libosinfo_utils.ml b/convert/libosinfo_utils.ml
index f0d70ffd4997..8504e2b2f001 100644
--- a/convert/libosinfo_utils.ml
+++ b/convert/libosinfo_utils.ml
@@ -76,12 +76,47 @@ let string_of_osinfo_device_driver { Libosinfo.architecture;
location;
     (if pre_installable then "pre-installable" else "not
pre-installable")
     (if signed then "signed" else "unsigned")
     priority
     (String.concat "\n" files)
     (string_of_osinfo_device_list devices)
 
+let best_driver drivers arch +  let debug_drivers +    List.iter (fun d ->
debug "Driver: %s" (string_of_osinfo_device_driver d))
+  (* The architecture that "inspect.i_arch" from libguestfs
+   * ("daemon/filearch.ml") calls "i386", the osinfo-db
schema
+   * ("data/schema/osinfo.rng.in") calls "i686".
+   *)
+  and arch = if arch = "i386" then "i686" else arch in
+  debug "libosinfo drivers before filtering:";
+  debug_drivers drivers;
+  let drivers +    List.filter (
+      fun { Libosinfo.architecture; location; pre_installable } ->
+        if architecture <> arch || not pre_installable then
+          false
+        else
+          try
+            (match Xml.parse_uri location with
+            | { Xml.uri_scheme = Some scheme;
+                Xml.uri_path = Some _ } when scheme = "file" ->
true
+            | _ -> false
+            )
+          with Invalid_argument _ -> false
+    ) drivers in
+  debug "libosinfo drivers after filtering:";
+  debug_drivers drivers;
+  let drivers +    List.sort (
+      fun { Libosinfo.priority = prioA } { Libosinfo.priority = prioB } ->
+        compare prioB prioA
+    ) drivers in
+  if drivers = [] then
+    raise Not_found;
+  List.hd drivers
+
 type os_support = {
   q35 : bool;
   vio10 : bool;
 }
 
 let os_support_of_osinfo_device_list diff --git a/convert/windows_virtio.ml
b/convert/windows_virtio.ml
index 610a56857c7e..5254322c7d28 100644
--- a/convert/windows_virtio.ml
+++ b/convert/windows_virtio.ml
@@ -405,59 +405,17 @@ and virtio_iso_path_matches_qemu_ga path inspect   *
  * Files that do not exist are silently skipped.
  *
  * Returns list of copied files.
  *)
 and copy_from_libosinfo g inspect destdir -  let debug_drivers -    List.iter (
-      fun d ->
-        debug "Driver: %s"
(Libosinfo_utils.string_of_osinfo_device_driver d)
-    )
-  in
   let { i_osinfo = osinfo; i_arch = arch } = inspect in
-  (* The architecture that "inspect.i_arch" from libguestfs
-   * ("daemon/filearch.ml") calls "i386", the osinfo-db
schema
-   * ("data/schema/osinfo.rng.in") calls "i686".
-   *)
-  let arch = if arch = "i386" then "i686" else arch in
   try
     let os = Libosinfo_utils.get_os_by_short_id osinfo in
     let drivers = os#get_device_drivers () in
-    debug "libosinfo drivers before filtering:"; debug_drivers
drivers;
-    (*
-     * Filter out drivers that we cannot use:
-     * - for a different architecture
-     * - non-pre-installable ones
-     * - location is an invalid URL, or a non-local one
-     *)
-    let drivers -      List.filter (
-        fun { Libosinfo.architecture; location; pre_installable } ->
-          if architecture <> arch || not pre_installable then
-            false
-          else
-            try
-              (match Xml.parse_uri location with
-              | { Xml.uri_scheme = Some scheme;
-                  Xml.uri_path = Some _ } when scheme = "file" ->
true
-              | _ -> false
-              )
-            with Invalid_argument _ -> false
-      ) drivers in
-    debug "libosinfo drivers after filtering:"; debug_drivers
drivers;
-    (* Sort the drivers by priority, like libosinfo does. *)
-    let drivers -      List.sort (
-        fun { Libosinfo.priority = prioA } { Libosinfo.priority = prioB } ->
-          compare prioB prioA
-      ) drivers in
-    (* Any driver available? *)
-    if drivers = [] then
-      raise Not_found;
-    let driver = List.hd drivers in
+    let driver = Libosinfo_utils.best_driver drivers arch in
     let uri = Xml.parse_uri driver.Libosinfo.location in
     let basedir        match uri.Xml.uri_path with
       | Some p -> p
       | None -> assert false in
     List.filter_map (
-- 
2.19.1.3.g30247aa5d201

On Fri, Jan 28, 2022 at 05:12:17PM +0100, Laszlo Ersek
wrote:
> The "copy_from_libosinfo" function in
"windows_virtio.ml" filters and
> sorts the driver list from libosinfo in order to find the best driver.
> Move this logic to a separate function (called "best_driver") in
the
> Libosinfo_utils module.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2043333
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
>  convert/libosinfo_utils.mli | 12 ++++++++++
>  convert/libosinfo_utils.ml  | 35 +++++++++++++++++++++++++++++
>  convert/windows_virtio.ml   | 44 +------------------------------------
>  3 files changed, 48 insertions(+), 43 deletions(-)
> 
> diff --git a/convert/libosinfo_utils.mli b/convert/libosinfo_utils.mli
> index 67be16c491a4..14991bc2d782 100644
> --- a/convert/libosinfo_utils.mli
> +++ b/convert/libosinfo_utils.mli
> @@ -28,12 +28,24 @@ val get_os_by_short_id : string ->
Libosinfo.osinfo_os
>  val string_of_osinfo_device_list : Libosinfo.osinfo_device list ->
string
>  (** Convert an [osinfo_device] list to a printable string for debugging.
*)
>  
>  val string_of_osinfo_device_driver : Libosinfo.osinfo_device_driver ->
string
>  (** Convert a [osinfo_device_driver] to a printable string for debugging.
*)
>  
> +val best_driver : Libosinfo.osinfo_device_driver list ->
> +                  string ->
> +                  Libosinfo.osinfo_device_driver
> +(** [best_driver drivers arch] picks the best driver from [drivers] as
follows:
> +    - filters out drivers that:
> +      - target a different architecture,
> +      - are not pre-installable,
> +      - have an invalid or non-local URL;
> +    - sorts the remaining drivers by priority, like libosinfo does;
> +    - picks the top driver of the sorted list.
> +    Raises Not_found if no driver in [drivers] survives filtering. *)
> +
>  type os_support = {
>    q35 : bool;
>    vio10 : bool;
>  }
>  (** Tell whether the operating system supports the Q35 board type and/or
>      non-transitional (virtio-1.0-only) virtio devices. (Internally, the
> diff --git a/convert/libosinfo_utils.ml b/convert/libosinfo_utils.ml
> index f0d70ffd4997..8504e2b2f001 100644
> --- a/convert/libosinfo_utils.ml
> +++ b/convert/libosinfo_utils.ml
> @@ -76,12 +76,47 @@ let string_of_osinfo_device_driver {
Libosinfo.architecture; location;
>      (if pre_installable then "pre-installable" else "not
pre-installable")
>      (if signed then "signed" else "unsigned")
>      priority
>      (String.concat "\n" files)
>      (string_of_osinfo_device_list devices)
>  
> +let best_driver drivers arch > +  let debug_drivers > +    List.iter
(fun d -> debug "Driver: %s" (string_of_osinfo_device_driver d))
> +  (* The architecture that "inspect.i_arch" from libguestfs
> +   * ("daemon/filearch.ml") calls "i386", the
osinfo-db schema
> +   * ("data/schema/osinfo.rng.in") calls "i686".
> +   *)
> +  and arch = if arch = "i386" then "i686" else arch in
> +  debug "libosinfo drivers before filtering:";
> +  debug_drivers drivers;
> +  let drivers > +    List.filter (
> +      fun { Libosinfo.architecture; location; pre_installable } ->
> +        if architecture <> arch || not pre_installable then
> +          false
> +        else
> +          try
> +            (match Xml.parse_uri location with
> +            | { Xml.uri_scheme = Some scheme;
> +                Xml.uri_path = Some _ } when scheme = "file"
-> true
> +            | _ -> false
> +            )
> +          with Invalid_argument _ -> false
> +    ) drivers in
> +  debug "libosinfo drivers after filtering:";
> +  debug_drivers drivers;
> +  let drivers > +    List.sort (
> +      fun { Libosinfo.priority = prioA } { Libosinfo.priority = prioB }
->
> +        compare prioB prioA
> +    ) drivers in
> +  if drivers = [] then
> +    raise Not_found;
> +  List.hd drivers
> +
>  type os_support = {
>    q35 : bool;
>    vio10 : bool;
>  }
>  
>  let os_support_of_osinfo_device_list > diff --git
a/convert/windows_virtio.ml b/convert/windows_virtio.ml
> index 610a56857c7e..5254322c7d28 100644
> --- a/convert/windows_virtio.ml
> +++ b/convert/windows_virtio.ml
> @@ -405,59 +405,17 @@ and virtio_iso_path_matches_qemu_ga path inspect >
*
>   * Files that do not exist are silently skipped.
>   *
>   * Returns list of copied files.
>   *)
>  and copy_from_libosinfo g inspect destdir > -  let debug_drivers > -
List.iter (
> -      fun d ->
> -        debug "Driver: %s"
(Libosinfo_utils.string_of_osinfo_device_driver d)
> -    )
> -  in
>    let { i_osinfo = osinfo; i_arch = arch } = inspect in
> -  (* The architecture that "inspect.i_arch" from libguestfs
> -   * ("daemon/filearch.ml") calls "i386", the
osinfo-db schema
> -   * ("data/schema/osinfo.rng.in") calls "i686".
> -   *)
> -  let arch = if arch = "i386" then "i686" else arch in
>    try
>      let os = Libosinfo_utils.get_os_by_short_id osinfo in
>      let drivers = os#get_device_drivers () in
> -    debug "libosinfo drivers before filtering:"; debug_drivers
drivers;
> -    (*
> -     * Filter out drivers that we cannot use:
> -     * - for a different architecture
> -     * - non-pre-installable ones
> -     * - location is an invalid URL, or a non-local one
> -     *)
> -    let drivers > -      List.filter (
> -        fun { Libosinfo.architecture; location; pre_installable } ->
> -          if architecture <> arch || not pre_installable then
> -            false
> -          else
> -            try
> -              (match Xml.parse_uri location with
> -              | { Xml.uri_scheme = Some scheme;
> -                  Xml.uri_path = Some _ } when scheme = "file"
-> true
> -              | _ -> false
> -              )
> -            with Invalid_argument _ -> false
> -      ) drivers in
> -    debug "libosinfo drivers after filtering:"; debug_drivers
drivers;
> -    (* Sort the drivers by priority, like libosinfo does. *)
> -    let drivers > -      List.sort (
> -        fun { Libosinfo.priority = prioA } { Libosinfo.priority = prioB }
->
> -          compare prioB prioA
> -      ) drivers in
> -    (* Any driver available? *)
> -    if drivers = [] then
> -      raise Not_found;
> -    let driver = List.hd drivers in
> +    let driver = Libosinfo_utils.best_driver drivers arch in
>      let uri = Xml.parse_uri driver.Libosinfo.location in
>      let basedir >        match uri.Xml.uri_path with
>        | Some p -> p
>        | None -> assert false in
>      List.filter_map (
> -- 
> 2.19.1.3.g30247aa5d201
> 
Nice refactoring, ACK

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v