Re: [PATCH v3] usbip: tools: add hint when no exported devices are found

[Shuah Khan <skhan@linuxfoundation.org>]

On 3/31/26 20:47, Zongmin Zhou wrote:
> 
> On 2026/4/1 08:27, Shuah Khan wrote:
>> On 3/31/26 03:58, Zongmin Zhou wrote:
>>> From: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>>
>>> When refresh_exported_devices() finds no devices, it's helpful to
>>> inform users about potential causes. This could be due to:
>>>
>>> 1. The usbip driver module is not loaded.
>>> 2. No devices have been exported yet.
>>>
>>> Add an informational message to guide users when ndevs == 0.
>>>
>>> Message visibility by scenario:
>>> - usbipd (console mode): Show on console/serial, this allows instant
>>>    visibility for debugging.
>>> - usbipd -D (daemon mode): Message logged to syslog, can keep logs for
>>>    later traceability in production. Also can use "journalctl -f" to
>>>    trace on console.
>>>
>>> Suggested-by: Shuah Khan <skhan@linuxfoundation.org>
>>> Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
>>> ---
>>> Changes in v3:
>>> - Just add an informational message when no devices are found.
>>> Changes in v2:
>>> - Use system calls directly instead of checking sysfs dir.
>>>
>>>   tools/usb/usbip/libsrc/usbip_host_common.c | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c b/tools/usb/usbip/libsrc/usbip_host_common.c
>>> index ca78aa368476..cd92baee310c 100644
>>> --- a/tools/usb/usbip/libsrc/usbip_host_common.c
>>> +++ b/tools/usb/usbip/libsrc/usbip_host_common.c
>>> @@ -149,6 +149,13 @@ static int refresh_exported_devices(struct usbip_host_driver *hdriver)
>>>           }
>>>       }
>>>   +    if (hdriver->ndevs == 0) {
>>> +        if (!strcmp(hdriver->udev_subsystem, "usb"))
>>> +            info("Please check if %s driver is loaded or export devices.",USBIP_HOST_DRV_NAME);
>>
>> Check coding guidelines and match the new code to the existing. Need a space between
>> the string and the next argument.
>>
>>
>>> +        else
>>> +            info("Please check if %s driver is loaded or export devices.",USBIP_DEVICE_DRV_NAME);
>>
>> When will this be true? Isn't refresh_exported_devices() called from
>>
>> Thinking about this more, since you have to differentiate between
>> host and device, it makes sense to make this change to
>>
>> usbip_device_driver_open() and usbip_host_driver_open()
>> There is an err() message already in both of these routines.
>> that prints the right information.
>>
> Shuah,
> 
> Regarding your comment about the existing err() message in these routines:
> 
> The existing err("please load *.ko") message only triggers when usbip_generic_driver_open()
> returns a non-zero value. Since refresh_exported_devices() always returns 0
> (even when ndevs == 0 and no devices are found), usbip_generic_driver_open()
> only returns non-zero when udev_new() fails.
> 
> Therefore, when ndevs == 0, the existing err() message is never printed,
> so it doesn't actually help users diagnose the "no devices found or driver not load" situation.

Correct. There is no need to remove this. What I am suggesting is these
messages from usbip_device_driver_open() and usbip_host_driver_open()

What I am suggesting changing usbip_device_driver_open() and
usbip_host_driver_open() like this:

if (ret || hdriver->ndevs == 0)
                 info("please load " USBIP_CORE_MOD_NAME ".ko and "
                     USBIP_DEVICE_DRV_NAME ".ko!");


This should solve the problem for you ...

I don't like the idea of adding string compares to refresh_exported_devices()

if (!strcmp(hdriver->udev_subsystem, "usb"))

to differentiate when you can simply leverage the existing code by adding
one more conditional and changing the err() to info()

thanks,
-- Shuah