Sry for the wrong operation, the comment: https://bugzilla.redhat.com/show_bug.cgi?id=1595525#c3 is not related to this bug.

Tracking this down. My very first finding is that turning off namespaces makes the issue disappear. This can be used as a workaround.

Also, I've found that this bug does reproduce on libvirt-3.9.0-14.el7_5.6.x86_64 if namespaces are turned on. Therefore I don't think this is a regression. Han, can you please confirm and if it is really so can we remove the regression keyword? Thanks.

Okay, so here are my findings:

qemu uses libusb which is initialized on the first attempt to lookup an USB device (in this case the first hostdev hotplug). During this init, libusb creates an internal cache of available devices and creates a thread to listen to udev events to keep the internal cache in sync. The list of devices is then updated only from that thread as the libusb init function is never called again (nor it should).

However, since on the first hotplug no device is available, the internal cache contains no entries and thus when we try to plug a real device in the second step, libusb tells to qemu "no such device" even though /dev/bus/usb/* entry is created by libvirt correctly.

In fact, the internal cache is accessible through hmp command and after I plug in non-existent USB device I get no output (meaning the cache is empty). However, if I plug in an existing device first, the cache is populated and I can see the following output:

virsh qemu-monitor-command --hmp fedora "info usbhost"
  Bus 1, Addr 4, Port 8, Speed 480 Mb/s
    Class ef: USB device 04f2:b541
  Bus 1, Addr 3, Port 7, Speed 12 Mb/s
    Class e0: USB device 8087:0a2b
  Bus 1, Addr 2, Port 6, Speed 12 Mb/s
    Class ff: USB device 138a:0090


So the problem is either in libusb or in the way qemu uses it. At any rate, it's not in libvirt. Switching over to qemu to further investigation. Having said that, my question in the previous comment might be wrong after all, perhaps this problem did not exist with older qemu/libusb.

> qemu uses libusb which is initialized on the first attempt to lookup an USB
> device (in this case the first hostdev hotplug). During this init, libusb
> creates an internal cache of available devices and creates a thread to
> listen to udev events to keep the internal cache in sync.

> So the problem is either in libusb or in the way qemu uses it.

> perhaps this problem did not exist with older qemu/libusb.

I think the cache and the udev thread where not present in older libusb versions, this is most likely where the problem comes from.

On hotplug libvirt adds the usb device nodes to the namespace (and also sets selinux labels) to allow qemu access it.  Any libusb device scan done before that will not see the device, so due to the caching libvirt thinks it isn't there even though it only had no permission to access it.

Hi all:
I test this in rhel7.6 host and qemu, didn't hit this issue, test steps please see followings:

1.host and guest version
kernel-3.10.0-940.el7.x86_64 & qemu-kvm-rhev-2.12.0-5.el7.x86_64


2.test steps
(1)on host, plug a usb device:
(host)# lsusb
Bus 001 Device 003: ID 04ca:0027 Lite-On Technology Corp. 

(2)boot up a guest, command like following:
/usr/libexec/qemu-kvm -M pc \
...
-device virtio-scsi-pci,id=scsi0 \
-drive file=/mnt/rhel-image/rhel7.6-940-scsi.qcow2,if=none,id=drive-scsi0-0-0-0 \
-device scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \
-device virtio-net-pci,mac=70:5a:0f:38:cc:1c,id=netdev1,vectors=4,netdev=net1 -netdev tap,id=net1,vhost=on \
-device qemu-xhci,id=controller \
-qmp tcp:0:4443,server,nowait \

(3)hotplug a non-existed usb-host device, and then unhotplug it:
qmp command:
{"execute":"human-monitor-command","arguments":{"command-line":"info usb"}} 
{"return": ""}
{"execute":"device_add","arguments":{"driver":"usb-host","id":"host-usbkbd","vendorid":"0x04ca","productid":"0x0028"}}
{"return": {}}
{"execute":"human-monitor-command","arguments":{"command-line":"info usb"}} 
{"return": "  Device 0.0, Port 1, Speed 1.5 Mb/s, Product USB Host Device, ID: host-usbkbd\r\n"}
{"execute":"device_del","arguments":{"id":"host-usbkbd"}}
{"timestamp": {"seconds": 1536312434, "microseconds": 788104}, "event": "DEVICE_DELETED", "data": {"device": "host-usbkbd", "path": "/machine/peripheral/host-usbkbd"}}
{"return": {}}
{"execute":"human-monitor-command","arguments":{"command-line":"info usb"}} 
{"return": ""}

and after hotplug and unhotplug, guest command get results:
[root@localhost ~]# dmesg | grep usb
[    0.250370] usbcore: registered new interface driver usbfs
[    0.250376] usbcore: registered new interface driver hub
[    0.250392] usbcore: registered new device driver usb
[    0.667032] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002
[    0.667033] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
[    0.667035] usb usb1: Product: xHCI Host Controller
[    0.667037] usb usb1: Manufacturer: Linux 3.10.0-940.el7.x86_64 xhci-hcd
[    0.667038] usb usb1: SerialNumber: 0000:00:05.0
[    0.667508] usb usb2: We don't know the algorithms for LPM for this host, disabling LPM.
[    0.667520] usb usb2: New USB device found, idVendor=1d6b, idProduct=0003
[    0.667522] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1
[    0.667523] usb usb2: Product: xHCI Host Controller
[    0.667525] usb usb2: Manufacturer: Linux 3.10.0-940.el7.x86_64 xhci-hcd
[    0.667526] usb usb2: SerialNumber: 0000:00:05.0
[    0.667871] usbcore: registered new interface driver usbserial_generic
[    0.667876] usbserial: USB Serial support registered for generic
[    0.671908] usbcore: registered new interface driver usbhid
[    0.671909] usbhid: USB HID core driver
[root@localhost ~]# lsusb
Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

(4)hotplug the existed usb-host device:
qmp command:
{"execute":"device_add","arguments":{"driver":"usb-host","id":"host-usbkbd","vendorid":"0x04ca","productid":"0x0027"}}
{"return": {}}
{"execute":"human-monitor-command","arguments":{"command-line":"info usb"}} 
{"return": "  Device 0.1, Port 2, Speed 1.5 Mb/s, Product USB Multimedia Keyboard, ID: host-usbkbd\r\n"}

and after hotplug, guest get results:
[root@localhost ~]# lsusb -t
/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 5000M
/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 480M
    |__ Port 2: Dev 2, If 0, Class=Human Interface Device, Driver=usbhid, 1.5M
    |__ Port 2: Dev 2, If 1, Class=Human Interface Device, Driver=usbhid, 1.5M

Notes: Human Interface Device is hotplugged usb-host keyboard


3.Actual Result
can hotplug the exit usb-host device successfully after attaching a non-existed  host usb


but I have one question here: 
why can hotplug a non-existed usb-host device successfully? I think this action should get failure because the device didn't exist on host. what's more, though qmp return successful after hotplug a non-existed device, in fact didn't find this device in guest through "lsusb " command

> but I have one question here: 
> why can hotplug a non-existed usb-host device successfully?

<source startupPolicy='optional'>

This makes non-existing devices being simply ignored.
Drop this line and you should get an error instead.

(In reply to Gerd Hoffmann from comment #10)
> > but I have one question here: 
> > why can hotplug a non-existed usb-host device successfully?
> 
> <source startupPolicy='optional'>
> 
> This makes non-existing devices being simply ignored.
> Drop this line and you should get an error instead.

Thanks, I got it.
is this a bug for libvirt?

(In reply to Li Xiaohui from comment #11)
> (In reply to Gerd Hoffmann from comment #10)
> > > but I have one question here: 
> > > why can hotplug a non-existed usb-host device successfully?
> > 
> > <source startupPolicy='optional'>
> > 
> > This makes non-existing devices being simply ignored.
> > Drop this line and you should get an error instead.
> 
> Thanks, I got it.
> is this a bug for libvirt?

No, it is intentional behavior.  Doesn't make that much sense for the hotplug case.  But when included in a guest xml config it makes sense to allow the guest to start even in case the host device is not present.

Created attachment 1482104 [details]
repro.tar.gz

I'm attaching a small "reproducer" which I wrote just to confirm that the bug does not lie in libvirt code. It sets up namespaces, CGroups, creates a fake /dev, etc. Just like libvirtd would. However, it is still missing something because it right now it works like charm. Anyway, do whatever you want with it.

libusb is the component for the old libusb-0.1 compat library qemu uses the new libusb-1.0 which is the libusb1 component, changing component.

libusbx is maintained by the spice team now, changing assignee.

I do not believe that this is a libusbx bug, as mentioned this only happens when using namespaces.

When using namespaces libvirt itself generates the necessary /dev/bus/usb nodes when it wants to allow qemu access to these nodes.

But libusb does not need access to these nodes to enumerate the devices, it only needs this when an app actually wants to open a device. Enumeration is fully done through sysfs.

Is sysfs mounted in the namespace qemu is running under when using namespaces ?

And what about udev, is udev running inside the namespace ?

In order to see newly plugged in devices after libusb_init, libusb needs:
1) sysfs mounted and accessible
2) udev running
3) udev delivering hotplug events to libusb when new devices are plugged in

I've the feeling that the libvirt namespacing code is not fulfilling 1 or more of these 3 conditions.

(In reply to Hans de Goede from comment #16)
> I do not believe that this is a libusbx bug, as mentioned this only happens
> when using namespaces.
> 
> When using namespaces libvirt itself generates the necessary /dev/bus/usb
> nodes when it wants to allow qemu access to these nodes.

As long as those nodes exists. I mean, libvirt does not create *all* files under  /dev/bus/usb, only those which are 'plugged' into the domain.

> 
> But libusb does not need access to these nodes to enumerate the devices, it
> only needs this when an app actually wants to open a device. Enumeration is
> fully done through sysfs.
> 

So we are okay on this front. Libvirt mount only /dev and doesn't touch other mount points, so /sys is the same as in parent namespace.

> Is sysfs mounted in the namespace qemu is running under when using
> namespaces ?

Yes. It's only /dev that is mounted freshly new.

> 
> And what about udev, is udev running inside the namespace ?

No. We specifically done the whole namespace feature to avoid having udev running because it messes up uid:gid owner and selinux label for some /dev entries out of the blue. And thus our domains lose access to files. So it's libvirt who creates the nodes when starting a domain or when hotplugging a device into a domain.

> 
> In order to see newly plugged in devices after libusb_init, libusb needs:
> 1) sysfs mounted and accessible
> 2) udev running
> 3) udev delivering hotplug events to libusb when new devices are plugged in

When libvirtd creates a new node inside the namespace it doesn't issue any udev event. That might be the problem. However, is there a way to disable this caching on libusb side? Even a dirty hack to see if we are onto something.

> 
> I've the feeling that the libvirt namespacing code is not fulfilling 1 or
> more of these 3 conditions.

Well, the thing is that later, when user is attaching an USB device that exists libusb is already initialized, the /dev/bus/usb/* entry exists but opening the file still fails.

What nodes does libvirt need to create other than /dev/bus/usb/ in order to make libusb happy?

jomurphy, what was the reason setting the rhel 7.7.0 flag to "-"?

Can you verify the build Jonathon has provided?

Hello Jonathon,
Could you please provide the scratch build again? The former one is expired...

With my upstream libusb head on I'm still far from convinced that this is a libusb bug. libusb works fine when not confined, if you want to confine things fine, but then it is up to the code doing the confining to make sure things do not break, rather then libusb needing to go out of its way because you are causing weird conditions to exist which should not exist.

libusb in RHEL is build with the *compile-time* option for udev support enabled, so it expects udev to be present and functional, full stop.

Jonathon, I'm a bit surprised this is working without udev at all, since libusb calls into libudev not just for hotplug support but also to get the list of devices, so somehow udev is still somewhat functional ??? Can you please attach the patch with your quick workaround hack for this here?

Also can someone please run the reproducer for this with LIBUSB_DEBUG=1 set and then collect qemu's stderr output and attach that here?

Ok, so I just installed libusbx-tests-examples on F31 (vurrently not available on RHEL) and run the listdevs examples, which basically just calls libusb_get_device_list() once and then prints the results, then I ran it twice:

0557:8021 (bus 1, device 8) path: 9.4
0409:005a (bus 1, device 6) path: 9
05e3:0610 (bus 1, device 5) path: 8
05e3:0610 (bus 1, device 4) path: 7
08bb:2704 (bus 1, device 3) path: 6
0a12:0001 (bus 1, device 9) path: 12
05e3:0608 (bus 1, device 12) path: 10.2
046d:c22d (bus 1, device 13) path: 10.1.1
0409:0059 (bus 1, device 10) path: 10.1
1a40:0101 (bus 1, device 7) path: 10
048d:1336 (bus 1, device 2) path: 1
1d6b:0002 (bus 1, device 1)
[hans@shalem ~]$ sudo rm /dev/bus/usb/00
001/ 002/ 
[hans@shalem ~]$ sudo rm -r /dev/bus/usb/00*
[hans@shalem ~]$ libusb-example-listdevs
05e3:0616 (bus 2, device 3) path: 4
05e3:0612 (bus 2, device 2) path: 1
1d6b:0003 (bus 2, device 1)
046d:c534 (bus 1, device 11) path: 9.4.1
0557:8021 (bus 1, device 8) path: 9.4
0409:005a (bus 1, device 6) path: 9
05e3:0610 (bus 1, device 5) path: 8
05e3:0610 (bus 1, device 4) path: 7
08bb:2704 (bus 1, device 3) path: 6
0a12:0001 (bus 1, device 9) path: 12
05e3:0608 (bus 1, device 12) path: 10.2
046d:c22d (bus 1, device 13) path: 10.1.1
0409:0059 (bus 1, device 10) path: 10.1
1a40:0101 (bus 1, device 7) path: 10
048d:1336 (bus 1, device 2) path: 1
1d6b:0002 (bus 1, device 1)
[hans@shalem ~]$ ls /dev/bus/usb/00*
ls: cannot access '/dev/bus/usb/00*': No such file or directory

So this shows that under normal circumstances libusb works fine even without the /dev/bus/usb/*/* nodes and some other part of the sandboxing libvirt is doing is breaking it. The LIBUSB_DEBUG=1 stderr logs may help. Also have you tried using strace ?

Anyways I still believe that this is not a libusb bug.

Oops copy and paste fail, the first run of libusb-example-listdevs is missing the first couple of lines...

* Investigation and proper fix pending.
* Regression flag set but based on comment #6 it might not be.

Proposing to 7.9, keeping in libusbx for now.

Han, could you please reply to comment #6 so we can remove/keep regression flag?

(In reply to Michal Privoznik from comment #6)
> Also, I've found that this bug does reproduce on
> libvirt-3.9.0-14.el7_5.6.x86_64 if namespaces are turned on. Therefore I
> don't think this is a regression. Han, can you please confirm and if it is
> really so can we remove the regression keyword? Thanks.

This indeed does not seem like a regression to me.

I tried agagin, not reproduced on: libvirt-3.9.0-14.el7.x86_64 qemu-kvm-rhev-2.10.0-21.el7_5.9.x86_64 libusbx-1.0.21-1.el7.x86_64

Remove regression tag.

(In reply to Han Han from comment #42)
> I tried agagin, not reproduced on: libvirt-3.9.0-14.el7.x86_64
Sorry. Reproduced. Delete not.
> qemu-kvm-rhev-2.10.0-21.el7_5.9.x86_64 libusbx-1.0.21-1.el7.x86_64
> 
> Remove regression tag.

Calling libusb_get_device_list() will make libusb check the udev socket for any new udev event. But if you do not allow it to communicate with udev then that will not help...

It seems to me that if you are running qemu, and thus libusb in a jail like environment you are better off doing USB device management outside of the container and passing a fd into the container for the USB device to pass through.

Recent versions of libusb have the new libusb_wrap_sys_device() API for this, this is e.g. also used by google-chrome for passing USB fds to isolated browser-processes for the possibility of accessing libusb devices from web-apps.

This seems like the sensible (and technical correct) solution if you want to make USB device pass through work with a jailed qemu process. Note I'm not sure if the libusb in RHEL is new enough, but we can rebase it as the API maintains backward compatibility.

(In reply to Hans de Goede from comment #49)
> Recent versions of libusb have the new libusb_wrap_sys_device() API for
> this, this is e.g. also used by google-chrome for passing USB fds to
> isolated browser-processes for the possibility of accessing libusb devices
> from web-apps.

This API is on v1.0.23 while RHEL 8.2 has v1.0.22 so, to keep the current history of the bug, I'll move this one to qemu on RHEL 8.3 and open a rebase bug for libusbx in RHEL 8.3 too.

Thanks all

Raw patch scetch:

+#if LIBUSB_API_VERSION >= 0x01000107
+    if (s->hostdevice) {
+        int fd;
+        s->needs_autoscan = false;
+        fd = qemu_open(s->hostdevice, O_RDWR);
+        if (fd < 0) {
+            error_setg_errno(errp, errno, "failed to open %s", s->hostdevice);
+            return;
+        }
+        rc = libusb_wrap_sys_device(ctx, fd, &ldev);
+        if (rc < 0) {
+            error_setg(errp, "failed to wrap usb device %s", s->hostdevice);
+            close(fd);
+            return;
+        }
+        rc = usb_host_open(s, ldev);
+        libusb_unref_device(ldev);
+        if (rc < 0) {
+            error_setg(errp, "failed to open host usb device %s", s->hostdevice);
+            return;
+        }
+    } else
+#endif

How does that look like?

The documentation on libusb_wrap_sys_device() is rather slim ...

Specifically I wonder whenever the call transfers the file handle ownership to libusb or whenever the caller has to take care to close the file handle.  In case of the latter: is it ok to close the handle right away or must the caller wait until it doesn't want access the device any more?

(In reply to Gerd Hoffmann from comment #53)
> Raw patch scetch:
> 
> +#if LIBUSB_API_VERSION >= 0x01000107
> +    if (s->hostdevice) {
> +        int fd;
> +        s->needs_autoscan = false;
> +        fd = qemu_open(s->hostdevice, O_RDWR);
> +        if (fd < 0) {
> +            error_setg_errno(errp, errno, "failed to open %s",
> s->hostdevice);
> +            return;
> +        }
> +        rc = libusb_wrap_sys_device(ctx, fd, &ldev);
> +        if (rc < 0) {
> +            error_setg(errp, "failed to wrap usb device %s", s->hostdevice);
> +            close(fd);
> +            return;
> +        }
> +        rc = usb_host_open(s, ldev);
> +        libusb_unref_device(ldev);
> +        if (rc < 0) {
> +            error_setg(errp, "failed to open host usb device %s",
> s->hostdevice);
> +            return;
> +        }
> +    } else
> +#endif
> 
> How does that look like?

I'm afraid I've not been following libusb development much recently and libusb_wrap_device() was added after my active involvement. Anyways I've dived into the source-code of libusb to answer your questions. But please be aware that this info might not be 100% accurate.

libusb_wrap_device() returns (by reference) a 'libusb_device_handle *', where as libusb_unref_device() takes a 'libusb_device *'. The ldev you have here cannot be both, so I think you need to rework the code a bit.

Note libusb_wrap_device also creates a 'libusb_device *', you can get that by calling libusb_get_device() on the handle.

Also I assume that atm usb_host_open() does a libusb_open(). Calling libusb_open() on the libusb_device returned by calling libbusb_get_device() on the returned handle will not give you a handle to the wrapped_fd, instead it will try
to create a new handle and open the device itself. IOW you must use the returned handle and not call libusb_open() for using wrapped fds.

> The documentation on libusb_wrap_sys_device() is rather slim ...

Yeah, this was added by the google-chrome team for their own needs and since they wrote it they know how to used it, I guess your qemu changes will be the first user outside of google-chrome. If you could submit some patches to improve the docs that would be great.

> Specifically I wonder whenever the call transfers the file handle ownership
> to libusb or whenever the caller has to take care to close the file handle. 
> In case of the latter: is it ok to close the handle right away or must the
> caller wait until it doesn't want access the device any more?

I'm afraid the API is using what IMHO is the least ideal approach here, it does not take ownership, nor does it dup the fd. It simply copies the integer value into an internal integer. So not only does it not take ownership, you must also keep the fd open until you do not want to access the device any more.  The linux backend no even has a special internal flag to track that it should not close the fd which gets set from libusb_wrap_sys_device()...

https://patchwork.ozlabs.org/project/qemu-devel/patch/20200605125952.13113-1-kraxel@redhat.com/

upstream commit 9f815e83e983d247a3cd67579d2d9c1765adc644 (qemu 5.1)

I use libvirt directly to verify this bug, the test steps are the same as described in comment 0.

1. It is reproducible with libvirt-6.0.0-27.module+el8.3.0+7602+4b93512e.x86_64 and qemu-kvm-5.0.0-0.module+el8.3.0+7323+d54bb644.wrb200701.x86_64.

2. It is still reproduced with libvirt-6.0.0-27.module+el8.3.0+7602+4b93512e.x86_64 and qemu-kvm-5.1.0-3.module+el8.3.0+7708+740a1315.x86_64.

(In reply to yduan from comment #63)
> I use libvirt directly to verify this bug, the test steps are the same as
> described in comment 0.
> 
> 1. It is reproducible with
> libvirt-6.0.0-27.module+el8.3.0+7602+4b93512e.x86_64 and
> qemu-kvm-5.0.0-0.module+el8.3.0+7323+d54bb644.wrb200701.x86_64.
> 
> 2. It is still reproduced with
> libvirt-6.0.0-27.module+el8.3.0+7602+4b93512e.x86_64 and
> qemu-kvm-5.1.0-3.module+el8.3.0+7708+740a1315.x86_64.

Oh, well, b/c this adds a new feature to qemu libvirt needs changes too to actually use the new feature.
I think I've already created a bug fix this, but can't find it right now ...

This is the qemu fix:

commit 9f815e83e983d247a3cd67579d2d9c1765adc644
Author: Gerd Hoffmann <kraxel>
Date:   Fri Jun 5 14:59:52 2020 +0200

    usb: add hostdevice property to usb-host
    
    The new property allows to specify usb host device name.  Uses standard
    qemu_open(), so both file system path (/dev/bus/usb/$bus/$dev on linux)
    and file descriptor passing can be used.
    
    Requires libusb 1.0.23 or newer.  The hostdevice property is only
    present in case qemu is compiled against a new enough library version,
    so the presence of the property can be used for feature detection.
    
    Signed-off-by: Gerd Hoffmann <kraxel>
    Message-Id: <20200605125952.13113-1-kraxel>

> I think I've already created a bug fix this, but can't find it right now ...

Created bug 1877218 for libvirt.

Done, removed from errata.

I think it's just the wrong version, but I will check with @mrezanin

Changed to VERIFIED according to comment 79.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (virt:av bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2021:2098