[Qemu-devel] [PULL 4/8] usb: Suppress bogus error when automatic usb-hub creation fails

[Gerd Hoffmann]

From: Markus Armbruster <address@hidden>

USBDevice's realize method usb_qdev_realize() automatically creates a
usb-hub when only one port is left.  Creating devices in realize
methods is questionable, but works.

If usb-hub creation fails, an error is reported to stderr, but the
failure is otherwise ignored.  We then create the actual device using
the last port, which may well succeed.

Example:

    $ qemu -nodefaults -S -display none -machine usb=on -monitor stdio
    QEMU 2.2.50 monitor - type 'help' for more information
    (qemu) device_add usb-mouse
    [Repeat 36 times]
    (qemu) info usb
      Device 0.0, Port 1, Speed 12 Mb/s, Product QEMU USB Mouse
      Device 0.0, Port 2, Speed 12 Mb/s, Product QEMU USB Hub
      Device 0.0, Port 2.1, Speed 12 Mb/s, Product QEMU USB Mouse
    [More mice and hubs omitted...]
      Device 0.0, Port 2.8.8.8.8.7, Speed 12 Mb/s, Product QEMU USB Mouse
    (qemu) device_add usb-mouse
    usb hub chain too deep
    Failed to initialize USB device 'usb-hub'
    (qemu) info usb
    [...]
      Device 0.0, Port 2.8.8.8.8.7, Speed 12 Mb/s, Product QEMU USB Mouse
      Device 0.0, Port 2.8.8.8.8.8, Speed 12 Mb/s, Product QEMU USB Mouse

Despite the "Failed" message, the command actually succeeded.

In QMP, it's worse.  When adding the 37th mouse via QMP, the command
fails with

    {"error": {"class": "GenericError", "desc": "usb hub chain too deep"}}

Additionally, "Failed to initialize USB device 'usb-hub'" is reported
on stderr.  Despite the command failure, the device was created.  This
is wrong.

Fix by avoiding qdev_init() for usb-hub creation, so we can ignore
errors cleanly.

Signed-off-by: Markus Armbruster <address@hidden>
Signed-off-by: Gerd Hoffmann <address@hidden>
---
 hw/usb/bus.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 3e85afe..5abfac0 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -315,19 +315,40 @@ USBDevice *usb_create(USBBus *bus, const char *name)
     return USB_DEVICE(dev);
 }
 
-USBDevice *usb_create_simple(USBBus *bus, const char *name)
+static USBDevice *usb_try_create_simple(USBBus *bus, const char *name,
+                                        Error **errp)
 {
-    USBDevice *dev = usb_create(bus, name);
-    int rc;
+    Error *err = NULL;
+    USBDevice *dev;
 
-    rc = qdev_init(&dev->qdev);
-    if (rc < 0) {
-        error_report("Failed to initialize USB device '%s'", name);
+    dev = USB_DEVICE(qdev_try_create(&bus->qbus, name));
+    if (!dev) {
+        error_setg(errp, "Failed to create USB device '%s'", name);
+        return NULL;
+    }
+    object_property_set_bool(OBJECT(dev), true, "realized", &err);
+    if (err) {
+        error_setg(errp, "Failed to initialize USB device '%s': %s",
+                   name, error_get_pretty(err));
+        error_free(err);
+        object_unparent(OBJECT(dev));
         return NULL;
     }
     return dev;
 }
 
+USBDevice *usb_create_simple(USBBus *bus, const char *name)
+{
+    Error *err = NULL;
+    USBDevice *dev = usb_try_create_simple(bus, name, &err);
+
+    if (!dev) {
+        error_report("%s", error_get_pretty(err));
+        error_free(err);
+    }
+    return dev;
+}
+
 static void usb_fill_port(USBPort *port, void *opaque, int index,
                           USBPortOps *ops, int speedmask)
 {
@@ -419,7 +440,7 @@ void usb_claim_port(USBDevice *dev, Error **errp)
     } else {
         if (bus->nfree == 1 && strcmp(object_get_typename(OBJECT(dev)), 
"usb-hub") != 0) {
             /* Create a new hub and chain it on */
-            usb_create_simple(bus, "usb-hub");
+            usb_try_create_simple(bus, "usb-hub", NULL);
         }
         if (bus->nfree == 0) {
             error_setg(errp, "tried to attach usb device %s to a bus "
-- 
1.8.3.1