On 3/13/23 10:57, Andrey Drobyshev wrote:
> On 3/13/23 11:28, Laszlo Ersek wrote:
>> On 3/9/23 14: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 <andrey.drobys...@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.
>>>
>>> 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.
>>>
>>>> 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.
>>
>> I'm surprised by this patch (which doesn't say much of course, because I
>> know precious little about Windows). What I'm noticing is "run
>> pnputil.exe before pnp_wait.exe before everything else".
>>
>> We do queue a script earlier than those, at priority 2500, called
>> "v2vnetcf.ps1". See configure_network_interfaces ->
>> add_firstboot_powershell, and the reference to RHBZ 1788823.
>>
>> And that script is exactly what tends *not* to work, and we've been
>> unable to get to the bottom of that issue. (See also RH 2068361 and
>> 1963032.)
>>
>> So... This patch could be orthogonal to that pre-existent symptom... but
>> could the patch perhaps *fix* the prior symptom? Is it possible that, if
>> we "forcibly install" the virtio drivers first, and *then* try to
>> configure IP addresses for the virtio-net NICs, then the latter will
>> start working reliably?
>
> Could it be that what you've seen with non-working v2vnetcf.ps1 solution
> is directly connected with the issue we're facing here, i.e. drivers
> from the driver store not being automatically installed?
Yes, this is very much possible.
However, I have no evidence. All our attempts at analyzing the
"v2vnetcf.ps1" failures have been futile.
The failures are non-deterministic. One thing I had found (earlier) was
that failure versus success depended on guest-side timing. When I slowed
down the host arbitrarily, thereby causing guest-side actions to take
longer, the "v2vnetcf.ps1" script started succeeding. So "v2vnetcf.ps1"
seems to be racing something, but we could never figure out what it was.
>
> Is so, we may try prioritizing pnputil even further to make sure that
> *ALL* the drivers (including netkvm.sys) are installed before anything
> else happens. For instance, assign it "~prio:2000".
>
> What do you think?
Yes, that's something worth exploring IMO; although I don't think I'll
be able to provide any evidence in the short or mid term as to whether
this "experiment" has the intended effect. Again, non-deterministic
behavior is non-deterministic.
I guess it's worth trying -- even if it doesn't help, nothing should be
harmed by assigning prio 2000 to the manual driver installation, I expect.
Thanks
Laszlo
>> I have no idea really.
>>
>> Anyway, once Rich is happy with the logging here, I have nothing against
>> the patch.
>>
>> Acked-by: Laszlo Ersek <ler...@redhat.com>
>
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
