Re: [PATCH] usb-host: enable autoscan for bus+addr to survive host suspend/resume

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Apr 16, 2025 at 07:19:29PM +0300, Yuri Nesterov wrote:
> Currently, there is a special case for usb-host devices added using the
> hostbus= and hostaddr= properties to avoid adding them to the hotplug
> watchlist, since the address changes every time the device is plugged
> in. However, on Linux, when the host system goes into suspend and then
> resumes, those devices stop working in both the guest and the host.
> 
> Enabling autoscan and adding those devices to the watchlist allows them
> to keep working in the guest after host suspend/resume.

So IIUC what you're saying is that on suspend the host device
is removed by the kernel, and on resume, the USB device is
recreated. So QEMU's open file handle for the USB device is
invalid after resume.

If the /dev/bus/usb/NNN/NNN file goes away and then gets
re-created by the kernel though, we can't assume QEMU is
going to be able to re-open the new /dev/bus/usb device
file though.

When QEMU runs under libvirt, QEMU won't have any access
to the /dev/bus/usb device node unless libvirt has set the
right permissions and (where appropriate) also set the
SELinux label.

The current autoscan logic seems rather crude. AFAIK every 2
seconds it will re-scan every host USB device to see if any
has gone away and close it, and/or re-open if re-appeared.

If we enable this autoscan logic, then under libvirt, AFAICS,
QEMU is going to fail to re-open the device, and a counter in
the autoscan logic means that after 3 attempts to re-open QEMU
will stop attempting it at all...but strangely it appears QEMU
will keep the timer running, so every 2 seconds it will iterate
over every USB device, but never try to open any of them.

Regardless of your current patch this autoscan logic feels like
it needs improvement. We shouldn't need to busy-poll to see that
a USB device goes away, it should be possible to rely on event
notifications in some way.

Even then we'd still need a way to prevent this immediate auto
re-opening when under libvirt. Libvirt would have to detect the
reappearance of the device and perform relabelling and permissions
changes, before QEMU is able to be told to try to re-open.
Potentially this implies a new QMP command to tell QEMU to try
to re-open, unless perhaps QEMU can be triggered off an inotify
event for the permissions/label change of the device node.

> 
> Signed-off-by: Yuri Nesterov <yuri.nesterov@gmail.com>
> ---
>  hw/usb/host-libusb.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
> index c3d642c9d3..32c0251471 100644
> --- a/hw/usb/host-libusb.c
> +++ b/hw/usb/host-libusb.c
> @@ -1227,7 +1227,7 @@ static void usb_host_realize(USBDevice *udev, Error **errp)
>          !s->match.vendor_id &&
>          !s->match.product_id &&
>          !s->match.port) {
> -        s->needs_autoscan = false;
> +        s->needs_autoscan = true;
>          ldev = usb_host_find_ref(s->match.bus_num,
>                                   s->match.addr);
>          if (!ldev) {
> @@ -1244,6 +1244,9 @@ static void usb_host_realize(USBDevice *udev, Error **errp)
>          }
>      } else {
>          s->needs_autoscan = true;
> +    }
> +
> +    if (s->needs_autoscan) {
>          QTAILQ_INSERT_TAIL(&hostdevs, s, next);
>          usb_host_auto_check(NULL);
>      }
> -- 
> 2.43.0
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|