On 3/9/23 15:48, Richard W.M. Jones wrote:
> On Thu, Mar 09, 2023 at 03:47:10PM +0200, Andrey Drobyshev wrote:
>> 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 <[email protected]>
>>>> ---
>>>> 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.
>
> I think I'd probably get rid of those other scripts too. It's helpful
> to have everything go to one place. I'm not actually sure where those
> extra files end up.
The common log is set to %%firstboot%%\\log.txt, with %%firstboot%%
being C:\Program Files\Guestfs\Firstboot:
https://github.com/libguestfs/libguestfs-common/blob/7acf991a25/mlcustomize/firstboot.ml#L285
The scripts are given a priority and a relative number, resulting in a
prefix like "5000-0001-":, and put into %%firstboot%%\\scripts:
https://github.com/libguestfs/libguestfs-common/blob/7acf991a25/mlcustomize/firstboot.ml#L356
The script is moved from scripts to scripts-done right before getting run:
https://github.com/libguestfs/libguestfs-common/blob/7acf991a25/mlcustomize/firstboot.ml#L302
And "%~dpn0" in the script evaluates to Drive+Path+Name of the script
itself (apparently, with no .bat extension):
https://stackoverflow.com/a/112135
This results in having both the script
5000-0001-pnputil-install-drivers.bat and the log
5000-0001-pnputil-install-drivers.log ending up in C:\Program
Files\Guestfs\Firstboot\scripts-done.
>
>> All in all, if there're no other concerns, can we give this script a go?
>
> I think the patch is generally fine, so I'm just quibbling about
> the logging.
Alright, then let me send v2 with the logging part fixed.
>
> Rich.
>
_______________________________________________
Libguestfs mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/libguestfs
