Re: [Libguestfs] [PATCH common] mlcustomize: Add accessors for block driver priority list

2023-03-07 Thread Laszlo Ersek
On 3/6/23 17:50, Richard W.M. Jones wrote:
> When injecting virtio-win drivers, allow the list of block drivers
> that we search to be modified.
> ---
>  mlcustomize/inject_virtio_win.ml  | 12 +---
>  mlcustomize/inject_virtio_win.mli | 10 ++
>  2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/mlcustomize/inject_virtio_win.ml 
> b/mlcustomize/inject_virtio_win.ml
> index 4e977b3..2a7c742 100644
> --- a/mlcustomize/inject_virtio_win.ml
> +++ b/mlcustomize/inject_virtio_win.ml
> @@ -49,6 +49,9 @@ type t = {
>of libosinfo.  Although this behaviour is documented, IMHO it has
>always been a bad idea.  We should change this in future to allow
>the user to select where they want to get drivers from. XXX *)
> +
> +  mutable block_driver_priority : string list
> +  (** List of block drivers *)
>  }
>  
>  type block_type = Virtio_blk | IDE
> @@ -107,7 +110,11 @@ and get_inspection g root =
>{ g; root;
>  i_arch; i_major_version; i_minor_version; i_osinfo;
>  i_product_variant; i_windows_current_control_set; i_windows_systemroot;
> -virtio_win = ""; was_set = false }
> +virtio_win = ""; was_set = false;
> +block_driver_priority = ["virtio_blk"; "vrtioblk"; "viostor"] }
> +
> +let get_block_driver_priority t   = t.block_driver_priority
> +let set_block_driver_priority t v = t.block_driver_priority <- v
>  
>  let scsi_class_guid = "{4D36E97B-E325-11CE-BFC1-08002BE10318}"
>  let viostor_legacy_pciid = "VEN_1AF4&DEV_1001&SUBSYS_00021AF4&REV_00"
> @@ -176,14 +183,13 @@ let rec inject_virtio_win_drivers ({ g } as t) reg =
>else (
>  (* Can we install the block driver? *)
>  let block : block_type =
> -  let filenames = ["virtio_blk"; "vrtioblk"; "viostor"] in
>let viostor_driver = try (
>  Some (
>List.find (
>  fun driver_file ->
>let source = driverdir // driver_file ^ ".sys" in
>g#exists source
> -  ) filenames
> +  ) t.block_driver_priority
>  )
>) with Not_found -> None in
>match viostor_driver with
> diff --git a/mlcustomize/inject_virtio_win.mli 
> b/mlcustomize/inject_virtio_win.mli
> index 0ced02e..7bd9b9f 100644
> --- a/mlcustomize/inject_virtio_win.mli
> +++ b/mlcustomize/inject_virtio_win.mli
> @@ -64,6 +64,16 @@ val from_environment : Guestfs.guestfs -> string -> string 
> -> t
>  
>  This should only be used by [virt-v2v] and is considered a legacy 
> method. *)
>  
> +val get_block_driver_priority : t -> string list
> +val set_block_driver_priority : t -> string list -> unit
> +(** Get or set the current block driver priority list.  This is
> +a list of virtio-win block driver names (eg. ["viostor"]) that
> +we search until we come to the first [name ^ ".sys"] that
> +we find, and that is the block driver which gets installed.
> +
> +This module contains a default priority list which should
> +be suitable for most use cases. *)
> +
>  val inject_virtio_win_drivers : t -> Registry.t -> virtio_win_installed
>  (** [inject_virtio_win_drivers t reg]
>  installs virtio drivers from the driver directory or driver

I'm struggling to review this because the series containing e.g. the
patch that is now commit 1794e6bc7a88 ("convert: Add a handle type to
Windows_virtio", 2023-01-16) had never been posted to the list, and so I
didn't get a chance to study what was going to be the foundation of this
patch.

Anyway. We now have three handle-creating functions in the
Inject_virtio_win module in common: "from_path", "from_libosinfo", and
"from_environment". The first two are used by guestfs-tools/customize
(i.e., virt-customize and virt-sysprep), and the last one is used by
virt-v2v.

The "inject_virtio_win_drivers" function operates on the handle that was
created by either of those three functions (and is used by both
customize and virt-v2v).

This patch moves the (implicit) block driver priority list, from
"inject_virtio_win_drivers", to the handle, and adds functions to query
and set the list inside the handle.

Removing the list from "inject_virtio_win_drivers" affects both virt-v2v
and customize; they both call "inject_virtio_win_drivers".

Populating the (now settable) block driver priority list inside
"get_inspection" with the default values restores the default list for
both virt-v2v and customize. Namely, "from_path" (called only by
customize), "from_libosinfo" (called only by customize), and
"from_environment" (called only by virt-v2v) *all* derive their return
values from that of "get_inspection".

Reviewed-by: Laszlo Ersek 

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs



[Libguestfs] [PATCH common] mlcustomize: Add accessors for block driver priority list

2023-03-06 Thread Richard W.M. Jones
When injecting virtio-win drivers, allow the list of block drivers
that we search to be modified.
---
 mlcustomize/inject_virtio_win.ml  | 12 +---
 mlcustomize/inject_virtio_win.mli | 10 ++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/mlcustomize/inject_virtio_win.ml b/mlcustomize/inject_virtio_win.ml
index 4e977b3..2a7c742 100644
--- a/mlcustomize/inject_virtio_win.ml
+++ b/mlcustomize/inject_virtio_win.ml
@@ -49,6 +49,9 @@ type t = {
   of libosinfo.  Although this behaviour is documented, IMHO it has
   always been a bad idea.  We should change this in future to allow
   the user to select where they want to get drivers from. XXX *)
+
+  mutable block_driver_priority : string list
+  (** List of block drivers *)
 }
 
 type block_type = Virtio_blk | IDE
@@ -107,7 +110,11 @@ and get_inspection g root =
   { g; root;
 i_arch; i_major_version; i_minor_version; i_osinfo;
 i_product_variant; i_windows_current_control_set; i_windows_systemroot;
-virtio_win = ""; was_set = false }
+virtio_win = ""; was_set = false;
+block_driver_priority = ["virtio_blk"; "vrtioblk"; "viostor"] }
+
+let get_block_driver_priority t   = t.block_driver_priority
+let set_block_driver_priority t v = t.block_driver_priority <- v
 
 let scsi_class_guid = "{4D36E97B-E325-11CE-BFC1-08002BE10318}"
 let viostor_legacy_pciid = "VEN_1AF4&DEV_1001&SUBSYS_00021AF4&REV_00"
@@ -176,14 +183,13 @@ let rec inject_virtio_win_drivers ({ g } as t) reg =
   else (
 (* Can we install the block driver? *)
 let block : block_type =
-  let filenames = ["virtio_blk"; "vrtioblk"; "viostor"] in
   let viostor_driver = try (
 Some (
   List.find (
 fun driver_file ->
   let source = driverdir // driver_file ^ ".sys" in
   g#exists source
-  ) filenames
+  ) t.block_driver_priority
 )
   ) with Not_found -> None in
   match viostor_driver with
diff --git a/mlcustomize/inject_virtio_win.mli 
b/mlcustomize/inject_virtio_win.mli
index 0ced02e..7bd9b9f 100644
--- a/mlcustomize/inject_virtio_win.mli
+++ b/mlcustomize/inject_virtio_win.mli
@@ -64,6 +64,16 @@ val from_environment : Guestfs.guestfs -> string -> string 
-> t
 
 This should only be used by [virt-v2v] and is considered a legacy method. 
*)
 
+val get_block_driver_priority : t -> string list
+val set_block_driver_priority : t -> string list -> unit
+(** Get or set the current block driver priority list.  This is
+a list of virtio-win block driver names (eg. ["viostor"]) that
+we search until we come to the first [name ^ ".sys"] that
+we find, and that is the block driver which gets installed.
+
+This module contains a default priority list which should
+be suitable for most use cases. *)
+
 val inject_virtio_win_drivers : t -> Registry.t -> virtio_win_installed
 (** [inject_virtio_win_drivers t reg]
 installs virtio drivers from the driver directory or driver
-- 
2.39.2

___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs