Libguestfs - Apr 2019 - [PATCH v2v v2 1/2] v2v: windows: Add a helper function for installing Powershell firstboot scripts.

Essentially identical to v1 except that it now uses (overloads?)
the --mac option to supply this data.

Rich.

---
 v2v/Makefile.am |  1 +
 v2v/windows.ml  | 24 ++++++++++++++++++++++++
 v2v/windows.mli |  6 ++++++
 3 files changed, 31 insertions(+)

diff --git a/v2v/Makefile.am b/v2v/Makefile.am
index 147ada4dd..e7f543a06 100644
--- a/v2v/Makefile.am
+++ b/v2v/Makefile.am
@@ -667,6 +667,7 @@ check_PROGRAMS += v2v_unit_tests var_expander_tests
 endif
 
 v2v_unit_tests_BOBJECTS = \
+	$(top_builddir)/customize/firstboot.cmo \
 	types.cmo \
 	uefi.cmo \
 	utils.cmo \
diff --git a/v2v/windows.ml b/v2v/windows.ml
index 23d589b00..dde64e677 100644
--- a/v2v/windows.ml
+++ b/v2v/windows.ml
@@ -19,6 +19,7 @@
 open Printf
 
 open Common_gettext.Gettext
+open Std_utils
 open Tools_utils
 
 open Utils
@@ -45,3 +46,26 @@ and check_app { Guestfs.app2_name = name;
   publisher =~ rex_avg_tech
 
 and (=~) str rex = PCRE.matches rex str
+
+(* Unfortunately Powershell scripts cannot be directly executed
+ * (unless some system config changes are made which for other
+ * reasons we don't want to do) and so we have to run this via
+ * a regular batch file.
+ *)
+let install_firstboot_powershell g { Types.i_windows_systemroot; i_root }
+                                 filename code +  let tempdir = sprintf
"%s/Temp" i_windows_systemroot in
+  g#mkdir_p tempdir;
+  let code = String.concat "\r\n" code ^ "\r\n" in
+  g#write (sprintf "%s/%s" tempdir filename) code;
+
+  (* Powershell interpreter.  Should we check this exists? XXX *)
+  let ps_exe +    i_windows_systemroot ^
+    "\\System32\\WindowsPowerShell\\v1.0\\powershell.exe" in
+
+  (* Windows path to the Powershell script. *)
+  let ps_path = i_windows_systemroot ^ "\\Temp\\" ^ filename in
+
+  let fb = sprintf "%s -ExecutionPolicy ByPass -file %s" ps_exe
ps_path in
+  Firstboot.add_firstboot_script g i_root filename fb
diff --git a/v2v/windows.mli b/v2v/windows.mli
index 016ef2a78..6db7874b0 100644
--- a/v2v/windows.mli
+++ b/v2v/windows.mli
@@ -21,3 +21,9 @@
 val detect_antivirus : Types.inspect -> bool
 (** Return [true] if anti-virus (AV) software was detected in
     this Windows guest. *)
+
+val install_firstboot_powershell : Guestfs.guestfs -> Types.inspect ->
+                                   string -> string list -> unit
+(** [install_powershell_firstboot g inspect filename code] installs a
+    Powershell script (the lines of code) as a firstboot script in
+    the Windows VM. *)
-- 
2.20.1

For Linux the guest itself remembers the IP address associated with
each MAC address.  Thus it doesn't matter if the interface type
changes (ie. to virtio-net), because as long as we preserve the MAC
address the guest will use the same IP address or the same DHCP
configuration.

However on Windows this association is not maintained by MAC address.
In fact the MAC address isn't saved anywhere in the guest registry.
(It seems instead this is likely done through PCI device type and
address which we don't record at the moment and is almost impossible
to preserve.)  When a guest which doesn't use DHCP is migrated, the
guest sees the brand new virtio-net devices and doesn't know what to
do with them, and meanwhile the right static IPs are still associated
with the old and now-defunct interfaces in the registry.

We cannot collect the required information from within the guest.
However we can collect it outside the tool by some other means
(eg. using VMware Tools APIs) and present this information to virt-v2v
which then writes it into the Windows guest at firstboot time.

This commit adds the --mac ..:ip:.. sub-option which creates a
Powershell script to set network adapters at firstboot.  An option
such as:

  --mac 00:0c:29:e6:3d:9d:ip:192.168.0.89,192.168.0.1,24,192.168.0.254

approximately turns into this script:

  # Wait for the netkvm (virtio-net) driver to become active.
  $adapters = @()
  While (-Not $adapters) {
      Start-Sleep -Seconds 5
      $adapters = Get-NetAdapter -Physical |
                     Where DriverFileName -eq "netkvm.sys"
  }
  $mac_address = '00-0c-29-e6-3d-9d'
  $ifindex = (Get-NetAdapter -Physical |
                 Where MacAddress -eq $mac_address).ifIndex
  if ($ifindex) {
      New-NetIPAddress -InterfaceIndex $ifindex
                       -IPAddress '192.168.0.89'
                       -DefaultGateway '192.168.0.1'
                       -PrefixLength 24
      Set-DnsClientServerAddress -InterfaceIndex $ifindex
                       -ServerAddresses ('192.168.0.254')
  }

Thanks: Brett Thurber for diagnosing the problem and suggesting paths
towards a fix.
---
 v2v/cmdline.ml         | 41 ++++++++++++++++++-----
 v2v/cmdline.mli        |  1 +
 v2v/convert_linux.ml   |  2 +-
 v2v/convert_windows.ml | 75 +++++++++++++++++++++++++++++++++++++++++-
 v2v/modules_list.ml    |  2 +-
 v2v/modules_list.mli   |  2 +-
 v2v/types.ml           |  8 +++++
 v2v/types.mli          |  9 +++++
 v2v/v2v.ml             |  7 ++--
 v2v/virt-v2v.pod       | 18 ++++++++++
 10 files changed, 149 insertions(+), 16 deletions(-)

diff --git a/v2v/cmdline.ml b/v2v/cmdline.ml
index 641eed017..82f9a7da6 100644
--- a/v2v/cmdline.ml
+++ b/v2v/cmdline.ml
@@ -41,11 +41,12 @@ type cmdline = {
   print_estimate : bool;
   print_source : bool;
   root_choice : root_choice;
+  static_ips : static_ip list;
   ks : Tools_utils.key_store;
 }
 
 (* Matches --mac command line parameters. *)
-let mac_re = PCRE.compile ~anchored:true
"([[:xdigit:]]{2}:[[:xdigit:]]{2}:[[:xdigit:]]{2}:[[:xdigit:]]{2}:[[:xdigit:]]{2}:[[:xdigit:]]{2}):(network|bridge):(.*)"
+let mac_re = PCRE.compile ~anchored:true
"([[:xdigit:]]{2}:[[:xdigit:]]{2}:[[:xdigit:]]{2}:[[:xdigit:]]{2}:[[:xdigit:]]{2}:[[:xdigit:]]{2}):(network|bridge|ip):(.*)"
 
 let parse_cmdline ()    let bandwidth = ref None in
@@ -100,6 +101,7 @@ let parse_cmdline ()    in
 
   let network_map = Networks.create () in
+  let static_ips = ref [] in
   let add_network str      match String.split ":" str with
     | "", "" ->
@@ -122,11 +124,30 @@ let parse_cmdline ()      if not (PCRE.matches mac_re str)
then
       error (f_"cannot parse --mac \"%s\" parameter") str;
     let mac = PCRE.sub 1 and out = PCRE.sub 3 in
-    let vnet_type -      match PCRE.sub 2 with
-      | "network" -> Network | "bridge" -> Bridge
-      | _ -> assert false in
-    Networks.add_mac network_map mac vnet_type out
+    match PCRE.sub 2 with
+    | "network" ->
+       Networks.add_mac network_map mac Network out
+    | "bridge" ->
+       Networks.add_mac network_map mac Bridge out
+    | "ip" ->
+       let add if_mac_addr if_ip_address if_default_gateway
+               if_prefix_length if_nameservers +         List.push_back
static_ips
+                        { if_mac_addr; if_ip_address; if_default_gateway;
+                          if_prefix_length; if_nameservers }
+       in
+       (match String.nsplit "," out with
+        | [] | [_] ->
+           error (f_"invalid --mac ip option")
+        | [mac; ip] -> add mac ip None None []
+        | [mac; ip; gw] -> add mac ip (Some gw) None []
+        | mac :: ip :: gw :: len :: nameservers ->
+           let len +             try int_of_string len with
+             | Failure _ -> error (f_"cannot parse --mac ip prefix
length field as an integer: %s") len in
+           add mac ip (Some gw) (Some len) nameservers
+       );
+    | _ -> assert false
   in
 
   let no_trim_warning _ @@ -218,8 +239,8 @@ let parse_cmdline ()               
s_"Input transport";
     [ L"in-place" ], Getopt.Set in_place,
                                     s_"Only tune the guest in the input
VM";
-    [ L"mac" ],      Getopt.String
("mac:network|bridge:out", add_mac),
-                                    s_"Map NIC to network or bridge";
+    [ L"mac" ],      Getopt.String
("mac:network|bridge|ip:out", add_mac),
+                                    s_"Map NIC to network or bridge or
assign static IP";
     [ S 'n'; L"network" ], Getopt.String ("in:out",
add_network),
                                     s_"Map network ‘in’ to ‘out’";
     [ L"no-copy" ],  Getopt.Clear do_copy,
@@ -347,6 +368,7 @@ read the man page virt-v2v(1).
   let print_source = !print_source in
   let qemu_boot = !qemu_boot in
   let root_choice = !root_choice in
+  let static_ips = !static_ips in
 
   (* No arguments and machine-readable mode?  Print out some facts
    * about what this binary supports.
@@ -364,6 +386,7 @@ read the man page virt-v2v(1).
     pr "io/oo\n";
     pr "mac-option\n";
     pr "bandwidth-option\n";
+    pr "mac-ip-option\n";
     List.iter (pr "input:%s\n") (Modules_list.input_modules ());
     List.iter (pr "output:%s\n") (Modules_list.output_modules ());
     List.iter (pr "convert:%s\n") (Modules_list.convert_modules ());
@@ -698,7 +721,7 @@ read the man page virt-v2v(1).
   {
     bandwidth; compressed; debug_overlays; do_copy; in_place;
     network_map; output_alloc; output_format; output_name;
-    print_estimate; print_source; root_choice;
+    print_estimate; print_source; root_choice; static_ips;
     ks = opthandle.ks;
   },
   input, output
diff --git a/v2v/cmdline.mli b/v2v/cmdline.mli
index 1c9e6c258..e7a688359 100644
--- a/v2v/cmdline.mli
+++ b/v2v/cmdline.mli
@@ -31,6 +31,7 @@ type cmdline = {
   print_estimate : bool;
   print_source : bool;
   root_choice : Types.root_choice;
+  static_ips : Types.static_ip list;
   ks : Tools_utils.key_store;
 }
 
diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
index f9e811c8d..1ada36115 100644
--- a/v2v/convert_linux.ml
+++ b/v2v/convert_linux.ml
@@ -34,7 +34,7 @@ open Linux_kernels
 module G = Guestfs
 
 (* The conversion function. *)
-let convert (g : G.guestfs) inspect source output rcaps +let convert (g :
G.guestfs) inspect source output rcaps _   
(*----------------------------------------------------------------------*)
   (* Inspect the guest first.  We already did some basic inspection in
    * the common v2v.ml code, but that has to deal with generic guests
diff --git a/v2v/convert_windows.ml b/v2v/convert_windows.ml
index 7ea56592c..1c79c891b 100644
--- a/v2v/convert_windows.ml
+++ b/v2v/convert_windows.ml
@@ -38,7 +38,7 @@ module G = Guestfs
  * time the Windows VM is booted on KVM.
  *)
 
-let convert (g : G.guestfs) inspect source output rcaps +let convert (g :
G.guestfs) inspect source output rcaps static_ips   
(*----------------------------------------------------------------------*)
   (* Inspect the Windows guest. *)
 
@@ -228,6 +228,8 @@ let convert (g : G.guestfs) inspect source output rcaps     
Registry.with_hive_write g inspect.i_windows_software_hive
                              update_software_hive;
 
+    configure_network_interfaces net_driver;
+
     fix_ntfs_heads ();
 
     fix_win_esp ();
@@ -603,6 +605,77 @@ if errorlevel 3010 exit /b 0
     | None ->
        warning (f_"could not find registry key
HKLM\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion")
 
+  and configure_network_interfaces net_driver +    (* If we were asked to force
network interfaces to have particular
+     * static IP addresses then it is done here by installing a
+     * Powershell script which runs at boot.
+     *)
+    if static_ips <> [] then (
+      let psh_filename = "v2vnetcf.ps1" in
+      let psh = ref [] in
+      let add = List.push_back psh in
+
+      add "Set-PSDebug -Trace 1";
+      add "";
+
+      (* If virtio-net was added to the registry, we must wait for
+       * it to be installed at runtime.
+       *)
+      if net_driver = Virtio_net then (
+        add "# Wait for the netkvm (virtio-net) driver to become
active.";
+        add "$adapters = @()";
+        add "While (-Not $adapters) {";
+        add "    Start-Sleep -Seconds 5";
+        add "    $adapters = Get-NetAdapter -Physical | Where
DriverFileName -eq \"netkvm.sys\"";
+        add "    Write-Host \"adapters =
'$adapters'\"";
+        add "}";
+        add ""
+      );
+
+      List.iter (
+        fun { if_mac_addr; if_ip_address; if_default_gateway;
+              if_prefix_length; if_nameservers } ->
+          add (sprintf "$mac_address = '%s'"
+                       (String.replace if_mac_addr ":"
"-"));
+          add "$ifindex = (Get-NetAdapter -Physical | Where MacAddress -eq
$mac_address).ifIndex";
+          add "if ($ifindex) {";
+
+          add "    Write-Host \"setting IP address of adapter at
$ifindex\"";
+
+          (* New-NetIPAddress command *)
+          let args = ref [] in
+          List.push_back args "-InterfaceIndex";
+          List.push_back args "$ifindex";
+          List.push_back args "-IPAddress";
+          List.push_back args (sprintf "'%s'" if_ip_address);
+          (match if_default_gateway with
+           | None -> ()
+           | Some gw ->
+              List.push_back args "-DefaultGateway";
+              List.push_back args (sprintf "'%s'" gw)
+          );
+          (match if_prefix_length with
+           | None -> ()
+           | Some len ->
+              List.push_back args "-PrefixLength";
+              List.push_back args (string_of_int len)
+          );
+          let cmd1 = "New-NetIPAddress " ^ String.concat "
" !args in
+          add ("    " ^ cmd1);
+
+          (* Set-DnsClientServerAddress command *)
+          if if_nameservers <> [] then (
+            add (sprintf "    Set-DnsClientServerAddress -InterfaceIndex
$ifindex -ServerAddresses (%s)"
+                         (String.concat "," (List.map (sprintf
"'%s'") if_nameservers)))
+          );
+          add "}";
+          add ""
+      ) static_ips;
+
+      (* Install the Powershell script to run at firstboot. *)
+      Windows.install_firstboot_powershell g inspect psh_filename !psh
+    ) (* static_ips <> [] *)
+
   and fix_ntfs_heads ()      (* NTFS hardcodes the number of heads on the drive
which created
        it in the filesystem header. Modern versions of Windows
diff --git a/v2v/modules_list.ml b/v2v/modules_list.ml
index a0a74aaf2..76b3def5d 100644
--- a/v2v/modules_list.ml
+++ b/v2v/modules_list.ml
@@ -38,7 +38,7 @@ type inspection_fn = Types.inspect -> bool
 
 type conversion_fn    Guestfs.guestfs -> Types.inspect -> Types.source
-> Types.output_settings ->
-  Types.requested_guestcaps -> Types.guestcaps
+  Types.requested_guestcaps -> Types.static_ip list -> Types.guestcaps
 
 let convert_modules = ref []
 
diff --git a/v2v/modules_list.mli b/v2v/modules_list.mli
index 3e80d3e23..ad2024755 100644
--- a/v2v/modules_list.mli
+++ b/v2v/modules_list.mli
@@ -34,7 +34,7 @@ type inspection_fn = Types.inspect -> bool
 
 type conversion_fn    Guestfs.guestfs -> Types.inspect -> Types.source
-> Types.output_settings ->
-  Types.requested_guestcaps -> Types.guestcaps
+  Types.requested_guestcaps -> Types.static_ip list -> Types.guestcaps
 
 val register_convert_module : inspection_fn -> string -> conversion_fn
-> unit
 (** [register_convert_module inspect_fn name fn] registers a
diff --git a/v2v/types.ml b/v2v/types.ml
index 0dac7cc65..ad448b66e 100644
--- a/v2v/types.ml
+++ b/v2v/types.ml
@@ -510,6 +510,14 @@ type bandwidth  | StaticBandwidth of string
 | DynamicBandwidth of string option * string
 
+type static_ip = {
+  if_mac_addr : string;
+  if_ip_address : string;
+  if_default_gateway : string option;
+  if_prefix_length : int option;
+  if_nameservers : string list;
+}
+
 class virtual input = object
   method precheck () = ()
   method virtual as_options : string
diff --git a/v2v/types.mli b/v2v/types.mli
index 1425fe3cf..7be6d1d17 100644
--- a/v2v/types.mli
+++ b/v2v/types.mli
@@ -366,6 +366,15 @@ type bandwidth  | DynamicBandwidth of string option *
string
 (** [--bandwidth] and [--bandwidth-file] options. *)
 
+type static_ip = {
+  if_mac_addr : string;
+  if_ip_address : string;
+  if_default_gateway : string option;
+  if_prefix_length : int option;
+  if_nameservers : string list;
+}
+(** [--mac ..:ip:..] option. *)
+
 (** {2 Input object}
 
     This is subclassed for the various input [-i] options.
diff --git a/v2v/v2v.ml b/v2v/v2v.ml
index 5fb85bb38..a748ae162 100644
--- a/v2v/v2v.ml
+++ b/v2v/v2v.ml
@@ -133,7 +133,7 @@ let rec main ()        | In_place ->
          rcaps_from_source source in
 
-    do_convert g inspect source output rcaps in
+    do_convert g inspect source output rcaps cmdline.static_ips in
 
   g#umount_all ();
 
@@ -557,7 +557,7 @@ and estimate_target_size mpstats overlays    )
 
 (* Conversion. *)
-and do_convert g inspect source output rcaps +and do_convert g inspect source
output rcaps interfaces    (match inspect.i_product_name with
   | "unknown" ->
     message (f_"Converting the guest to run on KVM")
@@ -573,7 +573,8 @@ and do_convert g inspect source output rcaps    debug
"picked conversion module %s" conversion_name;
   debug "requested caps: %s" (string_of_requested_guestcaps rcaps);
   let guestcaps -    convert g inspect source (output :>
Types.output_settings) rcaps in
+    convert g inspect source (output :> Types.output_settings) rcaps
+            interfaces in
   debug "%s" (string_of_guestcaps guestcaps);
 
   (* Did we manage to install virtio drivers? *)
diff --git a/v2v/virt-v2v.pod b/v2v/virt-v2v.pod
index 8ba141be9..6d677a456 100644
--- a/v2v/virt-v2v.pod
+++ b/v2v/virt-v2v.pod
@@ -412,6 +412,24 @@ Map source NIC MAC address to a network or bridge.
 
 See L</Networks and bridges> below.
 
+=item B<--mac>
aa:bb:cc:dd:ee:ffB<:ip:>ipaddr[,gw[,len[,ns,ns,...]]]
+
+Force a particular interface (controlled by its MAC address) to have a
+static IP address after boot.
+
+The fields in the parameter are: C<ipaddr> is the IP address. 
C<gw>
+is the optional gateway IP address.  C<len> is the subnet mask length
+(an integer).  The final parameters are zero or more nameserver IP
+addresses.
+
+This option can be supplied zero or more times.
+
+You only need to use this option for certain broken guests such as
+Windows which are unable to preserve MAC to static IP address mappings
+automatically.  You don't need to use it if Windows is using DHCP.  It
+is currently ignored for Linux guests since they do not have this
+problem.
+
 =item B<--machine-readable>
 
 =item B<--machine-readable>=format
-- 
2.20.1

On Tue, Apr 16, 2019 at 09:55:38AM +0100, Richard W.M. Jones
wrote:
> Essentially identical to v1 except that it now uses (overloads?)
> the --mac option to supply this data.
There's a small parsing problem in v2 which I discovered when I
started the test.  Fixed by applying the attached patch on top.

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

On Tuesday, 16 April 2019 10:55:39 CEST Richard W.M. Jones
wrote:
> +(* Unfortunately Powershell scripts cannot be directly executed
> + * (unless some system config changes are made which for other
> + * reasons we don't want to do) and so we have to run this via
> + * a regular batch file.
> + *)
> +let install_firstboot_powershell g { Types.i_windows_systemroot; i_root }
> +                                 filename code > +  let tempdir =
sprintf "%s/Temp" i_windows_systemroot in
> +  g#mkdir_p tempdir;
> +  let code = String.concat "\r\n" code ^ "\r\n" in
The Firstboot module uses String.unix2dos to convert the script to the
proper newline style for Windows.  What about assuming that 'code' is a
single Unix-style string with the whole script, calling String.unix2dos,
instead of taking the list of lines?  Mostly for consistency, so
everything that writes files on Windows uses String.unix2dos.
> +  (* Powershell interpreter.  Should we check this exists? XXX *)
> +  let ps_exe > +    i_windows_systemroot ^
> +    "\\System32\\WindowsPowerShell\\v1.0\\powershell.exe" in
According to the Interweb, it looks like this location was kept even
for PowerShell 2.0+ for "compatibility".

OTOH the comment raises another concern: what about old Windows versions
without PowerShell?  Most probably virt-v2v needs to report that to the
user (a warning, perhaps) so they know that the static IPs (specified
on the command line) cannot be preserved anyway.  Otherwise, the
conversion will work fine, and the user realize that this did not work
only when booting the converted Windows.

Maybe add a simple is_powershell_available helper to this (Windows)
module?

-- 
Pino Toscano

On Tuesday, 16 April 2019 10:55:40 CEST Richard W.M. Jones
wrote:
> For Linux the guest itself remembers the IP address associated with
> each MAC address.  Thus it doesn't matter if the interface type
> changes (ie. to virtio-net), because as long as we preserve the MAC
> address the guest will use the same IP address or the same DHCP
> configuration.
> 
> However on Windows this association is not maintained by MAC address.
> In fact the MAC address isn't saved anywhere in the guest registry.
> (It seems instead this is likely done through PCI device type and
> address which we don't record at the moment and is almost impossible
> to preserve.)  When a guest which doesn't use DHCP is migrated, the
> guest sees the brand new virtio-net devices and doesn't know what to
> do with them, and meanwhile the right static IPs are still associated
> with the old and now-defunct interfaces in the registry.
> 
> We cannot collect the required information from within the guest.
> However we can collect it outside the tool by some other means
> (eg. using VMware Tools APIs) and present this information to virt-v2v
> which then writes it into the Windows guest at firstboot time.
> 
> This commit adds the --mac ..:ip:.. sub-option which creates a
> Powershell script to set network adapters at firstboot.  An option
> such as:
> 
>   --mac 00:0c:29:e6:3d:9d:ip:192.168.0.89,192.168.0.1,24,192.168.0.254
> 
> approximately turns into this script:
> 
>   # Wait for the netkvm (virtio-net) driver to become active.
>   $adapters = @()
>   While (-Not $adapters) {
>       Start-Sleep -Seconds 5
>       $adapters = Get-NetAdapter -Physical |
>                      Where DriverFileName -eq "netkvm.sys"
>   }
>   $mac_address = '00-0c-29-e6-3d-9d'
>   $ifindex = (Get-NetAdapter -Physical |
>                  Where MacAddress -eq $mac_address).ifIndex
>   if ($ifindex) {
>       New-NetIPAddress -InterfaceIndex $ifindex
>                        -IPAddress '192.168.0.89'
>                        -DefaultGateway '192.168.0.1'
>                        -PrefixLength 24
>       Set-DnsClientServerAddress -InterfaceIndex $ifindex
>                        -ServerAddresses ('192.168.0.254')
>   }
> 
> Thanks: Brett Thurber for diagnosing the problem and suggesting paths
> towards a fix.
> ---
I have no experience on the Windows-specific parts, so I can only
comment on the v2v integration.
> diff --git a/v2v/cmdline.ml b/v2v/cmdline.ml
> index 641eed017..82f9a7da6 100644
> --- a/v2v/cmdline.ml
> +++ b/v2v/cmdline.ml
> @@ -41,11 +41,12 @@ type cmdline = {
>    print_estimate : bool;
>    print_source : bool;
>    root_choice : root_choice;
> +  static_ips : static_ip list;
>    ks : Tools_utils.key_store;
>  }
Instead of a separate list (which needs to be passed around, etc), most
probably this can fit as part of the Network module, adding a new
element to the vnet_type type, e.g.:

and vnet_type = Bridge | Network | Static_ip of static_ip

Most probably the static IP specification string can be parsed in a new
Network.add_static_ip method, which would also take care to check for
duplicated MAC specifications.
> +  and configure_network_interfaces net_driver > +    (* If we were
asked to force network interfaces to have particular
> +     * static IP addresses then it is done here by installing a
> +     * Powershell script which runs at boot.
> +     *)
> +    if static_ips <> [] then (
> +      let psh_filename = "v2vnetcf.ps1" in
> +      let psh = ref [] in
> +      let add = List.push_back psh in
> +
> +      add "Set-PSDebug -Trace 1";
> +      add "";
I suggest adding a simple helper here:
  let sq = sprintf "'%s'" in
(sq = Single Quote)
> +
> +      (* If virtio-net was added to the registry, we must wait for
> +       * it to be installed at runtime.
> +       *)
> +      if net_driver = Virtio_net then (
> +        add "# Wait for the netkvm (virtio-net) driver to become
active.";
> +        add "$adapters = @()";
> +        add "While (-Not $adapters) {";
> +        add "    Start-Sleep -Seconds 5";
> +        add "    $adapters = Get-NetAdapter -Physical | Where
DriverFileName -eq \"netkvm.sys\"";
> +        add "    Write-Host \"adapters =
'$adapters'\"";
> +        add "}";
> +        add ""
> +      );
> +
> +      List.iter (
> +        fun { if_mac_addr; if_ip_address; if_default_gateway;
> +              if_prefix_length; if_nameservers } ->
> +          add (sprintf "$mac_address = '%s'"
> +                       (String.replace if_mac_addr ":"
"-"));
> +          add "$ifindex = (Get-NetAdapter -Physical | Where
MacAddress -eq $mac_address).ifIndex";
> +          add "if ($ifindex) {";
> +
> +          add "    Write-Host \"setting IP address of adapter at
$ifindex\"";
> +
> +          (* New-NetIPAddress command *)
> +          let args = ref [] in
> +          List.push_back args "-InterfaceIndex";
> +          List.push_back args "$ifindex";
> +          List.push_back args "-IPAddress";
> +          List.push_back args (sprintf "'%s'"
if_ip_address);
'sq' can be used here.
> +          (match if_default_gateway with
> +           | None -> ()
> +           | Some gw ->
> +              List.push_back args "-DefaultGateway";
> +              List.push_back args (sprintf "'%s'" gw)
'sq' can be used here.
> +          );
> +          (match if_prefix_length with
> +           | None -> ()
> +           | Some len ->
> +              List.push_back args "-PrefixLength";
> +              List.push_back args (string_of_int len)
> +          );
> +          let cmd1 = "New-NetIPAddress " ^ String.concat "
" !args in
> +          add ("    " ^ cmd1);
Most probably this can be:

         add (sprintf "    New-NetIPAddress %s" (String.concat "
" !args));

(just for consistency with the same pattern done few lines below this.)
> +
> +          (* Set-DnsClientServerAddress command *)
> +          if if_nameservers <> [] then (
> +            add (sprintf "    Set-DnsClientServerAddress
-InterfaceIndex $ifindex -ServerAddresses (%s)"
> +                         (String.concat "," (List.map (sprintf
"'%s'") if_nameservers)))
'sq' can be used here.
> +          );
> +          add "}";
> +          add ""
install_firstboot_powershell already adds a newline at the end, so most
probably this empty line can be removed.

-- 
Pino Toscano

On Tuesday, 16 April 2019 10:55:38 CEST Richard W.M. Jones
wrote:
> Essentially identical to v1 except that it now uses (overloads?)
> the --mac option to supply this data.
> 
> Rich.
Small nit common to both the patches: the name is "PowerShell" (not
"Powershell").

-- 
Pino Toscano