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

[Shuah Khan <skhan@linuxfoundation.org>]

On 3/11/26 20:17, Zongmin Zhou wrote:
> 
> On 2026/3/11 06:28, Shuah Khan wrote:
>> On 3/3/26 01:17, Zongmin Zhou wrote:
>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>
>>> Currently, usbip_generic_driver_open() doesn't verify that the required
>>> kernel module (usbip-host or usbip-vudc) is actually loaded.
>>> The function returns success even when no driver is present,
>>> leading to usbipd daemon run success without driver loaded.
>>
>> Doesn't usbip_generic_driver_open() try to refresh exported
>> device list and fail? It returns error if it can't find fetch
>> them.
>>
>> usbipd starts and the when usbip_host is loaded it works correctly.
>> Doesn't it?
> Actually, refresh_exported_devices() does not return an error
> when the driver is not loaded,it consistently returns 0.
> It only results in hdriver->ndevs being set to 0 if no exportable usbip devices are found.
> Consequently, if the driver is missing, usbipd will start successfully in silence,
> but subsequent usbip attach operations will fail.
> The lack of explicit error messages makes it difficult for users to troubleshoot the root cause.
> By adding a check to verify if the driver is loaded during the usbip daemon startup,
> we can prevent these silent exceptions and ensure users are alerted to
> the missing kernel module before they attempt to use the service.
>>>
>>> So add a check function to ensure usbip host driver has been loaded.
>>>
>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>> ---
>>>   tools/usb/usbip/libsrc/usbip_device_driver.c |  2 ++
>>>   tools/usb/usbip/libsrc/usbip_host_common.c   | 31 ++++++++++++++++++++
>>>   tools/usb/usbip/libsrc/usbip_host_common.h   |  2 ++
>>>   tools/usb/usbip/libsrc/usbip_host_driver.c   |  2 ++
>>>   4 files changed, 37 insertions(+)
>>>
>>> diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c b/tools/usb/usbip/libsrc/usbip_device_driver.c
>>> index 927a151fa9aa..6da000fab26d 100644
>>> --- a/tools/usb/usbip/libsrc/usbip_device_driver.c
>>> +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c
>>> @@ -147,6 +147,8 @@ static int usbip_device_driver_open(struct usbip_host_driver *hdriver)
>>>   struct usbip_host_driver device_driver = {
>>>       .edev_list = LIST_HEAD_INIT(device_driver.edev_list),
>>>       .udev_subsystem = "udc",
>>> +    .bus_type = "platform",
>>
>> Why are we adding this here - user-space shouldn't need to
>> know what kind of driver this is?
> The reason I added the bus_type and drv_name fields is to
> construct the specific sysfs paths required for the availability check.
> Although usbip-host and usbip-vudc share the same usbip_generic_driver_open() interface ,
> they operate on different bus types and have different driver names.
> These fields allow the generic open function to dynamically verify
> the correct driver path in sysfs based on the specific driver type being initialized.
> If you prefer not to expose these in the this structure,
> I'm happy to adjust further based on your suggestions.

Have you tried a simple system() e.g below:

if (system("/usr/bin/lsmod | grep usbip")) instead of adding
all of this code?

Take a look at other examples of driver checks in cpupower
check_msr_driver() for example.

thanks,
-- Shuah