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

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-wi... >> >> Signed-off-by: Andrey Drobyshev <andrey.drobyshev(a)virtuozzo.com&gt; >> --- >> 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/7acf991a25b3fd625eb1... 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_w... https://github.com/libguestfs/virt-v2v/blob/cb792fef27f/convert/convert_w... 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?