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

Zongmin Zhou
2026-03-12T02:18:14Z

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.
>
>> +    .drv_name = USBIP_DEVICE_DRV_NAME,
>
> And the name?
>
>>       .ops = {
>>           .open = usbip_device_driver_open,
>>           .close = usbip_generic_driver_close,
>> diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c
>> b/tools/usb/usbip/libsrc/usbip_host_common.c
>> index ca78aa368476..900370095b61 100644
>> --- a/tools/usb/usbip/libsrc/usbip_host_common.c
>> +++ b/tools/usb/usbip/libsrc/usbip_host_common.c
>> @@ -164,6 +164,31 @@ static void usbip_exported_device_destroy(struct
>> list_head *devs)
>>       }
>>   }
>>   +/* Check if the usbip host driver is available in sysfs */
>> +static int check_driver_available(struct usbip_host_driver *hdriver)
>> +{
>> +    char driver_path[SYSFS_PATH_MAX];
>> +    struct stat st;
>> +    int ret;
>> +
>> +    if (!hdriver->drv_name || !hdriver->bus_type)
>> +        return 0;
>> +
>> +    //Check if the usbip-host or usbip-vudc driver directory exists
>> in sysfs.
>> +    snprintf(driver_path, sizeof(driver_path), "%s/%s/%s/%s/%s",
>> +        SYSFS_MNT_PATH, SYSFS_BUS_NAME, hdriver->bus_type,
>> +        SYSFS_DRIVERS_NAME, hdriver->drv_name);
>> +
>> +    ret = stat(driver_path, &st);
>> +    if (ret == 0 && S_ISDIR(st.st_mode)) {
>> +        dbg("driver '%s' is available", hdriver->drv_name);
>> +        return 1;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +
>>   int usbip_generic_driver_open(struct usbip_host_driver *hdriver)
>>   {
>>       int rc;
>> @@ -174,6 +199,12 @@ int usbip_generic_driver_open(struct
>> usbip_host_driver *hdriver)
>>           return -1;
>>       }
>>   +    //Check if the required driver is actually available.
>> +    if (!check_driver_available(hdriver)) {
>> +        err("please load '%s' kernel module", hdriver->drv_name);
>> +        goto err;
>> +    }
>> +
>>       rc = refresh_exported_devices(hdriver);
>>       if (rc < 0)
>>           goto err;
>> diff --git a/tools/usb/usbip/libsrc/usbip_host_common.h
>> b/tools/usb/usbip/libsrc/usbip_host_common.h
>> index f46967c0aa18..cf9bd60846cf 100644
>> --- a/tools/usb/usbip/libsrc/usbip_host_common.h
>> +++ b/tools/usb/usbip/libsrc/usbip_host_common.h
>> @@ -40,6 +40,8 @@ struct usbip_host_driver {
>>       /* list of exported device */
>>       struct list_head edev_list;
>>       const char *udev_subsystem;
>> +    const char *bus_type;
>> +    const char *drv_name;
>>       struct usbip_host_driver_ops ops;
>>   };
>>   diff --git a/tools/usb/usbip/libsrc/usbip_host_driver.c
>> b/tools/usb/usbip/libsrc/usbip_host_driver.c
>> index 573e73ec36bd..e8f9d2ee2497 100644
>> --- a/tools/usb/usbip/libsrc/usbip_host_driver.c
>> +++ b/tools/usb/usbip/libsrc/usbip_host_driver.c
>> @@ -41,6 +41,8 @@ static int usbip_host_driver_open(struct
>> usbip_host_driver *hdriver)
>>   struct usbip_host_driver host_driver = {
>>       .edev_list = LIST_HEAD_INIT(host_driver.edev_list),
>>       .udev_subsystem = "usb",
>> +    .bus_type = "usb",
>> +    .drv_name = USBIP_HOST_DRV_NAME,
>
>>       .ops = {
>>           .open = usbip_host_driver_open,
>>           .close = usbip_generic_driver_close,
>
> I need more to justify these changes.
>
> thanks,
> -- Shuah