Re: [PATCH v2] usbip: tools: Add usbip host driver availability check

[Shuah Khan <skhan@linuxfoundation.org>]

On 3/29/26 21:10, Zongmin Zhou wrote:
> 
> On 2026/3/28 02:29, Shuah Khan wrote:
>> On 3/27/26 11:51, Shuah Khan wrote:
>>> On 3/27/26 02:39, Zongmin Zhou wrote:
>>>>
>>>> On 2026/3/27 02:43, Shuah Khan wrote:
>>>>> On 3/26/26 02:43, Greg KH wrote:
>>>
>>> [removed text]
>>>
>>>>>
>>>>> The problem Zongmin is trying fix ish when usbipd starts, it looks for
>>>>> exported if any - if it doesn't find any it assumes there aren't any
>>>>> exported and doesn't detect that usbip_host driver isn't loaded.
>>>>>
>>>>> refresh_exported_devices() doesn't have the logic and it shouldn't
>>>>> include that logic because this hook is called periodically to
>>>>> refresh the list of exported devices. Starting usbipd and loading
>>>>> the driver are distinct steps and without any dependencies.
>>>>>
>>>>> This patch he is trying to add that detection so a message can be printed
>>>>> to say "load the driver".
>>>>>
>>>>> A message can be added easily to cover two cases:
>>>>>
>>>>> 1. usbip_host driver isn't loaded
>>>>> 2. There are no exported devices.
>>>>>
>>>>> refresh_exported_devices() will not find any devices in both
>>>>> of the above scenarios. It isn't an error if it can't find
>>>>> any exported devices.
>>>>>
>>>>> An informational message when refresh_exported_devices()
>>>>> when it doesn't find any devices could help users.
>>>>>
>>>>> Zongmin,
>>>>>
>>>>> Would adding a message that says
>>>>> "Check if usbip_host driver is loaded or export devices"
>>>>> solve the problem of hard to debug problem you are addressing here?
>>>>>
>>>> Shuah,
>>>>
>>>> Your suggestion makes sense.
>>>> Adding an informational message when no devices are found would be a simple
>>>> and clean solution that helps users debug without being intrusive.
>>>>
>>>> However, I plan to add the info() message in usbip_generic_driver_open() instead of
>>>> refresh_exported_devices(), because:
>>>> - usbip_generic_driver_open() is called only once at initialization.
>>>> - refresh_exported_devices() is called periodically to refresh the exported device list.
>>>
>>> refresh_exported_devices() isn't called periodically - it is called
>>> from usbip_host_driver op: refresh_device_list and it will be called
>>> whenever usbipd (host side) calls it whenever it receives a request from
>>> user-space via process_request()
>>>
>>> For example running "usbip list -l" command will trigger a run of
>>> refresh_exported_devices() via usbip_host_driver op: refresh_device_list
>>>
>>> I don't think it will that noisy to add a message to refresh_exported_devices()
>>> when device list is zero. Currently the logic doesn't detect device list zero.
>>> You have add that condition to print informational message.
>>>
>>>
>>>> - When the server has no exported devices, having zero devices
>>>>    is normal and not worth frequent info messages.
>>>
>>> That is correct. Zero exported devices isn't an error and this could
>>> persist until devices are exported.
>>>
>>>>
>>>> Theoretically, we only need to prompt once at startup. Is my understanding correct?> > I'll also remove the existing error messages like below,
>>>> since they cannot accurately determine whether the driver is loaded:
>>>>
>>>> if (ret)
>>>>      err("please load " USBIP_CORE_MOD_NAME ".ko and "
>>>>          USBIP_HOST_DRV_NAME ".ko!");
>>>
>>> Leave this one alone, because it gets called from a couple of places.
> In usbip_generic_driver_open(), the only path that triggers this message is a failure of udev_new().
> This function fails due to system-level issues like memory exhaustion, not because usbip driver module is missing.
> 
> Furthermore, since refresh_exported_devices() doesn't practically return an error here,
> the message is never seen during actual driver loading failures.
> So I think it’s better to remove this inaccurate hint to avoid confusing the users.
> 
> This is the reason why I previously wanted to remove it.
>>>
>>
>> Better yet, why not change the usbip instead - usbip_list for example.
>> This is the one that prints the device list and the change can be made
>> there when the list is zero to say, "Check if driver is loaded and
>> exported devices"
> I think placing the check/message in refresh_exported_devices() would be more effective.
> This function covers all scenarios where the device list is refreshed, including:
> usbipd startup, usbip list -r, and usbip attach operations.

Right changing refresh_exported_devices() covers all cases. Think about the
following scenarios:

- Where will this message get printed when usbipd is started on the console and in
   background?

- Where will this message get printed when user runs various usbip list commands?

Ideally these commands should tell the user what happened without looking
at daemon logs.

thanks,
-- Shuah