Dear Maintainer,

I have a custom udev rule used to implement usb-hotplug support for the a guest, whose name is dgw.

This is the contents of /etc/udev/rules.d/91-hotplug.rules:
---
ACTION=="add", \
    SUBSYSTEM=="usb", \
    ENV{ID_VENDOR_ID}=="18ec", \
    ENV{ID_MODEL_ID}=="3399", \
    RUN+="/usr/bin/virsh attach-device dgw /home/david/udev/david-webcam-dgw.xml"

ACTION=="remove", \
    SUBSYSTEM=="usb", \
    ENV{ID_VENDOR_ID}=="18ec", \
    ENV{ID_MODEL_ID}=="3399", \
    RUN+="/usr/bin/virsh detach-device dgw /home/david/udev/david-webcam-dgw.xml"
---

This is the content of david-webcam-dgw.xml:
---
<hostdev mode='subsystem' type='usb'>
  <source>
    <vendor id='0x18ec'/>
    <product id='0x3399'/>
  </source>
</hostdev>
---

The rule is correctly activated when the device is plugged in, but with udev logging set to debug level, the syslog trace is as follows:
---
Nov 26 14:48:27 dgh systemd-udevd[8692]: Process 'mtp-probe
/sys/devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3.1 1 67' succeeded.
Nov 26 14:48:27 dgh systemd-udevd[8692]: RUN '/usr/bin/virsh attach-device dgw /home/david/udev/david-webcam-dgw.xml' /etc/udev/rules.d/91-dg-webcam-dgw-hotplug.rules:10
Nov 26 14:48:27 dgh systemd-udevd[8692]: handling device node '/dev/bus/usb/001/067', devnum=c189:66, mode=0664, uid=0, gid=100 Nov 26 14:48:27 dgh systemd-udevd[8692]: set permissions /dev/bus/usb/001/067, 020664, uid=0, gid=100 Nov 26 14:48:27 dgh systemd-udevd[8692]:
creating symlink '/dev/char/189:66' to '../bus/usb/001/067'
Nov 26 14:48:27 dgh systemd-udevd[8692]: created db file '/run/udev/data/c189:66' for '/devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3.1'
Nov 26 14:48:27 dgh systemd-udevd[8694]: starting '/usr/bin/virsh attach-device dgw /home/david/udev/david-webcam-dgw.xml'
Nov 26 14:48:27 dgh libvirtd[3841]: internal error: unable to execute QEMU command 'device_add':
failed to find host usb device 1:67 Nov 26 14:48:27 dgh systemd-udevd[8692]: '/usr/bin/virsh attach- device dgw /home/david/udev/david-webcam-dgw.xml'(err) 'error: '
Nov 26 14:48:27 dgh systemd-udevd[8692]: '/usr/bin/virsh attach-device dgw /home/david/udev/david-
webcam-dgw.xml'(err) 'Failed to attach device from /home/david/udev/david-webcam-dgw.xml'
Nov 26 14:48:27 dgh systemd-udevd[8692]: '/usr/bin/virsh attach-device dgw /home/david/udev/david-
webcam-dgw.xml'(err) 'error: internal error: unable to execute QEMU command 'device_add': failed to find host usb device 1:67'
Nov 26 14:48:27 dgh systemd-udevd[8692]: '/usr/bin/virsh attach-device dgw /home/david/udev/david-
webcam-dgw.xml'(out) ''
Nov 26 14:48:27 dgh systemd-udevd[8692]: Process '/usr/bin/virsh attach-device dgw /home/david/udev/david-webcam-dgw.xml' failed with exit code 1.
---

As you can see, the error is that QEMU device_add fails, complaining it cannot find the usb device that was indeed created.

Additional detail: The above log was generated after the addition of another rules file Thinking it might be a permission problem,I added another rules file 50-myusb.rules as follows:
---
SUBSYSTEMS=="usb", GROUP="users", MODE="0666"
---
but with or without this rules file the error reported is the same.

Intrestingly, the commands

virsh attach-devcice dgw /home/david/udev/david-webcam-dgw.xml
and
virsh detach-devcice dgw /home/david/udev/david-webcam-dgw.xml

both work and successfully attach the device to the guest if the device is connected - but the udev script fails.

USB hotplug on this host was working normally as recently as a couple of months ago; possibly a stretch update caused a regression. As of now, I am unable to make USB hotplug to guests work.

Thanks so much for looking into this.

I won't have time to look into this, sorry. I suggest to:

* set libvirt debugging to debug
* check which monitor commands get issued to attach the device

Create a new script run from the udev rule that

* checks the necessary device nodes are actually there
* uses the above monitor commands via "virsh qemu-monitor-command"

if it still fails it's within qemu if not there's something broken in
libvirt.

Cheers,
 -- Guido

Hi,

I did have some time and motivation to look into this, since we do this
for VM support in the bit-babbler (USB TRNG support) package too, and the
view that I have of it is that it's a libvirt bug, triggered by a change
in the qemu 2.7 release.  So I'm cloning a copy of it there too, since
we may want to back out that change until this is fixed properly in
libvirt.


The crux of the problem is that although QEMU gives us the option to
hotplug a USB device by vendor/product ID, or by hostbus+hostport
(the physical usb port on the host) or by hostbus+hostaddr (the logical
USB device number which changes each time it is plugged in or gets
re-enumerated) - libvirt will *always* pass it to QEMU using the
hostaddr that it determines, even if in its config you passed that as
vendor/product ID.  And libvirt gives us no option to use the hostport
at all.

This was somewhat workable (though suboptimal) in the case where you
might have multiple devices with the same product ID (a much better
solution there would be for libvirt to support vendor/product/serial
as a unique identifying tuple which never changes) - until this change
was made in QEMU 2.7:


 commit e058fa2dd599ccc780d334558be9c1d155222b80
 Author: Gerd Hoffmann <kraxel@redhat.com>
 Date:   Fri Jun 3 11:12:55 2016 +0200

    usb-host: add special case for bus+addr

    This patch changes usb-host behavior in case we hostbus= and hostaddr=
    properties are used to identify the usb device in question.  Instead of
    adding the device to the hotplug watchlist we try to open directly using
    the given bus number and device address.

    Putting a device specified by hostaddr to the hotplug watchlist isn't
    a great idea as the address isn't a fixed property.  It changes every
    time the device is plugged in.  So considering this case as "use the
    device at bus:addr _now_" is more sane.  Also usb-host will throw errors
    in case it can't initialize the host device.

    Note: For devices on the hotplug watchlist (hostport or vendorid or
    productid specified) qemu continues to ignore errors and keeps
    monitoring the usb bus to see if the device eventually shows up.

    Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
    Message-id: 1464945175-28939-1-git-send-email-kraxel@redhat.com

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 8b774f4..da59c29 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -81,6 +81,7 @@ struct USBHostDevice {
     uint32_t                         iso_urb_frames;
     uint32_t                         options;
     uint32_t                         loglevel;
+    bool                             needs_autoscan;

     /* state */
     QTAILQ_ENTRY(USBHostDevice)      next;
@@ -974,9 +975,32 @@ static void usb_host_exit_notifier(struct Notifier *n, void *data)
     }
 }

+static libusb_device *usb_host_find_ref(int bus, int addr)
+{
+    libusb_device **devs = NULL;
+    libusb_device *ret = NULL;
+    int i, n;
+
+    if (usb_host_init() != 0) {
+        return NULL;
+    }
+    n = libusb_get_device_list(ctx, &devs);
+    for (i = 0; i < n; i++) {
+        if (libusb_get_bus_number(devs[i]) == bus &&
+            libusb_get_device_address(devs[i]) == addr) {
+            ret = libusb_ref_device(devs[i]);
+            break;
+        }
+    }
+    libusb_free_device_list(devs, 1);
+    return ret;
+}
+
 static void usb_host_realize(USBDevice *udev, Error **errp)
 {
     USBHostDevice *s = USB_HOST_DEVICE(udev);
+    libusb_device *ldev;
+    int rc;

     if (s->match.vendor_id > 0xffff) {
         error_setg(errp, "vendorid out of range");
@@ -997,11 +1021,33 @@ static void usb_host_realize(USBDevice *udev, Error **errp)
     QTAILQ_INIT(&s->requests);
     QTAILQ_INIT(&s->isorings);

+    if (s->match.addr && s->match.bus_num &&
+        !s->match.vendor_id &&
+        !s->match.product_id &&
+        !s->match.port) {
+        s->needs_autoscan = false;
+        ldev = usb_host_find_ref(s->match.bus_num,
+                                 s->match.addr);
+        if (!ldev) {
+            error_setg(errp, "failed to find host usb device %d:%d",
+                       s->match.bus_num, s->match.addr);
+            return;
+        }
+        rc = usb_host_open(s, ldev);
+        libusb_unref_device(ldev);
+        if (rc < 0) {
+            error_setg(errp, "failed to open host usb device %d:%d",
+                       s->match.bus_num, s->match.addr);
+            return;
+        }
+    } else {
+        s->needs_autoscan = true;
+        QTAILQ_INSERT_TAIL(&hostdevs, s, next);
+        usb_host_auto_check(NULL);
+    }
+
     s->exit.notify = usb_host_exit_notifier;
     qemu_add_exit_notifier(&s->exit);
-
-    QTAILQ_INSERT_TAIL(&hostdevs, s, next);
-    usb_host_auto_check(NULL);
 }

 static void usb_host_instance_init(Object *obj)
@@ -1019,7 +1065,9 @@ static void usb_host_handle_destroy(USBDevice *udev)
     USBHostDevice *s = USB_HOST_DEVICE(udev);

     qemu_remove_exit_notifier(&s->exit);
-    QTAILQ_REMOVE(&hostdevs, s, next);
+    if (s->needs_autoscan) {
+        QTAILQ_REMOVE(&hostdevs, s, next);
+    }
     usb_host_close(s);
 }


So what happening here is that when `virsh attach-device` is called from
a udev rule, libudev has not yet notified libusb that the device has
been plugged in, so QEMU doesn't yet see it as present, and when libvirt
passed the logic address, QEMU looks that up, and errors out because it
doesn't know about it yet.  QED.

The problem is exacerbated by Cleverness ruling out all of the easy
workarounds that I can currently see.  libvirt doesn't pass on the
vendor or product ID even if we specify it along with an <address>
in its config snippet (which would bypass the new check in QEMU and
have it added to its hotplug watchlist again).  And systemd doesn't
let us background and delay the call to virsh until after it has
delivered the event via libudev ...


Ideally what we want here is for libvirt to better support the full
functionality that QEMU provides - allowing us to actually pass the
hostport and/or vendor and product ID when desired, since there are
good reasons to want to do that aside from this bug.  And even better
if (both) allowed specifying devices by vendor/product/serial to
allow uniquely designating one (or more) of multiple similar devices
without needing external hacks to first map that to a logical device
number or physical port address.

In the meantime, it would be nice if we can relax that test in QEMU
to keep things working until a Real Fix is in.  I can certainly see
the sense in being cautious about trusting a transient logical address
there - but if adding (and removing it again) already needs to be
managed by external logic to deal with the current less-than-ideal
state of things, then it's not unreasonable for us to be able to
say to it "trust us, you can't see it yet, but we've got this under
control, just wait until you get your notification event too").


Please do keep me in the CC for any discussion of this, I am very
interested in us ultimately Getting This Right as a supported thing.
I'd planned to open some discussion on improving this a while ago
(http://bitbabbler.org/blog.html#vm_hotplug), but this bug has now
made it a more urgent thing to deal with than it previously was.

  Cheers,
  Ron

Hi,

thanks for the detailed investigation. I hit this bug when I upgraded my
home server. I have USB devices assigned to VMs based on the USB topology
(hub and port number). Since I really need my USB printer to work I came up
with the following very hacky solution. I hope I can migrate to something
better in the future :)

(The following still has the bug that if I shutdown my server libvirt
will save my VMs to disk along with information on the attached USB
devices which probably is completely bogus when the guests are later
restored. I have not found a hook that'd get run when the VM is about to
be saved to disk but is still running.)

==== /etc/udev/rules.d/90-usb-libvirt-hotplug.rules

# 433 MHz transmitter for home automation
SUBSYSTEM=="usb",DEVPATH=="/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.1",RUN+="/root/udev-usb-hotplug radio.home"
# USB printer
SUBSYSTEM=="usb",DEVPATH=="/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.2",RUN+="/root/udev-usb-hotplug print.home"
# DVB capture cards
SUBSYSTEM=="usb",DEVPATH=="/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.4",RUN+="/root/udev-usb-hotplug tv.home"
SUBSYSTEM=="usb",DEVPATH=="/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.3",RUN+="/root/udev-usb-hotplug tv.home"
# 4G modem
SUBSYSTEM=="usb",DEVPATH=="/devices/pci0000:00/0000:00:14.0/usb1/1-4",RUN+="/root/udev-usb-hotplug backupgateway"

==== /root/udev-usb-hotplug

#!/bin/bash

domain="$1"

if [ "$ACTION" = "remove" ]; then
     /root/usb-libvirt-hotplug/usb-libvirt-hotplug.sh "$domain"
else
     # Delay "add" events to workaround https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=846173
     /usr/bin/systemd-run --on-active=5 --timer-property=AccuracySec=100ms env SUBSYSTEM="$SUBSYSTEM" DEVTYPE="$DEVTYPE" ACTION="$ACTION" BUSNUM="$BUSNUM" DEVNUM="$DEVNUM" /root/usb-libvirt-hotplug/usb-libvirt-hotplug.sh "$domain"
fi


==== /root/usb-libvirt-hotplug/usb-libvirt-hotplug.sh

See https://github.com/olavmrk/usb-libvirt-hotplug

==== /etc/libvirt/hooks/qemu


#!/bin/bash

add_usb_device() {
     sysdir="$1"
     vm="$2"
     if [ -e "$sysdir" ]; then
 	bus="$(grep BUSNUM= $sysdir/uevent | cut -d = -f2)"
 	dev="$(grep DEVNUM= $sysdir/uevent | cut -d = -f2)"
 	# hack: avoid deadlocks that happen if you call libvirt API from a libvirt hook..
 	setsid nohup env ACTION=add SUBSYSTEM=usb DEVTYPE=usb_device BUSNUM=$bus DEVNUM=$dev /root/usb-libvirt-hotplug/usb-libvirt-hotplug.sh "$vm" < /dev/null > /dev/null 2>&1 &

     fi
}

if [ "$2" = "started" -o "$2" = "restore" ]; then
     case "$1" in
 	radio.home)
 	    add_usb_device '/sys/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.1' radio.home
 	    ;;
 	print.home)
 	    add_usb_device '/sys/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.2' print.home
 	    ;;
 	tv.home)
 	    add_usb_device '/sys/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.3' tv.home
 	    add_usb_device '/sys/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.4' tv.home
 	    ;;
 	backupgateway)
 	    add_usb_device '/sys/devices/pci0000:00/0000:00:14.0/usb1/1-4' backupgateway
 	    ;;
     esac
fi

cat

exit 0

Timo, thanks.  Also, Ron, I wanted to add my belated thanks to you for taking this up and digging in so carefully, to the root cause.

Cheers all,

David

Hi,

thanks for the detailed investigation. I hit this bug when I upgraded my home server. I have USB devices assigned to VMs based on the USB topology (hub and port number). Since I really need my USB printer to work I came up with the following very hacky solution. I hope I can migrate to something better in the future :)

(The following still has the bug that if I shutdown my server libvirt will save my VMs to disk along with information on the attached USB devices which probably is completely bogus when the guests are later restored. I have not found a hook that'd get run when the VM is about to be saved to disk but is still running.)

==== /etc/udev/rules.d/90-usb-libvirt-hotplug.rules

# 433 MHz transmitter for home automation SUBSYSTEM=="usb",DEVPATH=="/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.1",RUN+="/root/udev-usb-hotplug radio.home"
# USB printer
SUBSYSTEM=="usb",DEVPATH=="/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.2",RUN+="/root/udev-usb-hotplug print.home"
# DVB capture cards
SUBSYSTEM=="usb",DEVPATH=="/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.4",RUN+="/root/udev-usb-hotplug tv.home"
SUBSYSTEM=="usb",DEVPATH=="/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.3",RUN+="/root/udev-usb-hotplug tv.home"
# 4G modem
SUBSYSTEM=="usb",DEVPATH=="/devices/pci0000:00/0000:00:14.0/usb1/1-4",RUN+="/root/udev-usb-hotplug backupgateway"

==== /root/udev-usb-hotplug

#!/bin/bash

domain="$1"

if [ "$ACTION" = "remove" ]; then
     /root/usb-libvirt-hotplug/usb-libvirt-hotplug.sh "$domain"
else
     # Delay "add" events to workaround https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=846173
     /usr/bin/systemd-run --on-active=5 --timer-property=AccuracySec=100ms env SUBSYSTEM="$SUBSYSTEM" DEVTYPE="$DEVTYPE" ACTION="$ACTION" BUSNUM="$BUSNUM" DEVNUM="$DEVNUM" /root/usb-libvirt-hotplug/usb-libvirt-hotplug.sh "$domain"
fi


==== /root/usb-libvirt-hotplug/usb-libvirt-hotplug.sh

See https://github.com/olavmrk/usb-libvirt-hotplug

==== /etc/libvirt/hooks/qemu


#!/bin/bash

add_usb_device() {
     sysdir="$1"
     vm="$2"
     if [ -e "$sysdir" ]; then
 	bus="$(grep BUSNUM= $sysdir/uevent | cut -d = -f2)"
 	dev="$(grep DEVNUM= $sysdir/uevent | cut -d = -f2)"
 	# hack: avoid deadlocks that happen if you call libvirt API from a libvirt hook..
 	setsid nohup env ACTION=add SUBSYSTEM=usb DEVTYPE=usb_device BUSNUM=$bus DEVNUM=$dev /root/usb-libvirt-hotplug/usb-libvirt-hotplug.sh "$vm" < /dev/null > /dev/null 2>&1 &

     fi
}

if [ "$2" = "started" -o "$2" = "restore" ]; then
     case "$1" in
 	radio.home)
 	    add_usb_device '/sys/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.1' radio.home
 	    ;;
 	print.home)
 	    add_usb_device '/sys/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.2' print.home
 	    ;;
 	tv.home)
 	    add_usb_device '/sys/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.3' tv.home
 	    add_usb_device '/sys/devices/pci0000:00/0000:00:14.0/usb1/1-9/1-9.1/1-9.1.4' tv.home
 	    ;;
 	backupgateway)
 	    add_usb_device '/sys/devices/pci0000:00/0000:00:14.0/usb1/1-4' backupgateway
 	    ;;
     esac
fi

cat

exit 0