Libguestfs - Mar 2023 - [V2V PATCH 1/1] convert_windows: add firstboot script to install drivers with pnputil

On Wed, Mar 08, 2023 at 08:05:35PM +0200, Andrey Drobyshev
wrote:
> During conversion we copy the necessary drivers to the directory
> "%systemroot%\Drivers\Virtio", adding it to the DevicePath
registry
> value.  As documented in [1], this should be enough for Windows to find
> device drivers and successfully install them.
> 
> However, it doesn't always happen.  Commit 73e009c04 ("v2v:
windows:
> Document use of pnputil to install drivers.") describes such issues
with
> Win2k12R2.  I'm seeing the same problem with Win2k16 and netkvm.sys
> driver not being installed.
> 
> That same commit 73e009c04 suggests adding a firstboot script invoking
> pnputil at an early stage to install all the drivers we put into the
> drivers store.  So let's add such a script to make sure all the
> necessary drivers are installed.
> 
> [1]
https://learn.microsoft.com/en-us/windows-hardware/drivers/install/how-windows-selects-a-driver-for-a-device
> 
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev at virtuozzo.com>
> ---
>  convert/convert_windows.ml | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/convert/convert_windows.ml b/convert/convert_windows.ml
> index 6bc2343b..e15a5e62 100644
> --- a/convert/convert_windows.ml
> +++ b/convert/convert_windows.ml
> @@ -295,9 +295,11 @@ let convert (g : G.guestfs) _ inspect i_firmware
block_driver _ static_ips >      | Virt -> Virt
>  
>    and configure_firstboot () > -    (* Note that pnp_wait.exe must be
the first firstboot script as it
> -     * suppresses PnP for all following scripts.
> +    (* Run the firstboot script with pnputil.exe before the one with
> +     * pnp_wait.exe as the latter suppresses PnP for all following
scripts.
>       *)
> +    configure_pnputil_install ();
> +
>      let tool_path = virt_tools_data_dir () // "pnp_wait.exe" in
>      if Sys.file_exists tool_path then
>        configure_wait_pnp tool_path
> @@ -345,6 +347,16 @@ let convert (g : G.guestfs) _ inspect i_firmware
block_driver _ static_ips >                        strkey name value
>      | None -> sprintf "reg delete \"%s\" /v %s /f"
strkey name
>  
> +  and configure_pnputil_install () > +    let fb_script = "@echo
off\n\
> +                     \n\
> +                     echo Wait for VirtIO drivers to be installed\n\
> +                     %systemroot%\\Sysnative\\PnPutil -i -a \
> +                     %systemroot%\\Drivers\\Virtio\\*.inf
>\"%~dpn0.log\" 2>&1\
> +                    " in
> +    Firstboot.add_firstboot_script g inspect.i_root
> +      "pnputil install drivers" fb_script;
> +
I'm not sure I'm really qualified to comment on this, since Windows is
crazy.  I guess the worst this can do is fail to run, but that won't
stop anything else from happening.

Note that firstboot scripts already do logging, so I don't believe
writing to a separate log file is needed here.  All the output from
all firstboot scripts should go to
C:\Program Files\Guestfs\Firstboot\log.txt:

https://github.com/libguestfs/libguestfs-common/blob/7acf991a25b3fd625eb1ff1fbd8dc9fedf245942/mlcustomize/firstboot.ml#L276

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On 3/8/23 22:52, Richard W.M. Jones wrote:
> On Wed, Mar 08, 2023 at 08:05:35PM +0200, Andrey Drobyshev wrote:
>> During conversion we copy the necessary drivers to the directory
>> "%systemroot%\Drivers\Virtio", adding it to the DevicePath
registry
>> value.  As documented in [1], this should be enough for Windows to find
>> device drivers and successfully install them.
>>
>> However, it doesn't always happen.  Commit 73e009c04 ("v2v:
windows:
>> Document use of pnputil to install drivers.") describes such
issues with
>> Win2k12R2.  I'm seeing the same problem with Win2k16 and netkvm.sys
>> driver not being installed.
>>
>> That same commit 73e009c04 suggests adding a firstboot script invoking
>> pnputil at an early stage to install all the drivers we put into the
>> drivers store.  So let's add such a script to make sure all the
>> necessary drivers are installed.
>>
>> [1]
https://learn.microsoft.com/en-us/windows-hardware/drivers/install/how-windows-selects-a-driver-for-a-device
>>
>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev at
virtuozzo.com>
>> ---
>>  convert/convert_windows.ml | 16 ++++++++++++++--
>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/convert/convert_windows.ml b/convert/convert_windows.ml
>> index 6bc2343b..e15a5e62 100644
>> --- a/convert/convert_windows.ml
>> +++ b/convert/convert_windows.ml
>> @@ -295,9 +295,11 @@ let convert (g : G.guestfs) _ inspect i_firmware
block_driver _ static_ips >>      | Virt -> Virt
>>  
>>    and configure_firstboot () >> -    (* Note that pnp_wait.exe
must be the first firstboot script as it
>> -     * suppresses PnP for all following scripts.
>> +    (* Run the firstboot script with pnputil.exe before the one with
>> +     * pnp_wait.exe as the latter suppresses PnP for all following
scripts.
>>       *)
>> +    configure_pnputil_install ();
>> +
>>      let tool_path = virt_tools_data_dir () // "pnp_wait.exe"
in
>>      if Sys.file_exists tool_path then
>>        configure_wait_pnp tool_path
>> @@ -345,6 +347,16 @@ let convert (g : G.guestfs) _ inspect i_firmware
block_driver _ static_ips >>                        strkey name value
>>      | None -> sprintf "reg delete \"%s\" /v %s
/f" strkey name
>>  
>> +  and configure_pnputil_install () >> +    let fb_script =
"@echo off\n\
>> +                     \n\
>> +                     echo Wait for VirtIO drivers to be installed\n\
>> +                     %systemroot%\\Sysnative\\PnPutil -i -a \
>> +                     %systemroot%\\Drivers\\Virtio\\*.inf
>\"%~dpn0.log\" 2>&1\
>> +                    " in
>> +    Firstboot.add_firstboot_script g inspect.i_root
>> +      "pnputil install drivers" fb_script;
>> +
> 
> I'm not sure I'm really qualified to comment on this, since Windows
is
> crazy.  I guess the worst this can do is fail to run, but that won't
> stop anything else from happening.
> 
> Note that firstboot scripts already do logging, so I don't believe
> writing to a separate log file is needed here.  All the output from
> all firstboot scripts should go to
> C:\Program Files\Guestfs\Firstboot\log.txt:
> 
>
https://github.com/libguestfs/libguestfs-common/blob/7acf991a25b3fd625eb1ff1fbd8dc9fedf245942/mlcustomize/firstboot.ml#L276
Right, but apart from having the log common for all firstboot scripts,
some of them also utilise separate log files in scripts-done directory,
e.g.:

https://github.com/libguestfs/virt-v2v/blob/cb792fef27f/convert/convert_windows.ml#L381
https://github.com/libguestfs/virt-v2v/blob/cb792fef27f/convert/convert_windows.ml#L168

So I did the same considering that pnputil's output is relatively long.

All in all, if there're no other concerns, can we give this script a go?
> 
> Rich.
>