[Qemu-devel] [PATCH 1/4] usb: Propagate errors through usb_register_companion()

[Markus Armbruster <armbru@redhat.com>]

This loses the messages explaining the error printed with
error_printf_unless_qmp().  The next commit will make up for the loss.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/usb/bus.c      | 17 ++++++++++-------
 hw/usb/hcd-ehci.c | 23 +++++++++++++----------
 hw/usb/hcd-ohci.c | 10 +++++++---
 hw/usb/hcd-uhci.c | 10 +++++++---
 include/hw/usb.h  | 12 +++++++-----
 5 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 986b2d8..8be942f 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -350,9 +350,10 @@ void usb_register_port(USBBus *bus, USBPort *port, void *opaque, int index,
     bus->nfree++;
 }
 
-int usb_register_companion(const char *masterbus, USBPort *ports[],
-                           uint32_t portcount, uint32_t firstport,
-                           void *opaque, USBPortOps *ops, int speedmask)
+void usb_register_companion(const char *masterbus, USBPort *ports[],
+                            uint32_t portcount, uint32_t firstport,
+                            void *opaque, USBPortOps *ops, int speedmask,
+                            Error **errp)
 {
     USBBus *bus;
     int i;
@@ -364,21 +365,23 @@ int usb_register_companion(const char *masterbus, USBPort *ports[],
     }
 
     if (!bus || !bus->ops->register_companion) {
-        qerror_report(QERR_INVALID_PARAMETER_VALUE, "masterbus",
-                      "an USB masterbus");
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "masterbus",
+                  "an USB masterbus");
+#if 0 /* conversion from qerror_report() to error_set() broke this: */
         if (bus) {
             error_printf_unless_qmp(
                 "USB bus '%s' does not allow companion controllers\n",
                 masterbus);
         }
-        return -1;
+#endif
+        return;
     }
 
     for (i = 0; i < portcount; i++) {
         usb_fill_port(ports[i], opaque, i, ops, speedmask);
     }
 
-    return bus->ops->register_companion(bus, ports, portcount, firstport);
+    bus->ops->register_companion(bus, ports, portcount, firstport, errp);
 }
 
 void usb_port_location(USBPort *downstream, USBPort *upstream, int portnr)
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index ccf54b6..7d16ba8 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -769,30 +769,35 @@ static void ehci_wakeup(USBPort *port)
     qemu_bh_schedule(s->async_bh);
 }
 
-static int ehci_register_companion(USBBus *bus, USBPort *ports[],
-                                   uint32_t portcount, uint32_t firstport)
+static void ehci_register_companion(USBBus *bus, USBPort *ports[],
+                                    uint32_t portcount, uint32_t firstport,
+                                    Error **errp)
 {
     EHCIState *s = container_of(bus, EHCIState, bus);
     uint32_t i;
 
     if (firstport + portcount > NB_PORTS) {
-        qerror_report(QERR_INVALID_PARAMETER_VALUE, "firstport",
-                      "firstport on masterbus");
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "firstport",
+                  "firstport on masterbus");
+#if 0 /* conversion from qerror_report() to error_set() broke this: */
         error_printf_unless_qmp(
             "firstport value of %u makes companion take ports %u - %u, which "
             "is outside of the valid range of 0 - %u\n", firstport, firstport,
             firstport + portcount - 1, NB_PORTS - 1);
-        return -1;
+#endif
+        return;
     }
 
     for (i = 0; i < portcount; i++) {
         if (s->companion_ports[firstport + i]) {
-            qerror_report(QERR_INVALID_PARAMETER_VALUE, "masterbus",
-                          "an USB masterbus");
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, "masterbus",
+                      "an USB masterbus");
+#if 0 /* conversion from qerror_report() to error_set() broke this: */
             error_printf_unless_qmp(
                 "port %u on masterbus %s already has a companion assigned\n",
                 firstport + i, bus->qbus.name);
-            return -1;
+#endif
+            return;
         }
     }
 
@@ -806,8 +811,6 @@ static int ehci_register_companion(USBBus *bus, USBPort *ports[],
 
     s->companion_count++;
     s->caps[0x05] = (s->companion_count << 4) | portcount;
-
-    return 0;
 }
 
 static void ehci_wakeup_endpoint(USBBus *bus, USBEndpoint *ep,
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index a0d478e..21ec65f 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1832,6 +1832,7 @@ static int usb_ohci_init(OHCIState *ohci, DeviceState *dev,
                          char *masterbus, uint32_t firstport,
                          AddressSpace *as)
 {
+    Error *err = NULL;
     int i;
 
     ohci->as = as;
@@ -1857,9 +1858,12 @@ static int usb_ohci_init(OHCIState *ohci, DeviceState *dev,
         for(i = 0; i < num_ports; i++) {
             ports[i] = &ohci->rhport[i].port;
         }
-        if (usb_register_companion(masterbus, ports, num_ports,
-                firstport, ohci, &ohci_port_ops,
-                USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL) != 0) {
+        usb_register_companion(masterbus, ports, num_ports,
+                               firstport, ohci, &ohci_port_ops,
+                               USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL,
+                               &err);
+        if (err) {
+            error_report_err(err);
             return -1;
         }
     } else {
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index f903de7..2ca8de3 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -1192,6 +1192,7 @@ static USBBusOps uhci_bus_ops = {
 
 static int usb_uhci_common_initfn(PCIDevice *dev)
 {
+    Error *err = NULL;
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
     UHCIPCIDeviceClass *u = container_of(pc, UHCIPCIDeviceClass, parent_class);
     UHCIState *s = DO_UPCAST(UHCIState, dev, dev);
@@ -1209,9 +1210,12 @@ static int usb_uhci_common_initfn(PCIDevice *dev)
         for(i = 0; i < NB_PORTS; i++) {
             ports[i] = &s->ports[i].port;
         }
-        if (usb_register_companion(s->masterbus, ports, NB_PORTS,
-                s->firstport, s, &uhci_port_ops,
-                USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL) != 0) {
+        usb_register_companion(s->masterbus, ports, NB_PORTS,
+                               s->firstport, s, &uhci_port_ops,
+                               USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL,
+                               &err);
+        if (err) {
+            error_report_err(err);
             return -1;
         }
     } else {
diff --git a/include/hw/usb.h b/include/hw/usb.h
index b20b959..453b22d 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -526,8 +526,9 @@ struct USBBus {
 };
 
 struct USBBusOps {
-    int (*register_companion)(USBBus *bus, USBPort *ports[],
-                              uint32_t portcount, uint32_t firstport);
+    void (*register_companion)(USBBus *bus, USBPort *ports[],
+                               uint32_t portcount, uint32_t firstport,
+                               Error **errp);
     void (*wakeup_endpoint)(USBBus *bus, USBEndpoint *ep, unsigned int stream);
 };
 
@@ -543,9 +544,10 @@ USBDevice *usb_create_simple(USBBus *bus, const char *name);
 USBDevice *usbdevice_create(const char *cmdline);
 void usb_register_port(USBBus *bus, USBPort *port, void *opaque, int index,
                        USBPortOps *ops, int speedmask);
-int usb_register_companion(const char *masterbus, USBPort *ports[],
-                           uint32_t portcount, uint32_t firstport,
-                           void *opaque, USBPortOps *ops, int speedmask);
+void usb_register_companion(const char *masterbus, USBPort *ports[],
+                            uint32_t portcount, uint32_t firstport,
+                            void *opaque, USBPortOps *ops, int speedmask,
+                            Error **errp);
 void usb_port_location(USBPort *downstream, USBPort *upstream, int portnr);
 void usb_unregister_port(USBBus *bus, USBPort *port);
 void usb_claim_port(USBDevice *dev, Error **errp);
-- 
1.9.3