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

[Shuah Khan]

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@xxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxxxxxxxxxxx>
> > > Signed-off-by: Zongmin Zhou <zhouzongmin@xxxxxxxxxx>
> > > ---
> > > 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