thr3ads.net - Libguestfs - [Libguestfs] [v2v PATCH 0/7] Fix "virtio-transitional" regression for Windows guests [Jan 2022]

If this information is useful, please help other people find it:
Share via:

Laszlo Ersek

2022-Jan-28 16:12 UTC

[Libguestfs] [v2v PATCH 0/7] Fix "virtio-transitional" regression for Windows guests

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2043333

My series merged as commit range f9d5448d2efe..f0cea012d018 (for RHBZ
1942325) caused a regression when converting Windows guests.

Namely, osinfo_os_get_all_devices(), the core of that series, only
reports devices that an OS supports "out of the box". So, for modern
Windows releases, Q35 is reported, but virtio1.0-net is never reported,
as the latter requires external drivers. As a result, the series in
question (incorrectly) requires virtio-transitional for Windows guests.

The current series fixes the issue as follows:

- The first two patches fix existent bugs in our "copy_from_libosinfo"
  function.

- Patches #3 and #4 OCaml-ify the osinfo_device_driver_get_devices()
  function from libosinfo.

- Patch #5 improves libosinfo-related logging.

- Patch #6 extracts the "best virtio driver for the guest" logic from
  "copy_from_libosinfo".

- Patch #7 grabs the device list supported by the "best driver", and
  concatenates it with the list of devices supported by the OS "out of
  the box". The unified list is then checked for both Q35 and virtio-1.0
  support.

Note: I still don't have a local environment for testing this against
actual (and, long-term!) Windows guests. I'll do that later, and/or
prepare a RHEL9 scratch build for Virt-tools QE at RH, for testing this
series.

Thanks,
Laszlo

Laszlo Ersek (7):
  convert/windows_virtio: fix copy_from_libosinfo <-> VIRTIO_WIN
    priority
  convert/windows_virtio: map 32-bit arch name from libguestfs to osinfo
  convert/libosinfo: factor out v2v_osinfo_device_list_to_value_list()
  convert/libosinfo: retrieve the device list for OsinfoDeviceDriver
  convert/libosinfo_utils: debug-log the devices supported by a driver
  convert/libosinfo_utils: extract "best_driver" from
    "windows_virtio.ml"
  convert/convert_windows: consult "best driver"'s dev list for
    virtio-1.0

 convert/convert_windows.ml     |  15 +++-
 convert/libosinfo-c.c          | 132 ++++++++++++++++++---------------
 convert/libosinfo.ml           |  19 ++---
 convert/libosinfo.mli          |  19 ++---
 convert/libosinfo_utils.ml     |  56 +++++++++++---
 convert/libosinfo_utils.mli    |  18 ++++-
 convert/windows_virtio.ml      |  51 ++-----------
 docs/virt-v2v.pod              |  24 ++++--
 tests/test-v2v-cdrom.expected  |   2 +-
 tests/test-v2v-floppy.expected |   2 +-
 tests/test-v2v-i-ova.xml       |   8 +-
 11 files changed, 197 insertions(+), 149 deletions(-)


base-commit: 68af35d48ca845133ede948d36ee351d171e3de8
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jan-28 16:12 UTC

head link

[Libguestfs] [v2v PATCH 1/7] convert/windows_virtio: fix copy_from_libosinfo <-> VIRTIO_WIN priority

Commit 258e4b718d5d introduced the "copy_from_libosinfo" branch in
"copy_drivers" with higher priority than the existent
"copy_from_virtio_win".

This introduced a conflict with the documentation
("docs/virt-v2v.pod").
The documentation stated (and still states) that the VIRTIO_WIN env var,
when set, dictates where virt-v2v look for the virtio-win drivers. The
conflict is that, even in case we set VIRTIO_WIN, virt-v2v still consults
libosinfo first.

Skip the "copy_from_libosinfo" branch in case VIRTIO_WIN is set, plus
explain in the documentation that VIRTIO_WIN, when set, overrides
libosinfo.

NOTE: the necessity of this fix is made apparent by the next patch in the
series. Namely, "copy_from_libosinfo" has a bug in the architecture
filter. Once we fix that bug in the next patch, *and* install (e.g.)
"virtio-win-1.9.24-4.el8.noarch" on the conversion host, the
"i-ova" test
case will suddenly prefer the virtio drivers exposed via libosinfo, in
spite of the "tests/test-v2v-i-ova.sh" script explicitly setting
VIRTIO_WIN to "test-data/fake-virtio-win". The symptom is that the
pvpanic
device will be enabled in the domain XML, due to it being supported by the
actual (not fake) virtio-win drivers.

Fixes: 258e4b718d5d80c79634fe864c8c52b12e41777c
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2043333
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 convert/windows_virtio.ml | 12 ++++++------
 docs/virt-v2v.pod         | 24 +++++++++++++++++-------
 2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/convert/windows_virtio.ml b/convert/windows_virtio.ml
index 1c5c148e65fd..d21ee6814458 100644
--- a/convert/windows_virtio.ml
+++ b/convert/windows_virtio.ml
@@ -26,20 +26,20 @@ open Regedit
 
 open Types
 open Utils
 
 module G = Guestfs
 
-let virtio_win -  try Sys.getenv "VIRTIO_WIN"
+let virtio_win, virtio_win_from_env +  try Sys.getenv "VIRTIO_WIN",
true
   with Not_found ->
-    try Sys.getenv "VIRTIO_WIN_DIR" (* old name for VIRTIO_WIN *)
+    try Sys.getenv "VIRTIO_WIN_DIR" (* old name for VIRTIO_WIN *),
true
     with Not_found ->
       let iso = Config.datadir // "virtio-win" //
"virtio-win.iso" in
-      if Sys.file_exists iso then iso
-      else Config.datadir // "virtio-win"
+      (if Sys.file_exists iso then iso
+       else Config.datadir // "virtio-win"), false
 
 let scsi_class_guid = "{4D36E97B-E325-11CE-BFC1-08002BE10318}"
 let viostor_legacy_pciid =
"VEN_1AF4&DEV_1001&SUBSYS_00021AF4&REV_00"
 let viostor_modern_pciid =
"VEN_1AF4&DEV_1042&SUBSYS_11001AF4&REV_01"
 let vioscsi_legacy_pciid =
"VEN_1AF4&DEV_1004&SUBSYS_00081AF4&REV_00"
 let vioscsi_modern_pciid =
"VEN_1AF4&DEV_1048&SUBSYS_11001AF4&REV_01"
@@ -225,13 +225,13 @@ and ddb_regedits inspect drv_name drv_pciid    ]
 
 (* Copy the matching drivers to the driverdir; return true if any have
  * been copied.
  *)
 and copy_drivers g inspect driverdir -  [] <> copy_from_libosinfo g
inspect driverdir ||
+  (not virtio_win_from_env && [] <> copy_from_libosinfo g inspect
driverdir) ||
     [] <> copy_from_virtio_win g inspect "/" driverdir
       virtio_iso_path_matches_guest_os
       (fun () ->
         error (f_"root directory ?/? is missing from the virtio-win
directory or ISO.\n\nThis should not happen and may indicate that virtio-win or
virt-v2v is broken in some way.  Please report this as a bug with a full debug
log."))
 
 and copy_qemu_ga g inspect diff --git a/docs/virt-v2v.pod b/docs/virt-v2v.pod
index 3d0e00a354ae..89c47cd3823c 100644
--- a/docs/virt-v2v.pod
+++ b/docs/virt-v2v.pod
@@ -916,13 +916,18 @@ below.
 
  Debian 6+      All versions support virtio
 
  Ubuntu 10.04+  All versions support virtio
 
  Windows        Drivers are installed from the ISO or directory pointed
-                to by "VIRTIO_WIN" environment variable if present
+                to by the "VIRTIO_WIN" environment variable if
present.
+                If the "VIRTIO_WIN" environment variable is absent
+                (which is the recommended setting), then libosinfo is
+                consulted first, for driver files that are locally
+                available on the conversion host.
+
 
 =head2 RHEL 4: SELinux relabel appears to hang forever
 
 In RHEL E<le> 4.7 there was a bug which causes SELinux relabelling
 to appear to hang forever at:
 
@@ -1592,29 +1597,34 @@ script in Windows guests.  It is required if you intend
to use the
 I<--firstboot> or I<--firstboot-command> options with Windows
guests.
 
 =back
 
 =item C<VIRTIO_WIN>
 
-This is where virtio drivers for Windows are searched for.  It can be
-a directory I<or> point to F<virtio-win.iso> (CD ROM image
containing
-drivers).
+This is an override for where virtio drivers for Windows are searched
+for.  It can be a directory I<or> point to F<virtio-win.iso> (CD
ROM
+image containing drivers).
 
-If unset, then we look for drivers in whichever of these paths
-is found first:
+If unset, then we look for drivers via whichever of these methods
+succeeds first:
 
 =over 4
 
+=item C<osinfo-db>
+
+Load osinfo data from the default paths, and attempt to find drivers via
+libosinfo lookup.  This is the preferred method.
+
 =item F</usr/share/virtio-win/virtio-win.iso>
 
 The ISO containing virtio drivers for Windows.
 
 =item F</usr/share/virtio-win>
 
 The exploded tree of virtio drivers for Windows.  This is
-usually incomplete, hence the ISO is preferred.
+usually incomplete, hence the least preferred method.
 
 =back
 
 See L</Enabling virtio>.
 
 =back
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jan-28 16:12 UTC

head link

[Libguestfs] [v2v PATCH 2/7] convert/windows_virtio: map 32-bit arch name from libguestfs to osinfo

For Windows guests, the "inspect.i_arch" field is ultimately
determined in
libguestfs, on the following call path:

  inspect_os                             [daemon/inspect.ml]
    check_for_filesystem_on              [daemon/inspect_fs.ml]
      check_filesystem                   [daemon/inspect_fs.ml]
        check_windows_root               [daemon/inspect_fs_windows.ml]
          check_windows_arch             [daemon/inspect_fs_windows.ml]
            file_architecture            [daemon/filearch.ml]
              file_architecture_of_magic [daemon/filearch.ml]

where the last function maps "PE32 executable" to "i386".

(As of libguestfs commit 5858c2cf6c24.)

However, in osinfo-db (as of commit 72c69622e6db), the
"data/schema/osinfo.rng.in" schema calls the same architecture
"i686".

Perform this mapping in the "copy_from_libosinfo" function explicitly:
the
filter currently throws away all "i686" drivers from libosinfo because
they don't match "i386" from libguestfs.

(There is no such problem with "x86_64".)

Fixes: 258e4b718d5d80c79634fe864c8c52b12e41777c
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2043333
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 convert/windows_virtio.ml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/convert/windows_virtio.ml b/convert/windows_virtio.ml
index d21ee6814458..1faba8a874c0 100644
--- a/convert/windows_virtio.ml
+++ b/convert/windows_virtio.ml
@@ -412,12 +412,17 @@ and copy_from_libosinfo g inspect destdir      List.iter (
       fun d ->
         debug "\t%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:
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jan-28 16:12 UTC

head link

[Libguestfs] [v2v PATCH 3/7] convert/libosinfo: factor out v2v_osinfo_device_list_to_value_list()

Move the guts of v2v_osinfo_os_get_all_devices() to a new static function
called v2v_osinfo_device_list_to_value_list().

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2043333
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 convert/libosinfo-c.c | 126 +++++++++++++++++++++++-------------------
 1 file changed, 68 insertions(+), 58 deletions(-)

diff --git a/convert/libosinfo-c.c b/convert/libosinfo-c.c
index ec7c06d379e3..bc5816f6dacb 100644
--- a/convert/libosinfo-c.c
+++ b/convert/libosinfo-c.c
@@ -207,65 +207,12 @@ glist_to_value_list (GList *list)
     rv = v;
   }
 
   CAMLreturn (rv);
 }
 
-value
-v2v_osinfo_os_get_device_drivers (value osv)
-{
-  CAMLparam1 (osv);
-  CAMLlocal4 (rv, v, vi, copyv);
-  OsinfoDeviceDriverList *list;
-  gint i, len;
-
-  list = osinfo_os_get_device_drivers (OsinfoOs_t_val (osv));
-  len = osinfo_list_get_length (OSINFO_LIST(list));
-
-  rv = Val_emptylist;
-  for (i = len - 1; i >= 0; --i) {
-    OsinfoDeviceDriver *driver;
-    const gchar *str;
-    gboolean b;
-    GList *l;
-    gint64 i64;
-
-    driver = OSINFO_DEVICE_DRIVER(osinfo_list_get_nth (OSINFO_LIST(list), i));
-
-    vi = caml_alloc (6, 0);
-    str = osinfo_device_driver_get_architecture (driver);
-    copyv = caml_copy_string (str);
-    Store_field (vi, 0, copyv);
-    str = osinfo_device_driver_get_location (driver);
-    copyv = caml_copy_string (str);
-    Store_field (vi, 1, copyv);
-    b = osinfo_device_driver_get_pre_installable (driver);
-    Store_field (vi, 2, Val_bool (b));
-    b = osinfo_device_driver_get_signed (driver);
-    Store_field (vi, 3, Val_bool (b));
-#if IS_LIBOSINFO_VERSION(1, 7, 0)
-    i64 = osinfo_device_driver_get_priority (driver);
-#else
-    /* Same as OSINFO_DEVICE_DRIVER_DEFAULT_PRIORITY in libosinfo 1.7.0+. */
-    i64 = 50;
-#endif
-    copyv = caml_copy_int64 (i64);
-    Store_field (vi, 4, copyv);
-    l = osinfo_device_driver_get_files (driver);
-    Store_field (vi, 5, glist_to_value_list (l));
-    g_list_free (l);
-
-    v = caml_alloc (2, 0);
-    Store_field (v, 0, vi);
-    Store_field (v, 1, rv);
-    rv = v;
-  }
-
-  CAMLreturn (rv);
-}
-
 /* Collect OsinfoDevice properties from two levels:
  *
  * - The OSINFO_ENTITY_PROP_ID property, originating from the OsinfoEntity base
  *   class. This is a unique URI, identifying the device.
  *
  * - All currently known OSINFO_DEVICE_PROP_* properties, originating from the
@@ -285,23 +232,21 @@ static const char * const device_prop[] = {
   OSINFO_DEVICE_PROP_CLASS,
   OSINFO_DEVICE_PROP_BUS_TYPE,
   OSINFO_DEVICE_PROP_SUBSYSTEM,
 };
 #define NUM_DEVICE_PROPS (sizeof device_prop / sizeof device_prop[0])
 
-value
-v2v_osinfo_os_get_all_devices (value osv)
+static value
+v2v_osinfo_device_list_to_value_list (OsinfoDeviceList *dev_list)
 {
-  CAMLparam1 (osv);
+  CAMLparam0 ();
   CAMLlocal4 (retvalv, linkv, propsv, copyv);
-  g_autoptr (OsinfoDeviceList) dev_list = NULL;
   OsinfoList *ent_list;
   gint ent_nr;
 
   retvalv = Val_emptylist;
-  dev_list = osinfo_os_get_all_devices (OsinfoOs_t_val (osv), NULL);
   ent_list = OSINFO_LIST (dev_list);
   ent_nr = osinfo_list_get_length (ent_list);
 
   while (ent_nr > 0) {
     OsinfoEntity *ent;
     size_t prop_nr;
@@ -325,6 +270,71 @@ v2v_osinfo_os_get_all_devices (value osv)
     Store_field (linkv, 1, retvalv);
     retvalv = linkv;
   }
 
   CAMLreturn (retvalv);
 }
+
+value
+v2v_osinfo_os_get_device_drivers (value osv)
+{
+  CAMLparam1 (osv);
+  CAMLlocal4 (rv, v, vi, copyv);
+  OsinfoDeviceDriverList *list;
+  gint i, len;
+
+  list = osinfo_os_get_device_drivers (OsinfoOs_t_val (osv));
+  len = osinfo_list_get_length (OSINFO_LIST(list));
+
+  rv = Val_emptylist;
+  for (i = len - 1; i >= 0; --i) {
+    OsinfoDeviceDriver *driver;
+    const gchar *str;
+    gboolean b;
+    GList *l;
+    gint64 i64;
+
+    driver = OSINFO_DEVICE_DRIVER(osinfo_list_get_nth (OSINFO_LIST(list), i));
+
+    vi = caml_alloc (6, 0);
+    str = osinfo_device_driver_get_architecture (driver);
+    copyv = caml_copy_string (str);
+    Store_field (vi, 0, copyv);
+    str = osinfo_device_driver_get_location (driver);
+    copyv = caml_copy_string (str);
+    Store_field (vi, 1, copyv);
+    b = osinfo_device_driver_get_pre_installable (driver);
+    Store_field (vi, 2, Val_bool (b));
+    b = osinfo_device_driver_get_signed (driver);
+    Store_field (vi, 3, Val_bool (b));
+#if IS_LIBOSINFO_VERSION(1, 7, 0)
+    i64 = osinfo_device_driver_get_priority (driver);
+#else
+    /* Same as OSINFO_DEVICE_DRIVER_DEFAULT_PRIORITY in libosinfo 1.7.0+. */
+    i64 = 50;
+#endif
+    copyv = caml_copy_int64 (i64);
+    Store_field (vi, 4, copyv);
+    l = osinfo_device_driver_get_files (driver);
+    Store_field (vi, 5, glist_to_value_list (l));
+    g_list_free (l);
+
+    v = caml_alloc (2, 0);
+    Store_field (v, 0, vi);
+    Store_field (v, 1, rv);
+    rv = v;
+  }
+
+  CAMLreturn (rv);
+}
+
+value
+v2v_osinfo_os_get_all_devices (value osv)
+{
+  CAMLparam1 (osv);
+  CAMLlocal1 (retvalv);
+  g_autoptr (OsinfoDeviceList) dev_list = NULL;
+
+  dev_list = osinfo_os_get_all_devices (OsinfoOs_t_val (osv), NULL);
+  retvalv = v2v_osinfo_device_list_to_value_list (dev_list);
+  CAMLreturn (retvalv);
+}
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jan-28 16:12 UTC

head link

[Libguestfs] [v2v PATCH 4/7] convert/libosinfo: retrieve the device list for OsinfoDeviceDriver

The OsinfoDeviceDriver class supports the
osinfo_device_driver_get_devices() method, we've just had no use for that
thus far.

Using the previously extracted v2v_osinfo_device_list_to_value_list()
function, we can now relatively easily OCaml-ify the list of devices that
a driver supports. (We'll use this functionality in a subsequent patch.)

(Notably, osinfo_device_driver_get_devices() is marked "transfer
none",
unlike osinfo_os_get_all_devices(), which is marked "transfer full".)

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2043333
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 convert/libosinfo.mli | 19 ++++++++++---------
 convert/libosinfo.ml  | 19 ++++++++++---------
 convert/libosinfo-c.c |  6 ++++++
 3 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/convert/libosinfo.mli b/convert/libosinfo.mli
index 1ece7b41a310..aa436370e46d 100644
--- a/convert/libosinfo.mli
+++ b/convert/libosinfo.mli
@@ -17,33 +17,34 @@
  *)
 
 (** This module implements a minimal libosinfo API. *)
 
 type osinfo_os_t
 
-type osinfo_device_driver = {
-  architecture : string;
-  location : string;
-  pre_installable : bool;
-  signed : bool;
-  priority : int64;
-  files : string list;
-}
-
 type osinfo_device = {
   id : string;
   vendor : string;
   vendor_id : string;
   product : string;
   product_id : string;
   name : string;
   class_ : string;
   bus_type : string;
   subsystem : string;
 }
 
+type osinfo_device_driver = {
+  architecture : string;
+  location : string;
+  pre_installable : bool;
+  signed : bool;
+  priority : int64;
+  files : string list;
+  devices : osinfo_device list;
+}
+
 class osinfo_os : osinfo_os_t -> object
   method get_id : unit -> string
   (** Return the ID. *)
   method get_device_drivers : unit -> osinfo_device_driver list
   (** Return the list of device drivers. *)
   method get_devices : unit -> osinfo_device list
diff --git a/convert/libosinfo.ml b/convert/libosinfo.ml
index 78271be248ea..8ea0a279ca27 100644
--- a/convert/libosinfo.ml
+++ b/convert/libosinfo.ml
@@ -20,33 +20,34 @@ open Std_utils
 open Tools_utils
 open Common_gettext.Gettext
 
 type osinfo_db_t
 type osinfo_os_t
 
-type osinfo_device_driver = {
-  architecture : string;
-  location : string;
-  pre_installable : bool;
-  signed : bool;
-  priority : int64;
-  files : string list;
-}
-
 type osinfo_device = {
   id : string;
   vendor : string;
   vendor_id : string;
   product : string;
   product_id : string;
   name : string;
   class_ : string;
   bus_type : string;
   subsystem : string;
 }
 
+type osinfo_device_driver = {
+  architecture : string;
+  location : string;
+  pre_installable : bool;
+  signed : bool;
+  priority : int64;
+  files : string list;
+  devices : osinfo_device list;
+}
+
 external osinfo_os_get_id : osinfo_os_t -> string =
"v2v_osinfo_os_get_id"
 external osinfo_os_get_device_drivers : osinfo_os_t -> osinfo_device_driver
list = "v2v_osinfo_os_get_device_drivers"
 external osinfo_os_get_devices : osinfo_os_t -> osinfo_device list =
"v2v_osinfo_os_get_all_devices"
 
 class osinfo_os h    object (self)
diff --git a/convert/libosinfo-c.c b/convert/libosinfo-c.c
index bc5816f6dacb..93357fd91b7f 100644
--- a/convert/libosinfo-c.c
+++ b/convert/libosinfo-c.c
@@ -289,12 +289,13 @@ v2v_osinfo_os_get_device_drivers (value osv)
   for (i = len - 1; i >= 0; --i) {
     OsinfoDeviceDriver *driver;
     const gchar *str;
     gboolean b;
     GList *l;
     gint64 i64;
+    OsinfoDeviceList *dev_list;
 
     driver = OSINFO_DEVICE_DRIVER(osinfo_list_get_nth (OSINFO_LIST(list), i));
 
     vi = caml_alloc (6, 0);
     str = osinfo_device_driver_get_architecture (driver);
     copyv = caml_copy_string (str);
@@ -314,12 +315,17 @@ v2v_osinfo_os_get_device_drivers (value osv)
 #endif
     copyv = caml_copy_int64 (i64);
     Store_field (vi, 4, copyv);
     l = osinfo_device_driver_get_files (driver);
     Store_field (vi, 5, glist_to_value_list (l));
     g_list_free (l);
+    dev_list = osinfo_device_driver_get_devices (driver);
+    v = (dev_list == NULL) ?
+        Val_emptylist :
+        v2v_osinfo_device_list_to_value_list (dev_list);
+    Store_field (vi, 6, v);
 
     v = caml_alloc (2, 0);
     Store_field (v, 0, vi);
     Store_field (v, 1, rv);
     rv = v;
   }
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jan-28 16:12 UTC

head link

[Libguestfs] [v2v PATCH 5/7] convert/libosinfo_utils: debug-log the devices supported by a driver

While at it, put each file of a driver "pack" on a separate line, and
print "Driver:", "Files:" and "Devices:" headers
as well.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2043333
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 convert/libosinfo_utils.mli |  6 +++---
 convert/libosinfo_utils.ml  | 21 +++++++++++----------
 convert/windows_virtio.ml   |  2 +-
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/convert/libosinfo_utils.mli b/convert/libosinfo_utils.mli
index ab77ec97df30..67be16c491a4 100644
--- a/convert/libosinfo_utils.mli
+++ b/convert/libosinfo_utils.mli
@@ -22,18 +22,18 @@ val get_os_by_short_id : string -> Libosinfo.osinfo_os
 (** [get_os_by_short_id short-id] get the [Libosinfo.osinfo_os]
     that has the specified [short-id].
 
     Raise [Not_found] in case there is no matching OS.
  *)
 
-val string_of_osinfo_device_driver : Libosinfo.osinfo_device_driver ->
string
-(** Convert a [osinfo_device_driver] to a printable string for debugging. *)
-
 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. *)
+
 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 77f22272f979..f0d70ffd4997 100644
--- a/convert/libosinfo_utils.ml
+++ b/convert/libosinfo_utils.ml
@@ -30,22 +30,12 @@ let get_db ()  
 let get_os_by_short_id os    let os = (get_db ())#find_os_by_short_id os in
   debug "libosinfo: loaded OS: %s" (os#get_id ());
   os
 
-let string_of_osinfo_device_driver { Libosinfo.architecture; location;
-                                     pre_installable; signed; priority;
-                                     files } -  Printf.sprintf "%s: [%s,
%s, %s, priority %Ld] %s"
-    location architecture
-    (if pre_installable then "pre-installable" else "not
pre-installable")
-    (if signed then "signed" else "unsigned")
-    priority
-    (String.concat " " files)
-
 let string_of_osinfo_device_list dev_list  
   (* Turn the fields of an "osinfo_device" record into a list. *)
   let listify { Libosinfo.id; vendor; vendor_id; product; product_id; name;
                 class_; bus_type; subsystem }      [ id; vendor; vendor_id;
product; product_id; name;
@@ -75,12 +65,23 @@ let string_of_osinfo_device_list dev_list     * per line,
and (b) right-padding each field of each "osinfo_device" record
    * to the maximum width of that field.
    *)
   String.concat "\n"
     (List.map (fun dev -> columnate (listify dev) max_widths) dev_list)
 
+let string_of_osinfo_device_driver { Libosinfo.architecture; location;
+                                     pre_installable; signed; priority;
+                                     files; devices } +  Printf.sprintf
"%s: [%s, %s, %s, priority %Ld]\nFiles:\n%s\nDevices:\n%s"
+    location architecture
+    (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)
+
 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 1faba8a874c0..610a56857c7e 100644
--- a/convert/windows_virtio.ml
+++ b/convert/windows_virtio.ml
@@ -408,13 +408,13 @@ and virtio_iso_path_matches_qemu_ga path inspect   *
Returns list of copied files.
  *)
 and copy_from_libosinfo g inspect destdir    let debug_drivers      List.iter (
       fun d ->
-        debug "\t%s" (Libosinfo_utils.string_of_osinfo_device_driver
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".
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2022-Jan-28 16:12 UTC

head link

[Libguestfs] [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

Laszlo Ersek

2022-Jan-28 16:12 UTC

head link

[Libguestfs] [v2v PATCH 7/7] convert/convert_windows: consult "best driver"'s dev list for virtio-1.0

Life would be too simple if we could just call osinfo_os_get_all_devices()
for determining virtio-1.0 support for Windows guests. Now that we've
extracted the libosinfo logic that identifies the "best" virtio driver
(for copying its files into the guest), fetch the list of devices
supported by the same "best driver" as well. Use that list, in
addition to
the one from osinfo_os_get_all_devices(), for determining (Q35 and)
virtio-1.0 support.

Thankfully this mess is not needed when converting Linux guests.

With this, we need to revert the data for three Windows-based test cases
to their pre-f0cea012d018 status (modulo intermediary commits 4f6b143c1cb3
("output: -o libvirt, qemu: Use correct device name for vsock",
2022-01-20) and 4c3d0b8b3b4b ("output: -o libvirt: Fix <graphics/>
element
port/autoport", 2022-01-20)).

Fixes: f0cea012d0183edf6f7b769c28d5038593f3fe6a
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2043333
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 convert/convert_windows.ml     | 15 ++++++++++++---
 tests/test-v2v-cdrom.expected  |  2 +-
 tests/test-v2v-floppy.expected |  2 +-
 tests/test-v2v-i-ova.xml       |  8 ++++----
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/convert/convert_windows.ml b/convert/convert_windows.ml
index 1c2d17f26543..947775416ef5 100644
--- a/convert/convert_windows.ml
+++ b/convert/convert_windows.ml
@@ -240,17 +240,26 @@ let convert (g : G.guestfs) _ inspect _ static_ips  
     let machine, virtio_1_0        match inspect.i_arch with
       | ("i386"|"x86_64") ->
         (try
            let os = Libosinfo_utils.get_os_by_short_id inspect.i_osinfo in
-           let devices = os#get_devices () in
-           debug "libosinfo devices for OS \"%s\":\n%s"
inspect.i_osinfo
+           let devices = os#get_devices ()
+           and drivers = os#get_device_drivers () in
+           let best_drv_devs +             try (Libosinfo_utils.best_driver
drivers inspect.i_arch).devices
+             with Not_found -> [] in
+           debug "libosinfo internal devices for OS
\"%s\":\n%s"
+             inspect.i_osinfo
              (Libosinfo_utils.string_of_osinfo_device_list devices);
+           debug "libosinfo \"best driver\" devices for OS
\"%s\":\n%s"
+             inspect.i_osinfo
+             (Libosinfo_utils.string_of_osinfo_device_list best_drv_devs);
            let { Libosinfo_utils.q35; vio10 } -            
Libosinfo_utils.os_support_of_osinfo_device_list devices in
+             Libosinfo_utils.os_support_of_osinfo_device_list
+               (devices @ best_drv_devs) in
            (if q35 then Q35 else I440FX), vio10
          with
          | Not_found ->
            (* Pivot on the year 2007.  Any Windows version from earlier than
             * 2007 should use i440fx, anything 2007 or newer should use q35.
             * Luckily this coincides almost exactly with the release of NT 6.
diff --git a/tests/test-v2v-cdrom.expected b/tests/test-v2v-cdrom.expected
index b950492969c8..17bd152d8e64 100644
--- a/tests/test-v2v-cdrom.expected
+++ b/tests/test-v2v-cdrom.expected
@@ -1,7 +1,7 @@
-    <disk type='file' device='disk'
model='virtio-transitional'>
+    <disk type='file' device='disk'>
       <driver name='qemu' type='raw'/>
       <target dev='vda' bus='virtio'/>
     </disk>
     <disk device='cdrom' type='file'>
       <driver name='qemu' type='raw'/>
       <target dev='sdc' bus='sata'/>
diff --git a/tests/test-v2v-floppy.expected b/tests/test-v2v-floppy.expected
index f4b679540c9d..a718c21f94dd 100644
--- a/tests/test-v2v-floppy.expected
+++ b/tests/test-v2v-floppy.expected
@@ -1,7 +1,7 @@
-    <disk type='file' device='disk'
model='virtio-transitional'>
+    <disk type='file' device='disk'>
       <driver name='qemu' type='raw'/>
       <target dev='vda' bus='virtio'/>
     </disk>
     <disk device='floppy' type='file'>
       <driver name='qemu' type='raw'/>
       <target dev='fda'/>
diff --git a/tests/test-v2v-i-ova.xml b/tests/test-v2v-i-ova.xml
index 1915dd40aff0..6b8cda62f051 100644
--- a/tests/test-v2v-i-ova.xml
+++ b/tests/test-v2v-i-ova.xml
@@ -18,13 +18,13 @@
     <type arch='x86_64' machine='q35'>hvm</type>
   </os>
   <on_poweroff>destroy</on_poweroff>
   <on_reboot>restart</on_reboot>
   <on_crash>restart</on_crash>
   <devices>
-    <disk type='file' device='disk'
model='virtio-transitional'>
+    <disk type='file' device='disk'>
       <driver name='qemu' type='raw'/>
       <source file='TestOva-sda'/>
       <target dev='vda' bus='virtio'/>
     </disk>
     <disk device='cdrom' type='file'>
       <driver name='qemu' type='raw'/>
@@ -33,21 +33,21 @@
     <disk device='floppy' type='file'>
       <driver name='qemu' type='raw'/>
       <target dev='fda'/>
     </disk>
     <interface type='bridge'>
       <source bridge='VM Network'/>
-      <model type='virtio-transitional'/>
+      <model type='virtio'/>
     </interface>
     <video>
       <model type='vga' vram='16384' heads='1'/>
     </video>
     <graphics type='vnc' autoport='yes'/>
-    <rng model='virtio-transitional'>
+    <rng model='virtio'>
       <backend model='random'>/dev/urandom</backend>
     </rng>
-    <memballoon model='virtio-transitional'/>
+    <memballoon model='virtio'/>
     <input type='tablet' bus='usb'/>
     <input type='mouse' bus='ps2'/>
     <console type='pty'/>
   </devices>
 </domain>
-- 
2.19.1.3.g30247aa5d201

Libguestfs - Jan 2022 - [v2v PATCH 0/7] Fix "virtio-transitional" regression for Windows guests

[Libguestfs] [v2v PATCH 0/7] Fix "virtio-transitional" regression for Windows guests

[Libguestfs] [v2v PATCH 1/7] convert/windows_virtio: fix copy_from_libosinfo <-> VIRTIO_WIN priority

[Libguestfs] [v2v PATCH 2/7] convert/windows_virtio: map 32-bit arch name from libguestfs to osinfo

[Libguestfs] [v2v PATCH 3/7] convert/libosinfo: factor out v2v_osinfo_device_list_to_value_list()

[Libguestfs] [v2v PATCH 4/7] convert/libosinfo: retrieve the device list for OsinfoDeviceDriver

[Libguestfs] [v2v PATCH 5/7] convert/libosinfo_utils: debug-log the devices supported by a driver

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

[Libguestfs] [v2v PATCH 7/7] convert/convert_windows: consult "best driver"'s dev list for virtio-1.0