From 4806ec9b2c57ff42a91d5419ac1137fffd1c9fb5 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 4 Feb 2015 13:28:08 +0100 Subject: usb: usb_create() can't fail, drop useless error handling Signed-off-by: Markus Armbruster Signed-off-by: Gerd Hoffmann --- hw/usb/bus.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'hw/usb/bus.c') diff --git a/hw/usb/bus.c b/hw/usb/bus.c index 986b2d8da8..eeb6872324 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -320,10 +320,6 @@ USBDevice *usb_create_simple(USBBus *bus, const char *name) USBDevice *dev = usb_create(bus, name); int rc; - if (!dev) { - error_report("Failed to create USB device '%s'", name); - return NULL; - } rc = qdev_init(&dev->qdev); if (rc < 0) { error_report("Failed to initialize USB device '%s'", name); -- cgit v1.2.3 From 3bc36a401e0f33e63a4d2c58b646ddf78efb567c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 4 Feb 2015 13:28:09 +0100 Subject: usb: Improve -usbdevice error reporting a bit Most LegacyUSBFactory usbdevice_init() methods realize with qdev_init_nofail(), even though their caller usbdevice_create() can handle failure. Okay if it really can't fail (I didn't check), but somewhat brittle. usb_msd_init() and usb_bt_init() call qdev_init(). The latter additionally reports an error when qdev_init() fails. Realization failure produces multiple error reports: a specific one from qdev_init(), and generic ones from usb_bt_init(), usb_create_simple(), usbdevice_create() and usb_parse(). Remove realization from the usbdevice_init() methods. Realize in usbdevice_create(), and produce exactly one error message there. You still get another one from usb_parse(). Signed-off-by: Markus Armbruster Signed-off-by: Gerd Hoffmann --- hw/usb/bus.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) (limited to 'hw/usb/bus.c') diff --git a/hw/usb/bus.c b/hw/usb/bus.c index eeb6872324..3f69fe1b23 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -651,10 +651,12 @@ USBDevice *usbdevice_create(const char *cmdline) { USBBus *bus = usb_bus_find(-1 /* any */); LegacyUSBFactory *f = NULL; + Error *err = NULL; GSList *i; char driver[32]; const char *params; int len; + USBDevice *dev; params = strchr(cmdline,':'); if (params) { @@ -689,14 +691,28 @@ USBDevice *usbdevice_create(const char *cmdline) return NULL; } - if (!f->usbdevice_init) { + if (f->usbdevice_init) { + dev = f->usbdevice_init(bus, params); + } else { if (*params) { error_report("usbdevice %s accepts no params", driver); return NULL; } - return usb_create_simple(bus, f->name); + dev = usb_create(bus, f->name); + } + if (!dev) { + error_report("Failed to create USB device '%s'", f->name); + return NULL; } - return f->usbdevice_init(bus, params); + object_property_set_bool(OBJECT(dev), true, "realized", &err); + if (err) { + error_report("Failed to initialize USB device '%s': %s", + f->name, error_get_pretty(err)); + error_free(err); + object_unparent(OBJECT(dev)); + return NULL; + } + return dev; } static void usb_device_class_init(ObjectClass *klass, void *data) -- cgit v1.2.3 From 06f22eb78f3eb557c667f5d0a46099e43a2aeb0d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 4 Feb 2015 13:28:10 +0100 Subject: usb: Do not prefix error_setg() messages with "Error: " Because it produces beauties like (qemu) usb_add mouse Failed to initialize USB device 'usb-mouse': Error: tried to attach usb device QEMU USB Mouse to a bus with no free ports Signed-off-by: Markus Armbruster Signed-off-by: Gerd Hoffmann --- hw/usb/bus.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'hw/usb/bus.c') diff --git a/hw/usb/bus.c b/hw/usb/bus.c index 3f69fe1b23..3e85afeff5 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -412,7 +412,7 @@ void usb_claim_port(USBDevice *dev, Error **errp) } } if (port == NULL) { - error_setg(errp, "Error: usb port %s (bus %s) not found (in use?)", + error_setg(errp, "usb port %s (bus %s) not found (in use?)", dev->port_path, bus->qbus.name); return; } @@ -422,7 +422,7 @@ void usb_claim_port(USBDevice *dev, Error **errp) usb_create_simple(bus, "usb-hub"); } if (bus->nfree == 0) { - error_setg(errp, "Error: tried to attach usb device %s to a bus " + error_setg(errp, "tried to attach usb device %s to a bus " "with no free ports", dev->product_desc); return; } -- cgit v1.2.3 From bd8b92d5c8387c2c94f06665514c05000169fafd Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 4 Feb 2015 13:28:11 +0100 Subject: usb: Suppress bogus error when automatic usb-hub creation fails 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 Signed-off-by: Gerd Hoffmann --- hw/usb/bus.c | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) (limited to 'hw/usb/bus.c') diff --git a/hw/usb/bus.c b/hw/usb/bus.c index 3e85afeff5..5abfac0532 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 " -- cgit v1.2.3 From 599655c91f3a55ac75007e851deca09010787bd7 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 4 Feb 2015 13:28:12 +0100 Subject: usb: Change usb_create_simple() to abort on failure Instead of returning null pointer. Matches pci_create_simple(), isa_create_simple(), sysbus_create_simple(). It's unused since the previous commit, but I'll put it to use again shortly. Signed-off-by: Markus Armbruster Signed-off-by: Gerd Hoffmann --- hw/usb/bus.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) (limited to 'hw/usb/bus.c') diff --git a/hw/usb/bus.c b/hw/usb/bus.c index 5abfac0532..d83a93887f 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -339,14 +339,7 @@ static USBDevice *usb_try_create_simple(USBBus *bus, const char *name, 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; + return usb_try_create_simple(bus, name, &error_abort); } static void usb_fill_port(USBPort *port, void *opaque, int index, -- cgit v1.2.3