Laszlo Ersek
2022-Feb-01 12:32 UTC
[Libguestfs] [v2v PATCH v2 0/7] Fix "virtio-transitional" regression for Windows guests
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2043333 v1: https://listman.redhat.com/archives/libguestfs/2022-January/msg00200.html In v2, patch#7 has been made consistent with the bugfix in patch#1. (Please see the Notes sections of the individual patches for more details.) 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 | 26 ++++++- 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 ++----------- convert/windows_virtio.mli | 6 +- docs/virt-v2v.pod | 24 ++++-- tests/test-v2v-cdrom.expected | 2 +- tests/test-v2v-floppy.expected | 2 +- tests/test-v2v-i-ova.xml | 8 +- 12 files changed, 213 insertions(+), 150 deletions(-) base-commit: 8abc07a8589a48c79cc65159640e0d8ab3c9b261 -- 2.19.1.3.g30247aa5d201
Laszlo Ersek
2022-Feb-01 12:32 UTC
[Libguestfs] [v2v PATCH v2 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> --- Notes: v2: - no change 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-Feb-01 12:32 UTC
[Libguestfs] [v2v PATCH v2 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> Acked-by: Richard W.M. Jones <rjones at redhat.com> --- Notes: v2: - pick up Rich's ACK 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-Feb-01 12:32 UTC
[Libguestfs] [v2v PATCH v2 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> Acked-by: Richard W.M. Jones <rjones at redhat.com> --- Notes: v2: - pick up Rich's ACK 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-Feb-01 12:32 UTC
[Libguestfs] [v2v PATCH v2 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> Acked-by: Richard W.M. Jones <rjones at redhat.com> --- Notes: v2: - pick up Rich's ACK 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-Feb-01 12:32 UTC
[Libguestfs] [v2v PATCH v2 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> Acked-by: Richard W.M. Jones <rjones at redhat.com> --- Notes: v2: - pick up Rich's ACK 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-Feb-01 12:32 UTC
[Libguestfs] [v2v PATCH v2 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> Acked-by: Richard W.M. Jones <rjones at redhat.com> --- Notes: v2: - pick up Rich's ACK 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-Feb-01 12:32 UTC
[Libguestfs] [v2v PATCH v2 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)). The reason is that those tests all set the VIRTIO_WIN environment variable, in which case we simply assume "virtio_1_0 = true" on the fallback ("Not_found" exception) branch. Fixes: f0cea012d0183edf6f7b769c28d5038593f3fe6a Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2043333 Signed-off-by: Laszlo Ersek <lersek at redhat.com> --- Notes: v2: - mirror the behavior of "Windows_virtio.copy_drivers" exactly, with regard to: - the VIRTIO_WIN(_DIR) environment variables, and - "Libosinfo_utils.best_driver" not finding any matching driver. (The behavior upon "Libosinfo_utils.get_os_by_short_id" failing was consistent with "Windows_virtio.copy_drivers" in v1 already.) convert/windows_virtio.mli | 6 +++++- convert/convert_windows.ml | 26 +++++++++++++++++++++++--- tests/test-v2v-cdrom.expected | 2 +- tests/test-v2v-floppy.expected | 2 +- tests/test-v2v-i-ova.xml | 8 ++++---- 5 files changed, 34 insertions(+), 10 deletions(-) diff --git a/convert/windows_virtio.mli b/convert/windows_virtio.mli index 53603d240553..a92cc01da6df 100644 --- a/convert/windows_virtio.mli +++ b/convert/windows_virtio.mli @@ -13,13 +13,17 @@ * * You should have received a copy of the GNU General Public License along * with this program; if not, write to the Free Software Foundation, Inc., * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. *) -(** Functions for installing Windows virtio drivers. *) +(** Values and functions for installing Windows virtio drivers. *) + +val virtio_win_from_env : bool +(** [virtio_win_from_env] is true iff at least one of the VIRTIO_WIN and + VIRTIO_WIN_DIR variables is present in the environment. *) val install_drivers : Registry.t -> Types.inspect -> Types.guestcaps_block_type * Types.guestcaps_net_type * bool * bool * bool * bool (** [install_drivers reg inspect] installs virtio drivers from the driver directory or driver diff --git a/convert/convert_windows.ml b/convert/convert_windows.ml index 1c2d17f26543..7bdcaf52f33c 100644 --- a/convert/convert_windows.ml +++ b/convert/convert_windows.ml @@ -239,18 +239,38 @@ let convert (g : G.guestfs) _ inspect _ static_ips ); let machine, virtio_1_0 match inspect.i_arch with | ("i386"|"x86_64") -> (try + (* Fall back to the decision that's based on the year that the OS + * was released in under three circumstances: + * - the user specified the location of the Windows virtio drivers + * through an environment variable, or + * - "Libosinfo_utils.get_os_by_short_id" fails to look up the OS, + * or + * - "Libosinfo_utils.best_driver" cannot find any matching driver. + * In each of these cases, a "Not_found" exception is raised. This + * behavior exactly mirrors that of "Windows_virtio.copy_drivers". + *) + if Windows_virtio.virtio_win_from_env then + raise Not_found; 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 + (Libosinfo_utils.best_driver drivers inspect.i_arch).devices 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