[Qemu-devel] [PATCH v2 00/19] usb: convert device init to realize

[Qemu-devel] [PATCH v2 00/19] usb: convert device init to realize

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

DeviceClass->init is the old interface, let's convert usb
devices to the new realize API. In this way, all the
implementations now use error_setg instead of 
qerror_report/error_report for reporting error.

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>

v2 -> v1:
 - fix PATCH 2, using qerror_report_err print error messages
  when attach fails (Paolo)
 - using errp instead of qerror_report_err introduced by
  fix 1 in PATCH 12 (Paolo)
 - fix missing return in PATCH 14 (Paolo)
 - add 'Reviewed-by' tag for other patches

Thanks a lot for reviewing!


Gonglei (19):
  usb-storage: fix possible memory leak and missing error message
  usb-bus: convert USBDeviceClass init to realize
  usb-net: convert init to realize
  libusb: convert init to realize
  libusb: using error_report instead of fprintf
  usb-hub: convert init to realize
  dev-storage: convert init to realize
  dev-storage: usring error_report instead of fprintf/printf
  dev-uas: convert init to realize
  dev-uas: using error_report instead of fprintf
  dev-bluetooth: convert init to realize
  dev-serial: convert init to realize
  usb-ccid: convert init to realize
  dev-hid: convert init to realize
  dev-wacom: convert init to realize
  usb-audio: convert init to realize
  usb-redir: convert init to realize
  usb-mtp: convert init to realize
  usb-bus: remove "init" from USBDeviceClass struct

 hw/usb/bus.c                  | 79 ++++++++++++++++++++++---------------------
 hw/usb/dev-audio.c            |  5 ++-
 hw/usb/dev-bluetooth.c        |  6 ++--
 hw/usb/dev-hid.c              | 27 +++++++--------
 hw/usb/dev-hub.c              |  9 +++--
 hw/usb/dev-mtp.c              |  5 ++-
 hw/usb/dev-network.c          |  9 +++--
 hw/usb/dev-serial.c           | 22 +++++++-----
 hw/usb/dev-smartcard-reader.c |  5 ++-
 hw/usb/dev-storage.c          | 42 ++++++++++++-----------
 hw/usb/dev-uas.c              | 17 +++++-----
 hw/usb/dev-wacom.c            |  5 ++-
 hw/usb/host-libusb.c          | 33 +++++++++---------
 hw/usb/redirect.c             | 21 +++++++-----
 include/hw/usb.h              | 10 ++++--
 15 files changed, 150 insertions(+), 145 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH v2 01/19] usb-storage: fix possible memory leak and missing error message

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

When scsi_bus_legacy_add_drive() return NULL, meanwhile err will
be not NULL, which will casue memory leak and missing error message.

Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/dev-storage.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index ae4efcb..f731b0a 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -624,6 +624,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
                                          s->conf.bootindex, dev->serial,
                                          &err);
     if (!scsi_dev) {
+        error_report("%s", error_get_pretty(err));
+        error_free(err);
         return -1;
     }
     s->bus.qbus.allow_hotplug = 0;
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 02/19] usb-bus: convert USBDeviceClass init to realize

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Add "realize/unrealize" in USBDeviceClass, which has errp
as a parameter. So all the implementations now use
error_setg instead of error_report for reporting error.

Note: this patch still keep "init" in USBDeviceClass, and
call kclass->init in usb_device_realize(), avoid breaking
git bisect. After realize all usb devices, will be removed.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/usb/bus.c         | 81 +++++++++++++++++++++++++++-------------------------
 hw/usb/dev-serial.c  | 16 +++++++++--
 hw/usb/dev-storage.c | 11 +++++--
 hw/usb/host-libusb.c |  7 +++--
 hw/usb/redirect.c    |  6 +++-
 include/hw/usb.h     | 10 +++++--
 6 files changed, 81 insertions(+), 50 deletions(-)

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index c7c4dad..12881cb 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -9,7 +9,7 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);
 
 static char *usb_get_dev_path(DeviceState *dev);
 static char *usb_get_fw_dev_path(DeviceState *qdev);
-static int usb_qdev_exit(DeviceState *qdev);
+static void usb_qdev_unrealize(DeviceState *qdev, Error **errp);
 
 static Property usb_props[] = {
     DEFINE_PROP_STRING("port", USBDevice, port_path),
@@ -107,13 +107,15 @@ USBBus *usb_bus_find(int busnr)
     return NULL;
 }
 
-static int usb_device_init(USBDevice *dev)
+static void usb_device_realize(USBDevice *dev, Error **errp)
 {
     USBDeviceClass *klass = USB_DEVICE_GET_CLASS(dev);
-    if (klass->init) {
-        return klass->init(dev);
+
+    if (klass->realize) {
+        klass->realize(dev, errp);
+    } else if (klass->init) {
+        klass->init(dev);
     }
-    return 0;
 }
 
 USBDevice *usb_device_find_device(USBDevice *dev, uint8_t addr)
@@ -232,36 +234,41 @@ void usb_device_free_streams(USBDevice *dev, USBEndpoint **eps, int nr_eps)
     }
 }
 
-static int usb_qdev_init(DeviceState *qdev)
+static void usb_qdev_realize(DeviceState *qdev, Error **errp)
 {
     USBDevice *dev = USB_DEVICE(qdev);
-    int rc;
+    Error *local_err = NULL;
 
     pstrcpy(dev->product_desc, sizeof(dev->product_desc),
             usb_device_get_product_desc(dev));
     dev->auto_attach = 1;
     QLIST_INIT(&dev->strings);
     usb_ep_init(dev);
-    rc = usb_claim_port(dev);
-    if (rc != 0) {
-        return rc;
+
+    usb_claim_port(dev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
     }
-    rc = usb_device_init(dev);
-    if (rc != 0) {
+
+    usb_device_realize(dev, &local_err);
+    if (local_err) {
         usb_release_port(dev);
-        return rc;
+        error_propagate(errp, local_err);
+        return;
     }
+
     if (dev->auto_attach) {
-        rc = usb_device_attach(dev);
-        if (rc != 0) {
-            usb_qdev_exit(qdev);
-            return rc;
+        usb_device_attach(dev, &local_err);
+        if (local_err) {
+            usb_qdev_unrealize(qdev, NULL);
+            error_propagate(errp, local_err);
+            return;
         }
     }
-    return 0;
 }
 
-static int usb_qdev_exit(DeviceState *qdev)
+static void usb_qdev_unrealize(DeviceState *qdev, Error **errp)
 {
     USBDevice *dev = USB_DEVICE(qdev);
 
@@ -272,7 +279,6 @@ static int usb_qdev_exit(DeviceState *qdev)
     if (dev->port) {
         usb_release_port(dev);
     }
-    return 0;
 }
 
 typedef struct LegacyUSBFactory
@@ -392,7 +398,7 @@ void usb_unregister_port(USBBus *bus, USBPort *port)
     bus->nfree--;
 }
 
-int usb_claim_port(USBDevice *dev)
+void usb_claim_port(USBDevice *dev, Error **errp)
 {
     USBBus *bus = usb_bus_from_device(dev);
     USBPort *port;
@@ -406,9 +412,9 @@ int usb_claim_port(USBDevice *dev)
             }
         }
         if (port == NULL) {
-            error_report("Error: usb port %s (bus %s) not found (in use?)",
-                         dev->port_path, bus->qbus.name);
-            return -1;
+            error_setg(errp, "Error: usb port %s (bus %s) not found (in use?)",
+                       dev->port_path, bus->qbus.name);
+            return;
         }
     } else {
         if (bus->nfree == 1 && strcmp(object_get_typename(OBJECT(dev)), "usb-hub") != 0) {
@@ -416,9 +422,9 @@ int usb_claim_port(USBDevice *dev)
             usb_create_simple(bus, "usb-hub");
         }
         if (bus->nfree == 0) {
-            error_report("Error: tried to attach usb device %s to a bus "
-                         "with no free ports", dev->product_desc);
-            return -1;
+            error_setg(errp, "Error: tried to attach usb device %s to a bus "
+                       "with no free ports", dev->product_desc);
+            return;
         }
         port = QTAILQ_FIRST(&bus->free);
     }
@@ -432,7 +438,6 @@ int usb_claim_port(USBDevice *dev)
 
     QTAILQ_INSERT_TAIL(&bus->used, port, next);
     bus->nused++;
-    return 0;
 }
 
 void usb_release_port(USBDevice *dev)
@@ -475,7 +480,7 @@ static void usb_mask_to_str(char *dest, size_t size,
     }
 }
 
-int usb_device_attach(USBDevice *dev)
+void usb_device_attach(USBDevice *dev, Error **errp)
 {
     USBBus *bus = usb_bus_from_device(dev);
     USBPort *port = dev->port;
@@ -489,18 +494,16 @@ int usb_device_attach(USBDevice *dev)
                           devspeed, portspeed);
 
     if (!(port->speedmask & dev->speedmask)) {
-        error_report("Warning: speed mismatch trying to attach"
-                     " usb device \"%s\" (%s speed)"
-                     " to bus \"%s\", port \"%s\" (%s speed)",
-                     dev->product_desc, devspeed,
-                     bus->qbus.name, port->path, portspeed);
-        return -1;
+        error_setg(errp, "Warning: speed mismatch trying to attach"
+                   " usb device \"%s\" (%s speed)"
+                   " to bus \"%s\", port \"%s\" (%s speed)",
+                   dev->product_desc, devspeed,
+                   bus->qbus.name, port->path, portspeed);
+        return;
     }
 
     dev->attached++;
     usb_attach(port);
-
-    return 0;
 }
 
 int usb_device_detach(USBDevice *dev)
@@ -688,9 +691,9 @@ static void usb_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->bus_type = TYPE_USB_BUS;
-    k->init     = usb_qdev_init;
     k->unplug   = qdev_simple_unplug_cb;
-    k->exit     = usb_qdev_exit;
+    k->realize  = usb_qdev_realize;
+    k->unrealize = usb_qdev_unrealize;
     k->props    = usb_props;
 }
 
diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index d360614..eb1b115 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -9,7 +9,7 @@
  */
 
 #include "qemu-common.h"
-#include "qemu/error-report.h"
+#include "monitor/monitor.h"
 #include "hw/usb.h"
 #include "hw/usb/desc.h"
 #include "sysemu/char.h"
@@ -451,6 +451,7 @@ static void usb_serial_read(void *opaque, const uint8_t *buf, int size)
 static void usb_serial_event(void *opaque, int event)
 {
     USBSerialState *s = opaque;
+    Error *local_err = NULL;
 
     switch (event) {
         case CHR_EVENT_BREAK:
@@ -460,7 +461,11 @@ static void usb_serial_event(void *opaque, int event)
             break;
         case CHR_EVENT_OPENED:
             if (!s->dev.attached) {
-                usb_device_attach(&s->dev);
+                usb_device_attach(&s->dev, &local_err);
+                if (local_err) {
+                    qerror_report_err(local_err);
+                    error_free(local_err);
+                }
             }
             break;
         case CHR_EVENT_CLOSED:
@@ -474,6 +479,7 @@ static void usb_serial_event(void *opaque, int event)
 static int usb_serial_initfn(USBDevice *dev)
 {
     USBSerialState *s = DO_UPCAST(USBSerialState, dev, dev);
+    Error *local_err = NULL;
 
     usb_desc_create_serial(dev);
     usb_desc_init(dev);
@@ -489,7 +495,11 @@ static int usb_serial_initfn(USBDevice *dev)
     usb_serial_handle_reset(dev);
 
     if (s->cs->be_open && !dev->attached) {
-        usb_device_attach(dev);
+        usb_device_attach(dev, &local_err);
+        if (local_err) {
+            qerror_report_err(local_err);
+            error_free(local_err);
+        }
     }
     return 0;
 }
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index f731b0a..06f63d1 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -549,12 +549,17 @@ static void usb_msd_handle_data(USBDevice *dev, USBPacket *p)
 static void usb_msd_password_cb(void *opaque, int err)
 {
     MSDState *s = opaque;
+    Error *local_err = NULL;
 
-    if (!err)
-        err = usb_device_attach(&s->dev);
+    if (!err) {
+        usb_device_attach(&s->dev, &local_err);
+    }
 
-    if (err)
+    if (local_err) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         qdev_unplug(&s->dev.qdev, NULL);
+    }
 }
 
 static void *usb_msd_load_request(QEMUFile *f, SCSIRequest *req)
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index c189147..9f92705 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -834,6 +834,7 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev)
     int bus_num = libusb_get_bus_number(dev);
     int addr    = libusb_get_device_address(dev);
     int rc;
+    Error *local_err = NULL;
 
     trace_usb_host_open_started(bus_num, addr);
 
@@ -869,8 +870,10 @@ static int usb_host_open(USBHostDevice *s, libusb_device *dev)
                  "host:%d.%d", bus_num, addr);
     }
 
-    rc = usb_device_attach(udev);
-    if (rc) {
+    usb_device_attach(udev, &local_err);
+    if (local_err) {
+        error_report("%s", error_get_pretty(local_err));
+        error_free(local_err);
         goto fail;
     }
 
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 44522d9..95158b3 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -1256,6 +1256,7 @@ static void usbredir_device_reject_bh(void *opaque)
 static void usbredir_do_attach(void *opaque)
 {
     USBRedirDevice *dev = opaque;
+    Error *local_err = NULL;
 
     /* In order to work properly with XHCI controllers we need these caps */
     if ((dev->dev.port->speedmask & USB_SPEED_MASK_SUPER) && !(
@@ -1270,7 +1271,10 @@ static void usbredir_do_attach(void *opaque)
         return;
     }
 
-    if (usb_device_attach(&dev->dev) != 0) {
+    usb_device_attach(&dev->dev, &local_err);
+    if (local_err) {
+        error_report("%s", error_get_pretty(local_err));
+        error_free(local_err);
         WARNING("rejecting device due to speed mismatch\n");
         usbredir_reject_device(dev);
     }
diff --git a/include/hw/usb.h b/include/hw/usb.h
index 6b32a3b..612f09f 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -267,11 +267,17 @@ struct USBDevice {
 #define USB_DEVICE_GET_CLASS(obj) \
      OBJECT_GET_CLASS(USBDeviceClass, (obj), TYPE_USB_DEVICE)
 
+typedef void (*USBDeviceRealize)(USBDevice *dev, Error **errp);
+typedef void (*USBDeviceUnrealize)(USBDevice *dev, Error **errp);
+
 typedef struct USBDeviceClass {
     DeviceClass parent_class;
 
     int (*init)(USBDevice *dev);
 
+    USBDeviceRealize realize;
+    USBDeviceUnrealize unrealize;
+
     /*
      * Walk (enabled) downstream ports, check for a matching device.
      * Only hubs implement this.
@@ -544,9 +550,9 @@ int usb_register_companion(const char *masterbus, USBPort *ports[],
                            void *opaque, USBPortOps *ops, int speedmask);
 void usb_port_location(USBPort *downstream, USBPort *upstream, int portnr);
 void usb_unregister_port(USBBus *bus, USBPort *port);
-int usb_claim_port(USBDevice *dev);
+void usb_claim_port(USBDevice *dev, Error **errp);
 void usb_release_port(USBDevice *dev);
-int usb_device_attach(USBDevice *dev);
+void usb_device_attach(USBDevice *dev, Error **errp);
 int usb_device_detach(USBDevice *dev);
 int usb_device_delete_addr(int busnr, int addr);
 
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 03/19] usb-net: convert init to realize

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

meanwhile, qerror_report_err() is a transitional interface to
help with converting existing HMP commands to QMP. It should
not be used elsewhere.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/dev-network.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index 518d536..686bd69 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -27,7 +27,7 @@
 #include "hw/usb.h"
 #include "hw/usb/desc.h"
 #include "net/net.h"
-#include "qapi/qmp/qerror.h"
+#include "qemu/error-report.h"
 #include "qemu/queue.h"
 #include "qemu/config-file.h"
 #include "sysemu/sysemu.h"
@@ -1341,7 +1341,7 @@ static NetClientInfo net_usbnet_info = {
     .cleanup = usbnet_cleanup,
 };
 
-static int usb_net_initfn(USBDevice *dev)
+static void usb_net_realize(USBDevice *dev, Error **errrp)
 {
     USBNetState *s = DO_UPCAST(USBNetState, dev, dev);
 
@@ -1373,7 +1373,6 @@ static int usb_net_initfn(USBDevice *dev)
     usb_desc_set_string(dev, STRING_ETHADDR, s->usbstring_mac);
 
     add_boot_device_path(s->conf.bootindex, &dev->qdev, "/ethernet@0");
-    return 0;
 }
 
 static USBDevice *usb_net_init(USBBus *bus, const char *cmdline)
@@ -1392,7 +1391,7 @@ static USBDevice *usb_net_init(USBBus *bus, const char *cmdline)
 
     idx = net_client_init(opts, 0, &local_err);
     if (local_err) {
-        qerror_report_err(local_err);
+        error_report("%s", error_get_pretty(local_err));
         error_free(local_err);
         return NULL;
     }
@@ -1421,7 +1420,7 @@ static void usb_net_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
-    uc->init           = usb_net_initfn;
+    uc->realize           = usb_net_realize;
     uc->product_desc   = "QEMU USB Network Interface";
     uc->usb_desc       = &desc_net;
     uc->handle_reset   = usb_net_handle_reset;
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 04/19] libusb: convert init to realize

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

In this way, all the implementations now use
error_setg instead of error_report for reporting error.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/host-libusb.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 9f92705..863be64 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -951,21 +951,21 @@ static void usb_host_exit_notifier(struct Notifier *n, void *data)
     }
 }
 
-static int usb_host_initfn(USBDevice *udev)
+static void usb_host_realize(USBDevice *udev, Error **errp)
 {
     USBHostDevice *s = USB_HOST_DEVICE(udev);
 
     if (s->match.vendor_id > 0xffff) {
-        error_report("vendorid out of range");
-        return -1;
+        error_setg(errp, "vendorid out of range");
+        return;
     }
     if (s->match.product_id > 0xffff) {
-        error_report("productid out of range");
-        return -1;
+        error_setg(errp, "productid out of range");
+        return;
     }
     if (s->match.addr > 127) {
-        error_report("hostaddr out of range");
-        return -1;
+        error_setg(errp, "hostaddr out of range");
+        return;
     }
 
     loglevel = s->loglevel;
@@ -980,7 +980,6 @@ static int usb_host_initfn(USBDevice *udev)
     QTAILQ_INSERT_TAIL(&hostdevs, s, next);
     add_boot_device_path(s->bootindex, &udev->qdev, NULL);
     usb_host_auto_check(NULL);
-    return 0;
 }
 
 static void usb_host_handle_destroy(USBDevice *udev)
@@ -1480,7 +1479,7 @@ static void usb_host_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
-    uc->init           = usb_host_initfn;
+    uc->realize           = usb_host_realize;
     uc->product_desc   = "USB Host Device";
     uc->cancel_packet  = usb_host_cancel_packet;
     uc->handle_data    = usb_host_handle_data;
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 05/19] libusb: using error_report instead of fprintf

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/host-libusb.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 863be64..0650910 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -275,7 +275,7 @@ static void usb_host_libusb_error(const char *func, int rc)
     } else {
         errname = "?";
     }
-    fprintf(stderr, "%s: %d [%s]\n", func, rc, errname);
+    error_report("%s: %d [%s]", func, rc, errname);
 }
 
 /* ------------------------------------------------------------------------ */
@@ -1376,14 +1376,13 @@ static int usb_host_alloc_streams(USBDevice *udev, USBEndpoint **eps,
     if (rc < 0) {
         usb_host_libusb_error("libusb_alloc_streams", rc);
     } else if (rc != streams) {
-        fprintf(stderr,
-            "libusb_alloc_streams: got less streams then requested %d < %d\n",
-            rc, streams);
+        error_report("libusb_alloc_streams: got less streams "
+                     "then requested %d < %d", rc, streams);
     }
 
     return (rc == streams) ? 0 : -1;
 #else
-    fprintf(stderr, "libusb_alloc_streams: error not implemented\n");
+    error_report("libusb_alloc_streams: error not implemented");
     return -1;
 #endif
 }
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 06/19] usb-hub: convert init to realize

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

In this way, all the implementations now use
error_setg instead of error_report for reporting error.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/dev-hub.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
index 7492174..8e3f7a8 100644
--- a/hw/usb/dev-hub.c
+++ b/hw/usb/dev-hub.c
@@ -511,15 +511,15 @@ static USBPortOps usb_hub_port_ops = {
     .complete = usb_hub_complete,
 };
 
-static int usb_hub_initfn(USBDevice *dev)
+static void usb_hub_realize(USBDevice *dev, Error **errp)
 {
     USBHubState *s = DO_UPCAST(USBHubState, dev, dev);
     USBHubPort *port;
     int i;
 
     if (dev->port->hubcount == 5) {
-        error_report("usb hub chain too deep");
-        return -1;
+        error_setg(errp, "usb hub chain too deep");
+        return;
     }
 
     usb_desc_create_serial(dev);
@@ -533,7 +533,6 @@ static int usb_hub_initfn(USBDevice *dev)
         usb_port_location(&port->port, dev->port, i+1);
     }
     usb_hub_handle_reset(dev);
-    return 0;
 }
 
 static const VMStateDescription vmstate_usb_hub_port = {
@@ -564,7 +563,7 @@ static void usb_hub_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
-    uc->init           = usb_hub_initfn;
+    uc->realize           = usb_hub_realize;
     uc->product_desc   = "QEMU USB Hub";
     uc->usb_desc       = &desc_hub;
     uc->find_device    = usb_hub_find_device;
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 07/19] dev-storage: convert init to realize

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

In this way, all the implementations now use
error_setg instead of error_report for reporting error.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/dev-storage.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 06f63d1..182dd5a 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -595,7 +595,7 @@ static const struct SCSIBusInfo usb_msd_scsi_info_bot = {
     .load_request = usb_msd_load_request,
 };
 
-static int usb_msd_initfn_storage(USBDevice *dev)
+static void usb_msd_realize_storage(USBDevice *dev, Error **errp)
 {
     MSDState *s = DO_UPCAST(MSDState, dev, dev);
     BlockDriverState *bs = s->conf.bs;
@@ -603,8 +603,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
     Error *err = NULL;
 
     if (!bs) {
-        error_report("drive property not set");
-        return -1;
+        error_setg(errp, "drive property not set");
+        return;
     }
 
     blkconf_serial(&s->conf, &dev->serial);
@@ -629,9 +629,8 @@ static int usb_msd_initfn_storage(USBDevice *dev)
                                          s->conf.bootindex, dev->serial,
                                          &err);
     if (!scsi_dev) {
-        error_report("%s", error_get_pretty(err));
-        error_free(err);
-        return -1;
+        error_propagate(errp, err);
+        return;
     }
     s->bus.qbus.allow_hotplug = 0;
     usb_msd_handle_reset(dev);
@@ -644,11 +643,9 @@ static int usb_msd_initfn_storage(USBDevice *dev)
             autostart = 0;
         }
     }
-
-    return 0;
 }
 
-static int usb_msd_initfn_bot(USBDevice *dev)
+static void usb_msd_realize_bot(USBDevice *dev, Error **errp)
 {
     MSDState *s = DO_UPCAST(MSDState, dev, dev);
 
@@ -658,8 +655,6 @@ static int usb_msd_initfn_bot(USBDevice *dev)
                  &usb_msd_scsi_info_bot, NULL);
     s->bus.qbus.allow_hotplug = 0;
     usb_msd_handle_reset(dev);
-
-    return 0;
 }
 
 static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
@@ -765,7 +760,7 @@ static void usb_msd_class_initfn_storage(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
-    uc->init = usb_msd_initfn_storage;
+    uc->realize = usb_msd_realize_storage;
     dc->props = msd_properties;
     usb_msd_class_initfn_common(klass);
 }
@@ -774,7 +769,7 @@ static void usb_msd_class_initfn_bot(ObjectClass *klass, void *data)
 {
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
-    uc->init = usb_msd_initfn_bot;
+    uc->realize = usb_msd_realize_bot;
     usb_msd_class_initfn_common(klass);
 }
 
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 08/19] dev-storage: usring error_report instead of fprintf/printf

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/dev-storage.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 182dd5a..a5e9d4a 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -409,19 +409,19 @@ static void usb_msd_handle_data(USBDevice *dev, USBPacket *p)
         switch (s->mode) {
         case USB_MSDM_CBW:
             if (p->iov.size != 31) {
-                fprintf(stderr, "usb-msd: Bad CBW size");
+                error_report("usb-msd: Bad CBW size");
                 goto fail;
             }
             usb_packet_copy(p, &cbw, 31);
             if (le32_to_cpu(cbw.sig) != 0x43425355) {
-                fprintf(stderr, "usb-msd: Bad signature %08x\n",
-                        le32_to_cpu(cbw.sig));
+                error_report("usb-msd: Bad signature %08x",
+                             le32_to_cpu(cbw.sig));
                 goto fail;
             }
             DPRINTF("Command on LUN %d\n", cbw.lun);
             scsi_dev = scsi_device_find(&s->bus, 0, 0, cbw.lun);
             if (scsi_dev == NULL) {
-                fprintf(stderr, "usb-msd: Bad LUN %d\n", cbw.lun);
+                error_report("usb-msd: Bad LUN %d", cbw.lun);
                 goto fail;
             }
             tag = le32_to_cpu(cbw.tag);
@@ -680,13 +680,13 @@ static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
             pstrcpy(fmt, len, p2);
             qemu_opt_set(opts, "format", fmt);
         } else if (*filename != ':') {
-            printf("unrecognized USB mass-storage option %s\n", filename);
+            error_report("unrecognized USB mass-storage option %s", filename);
             return NULL;
         }
         filename = p1;
     }
     if (!*filename) {
-        printf("block device specification needed\n");
+        error_report("block device specification needed");
         return NULL;
     }
     qemu_opt_set(opts, "file", filename);
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 09/19] dev-uas: convert init to realize

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/dev-uas.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index 9832385..884c003 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -892,7 +892,7 @@ static void usb_uas_handle_destroy(USBDevice *dev)
     qemu_bh_delete(uas->status_bh);
 }
 
-static int usb_uas_init(USBDevice *dev)
+static void usb_uas_realize(USBDevice *dev, Error **errp)
 {
     UASDevice *uas = DO_UPCAST(UASDevice, dev, dev);
 
@@ -905,8 +905,6 @@ static int usb_uas_init(USBDevice *dev)
 
     scsi_bus_new(&uas->bus, sizeof(uas->bus), DEVICE(dev),
                  &usb_uas_scsi_info, NULL);
-
-    return 0;
 }
 
 static const VMStateDescription vmstate_usb_uas = {
@@ -928,7 +926,7 @@ static void usb_uas_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
-    uc->init           = usb_uas_init;
+    uc->realize           = usb_uas_realize;
     uc->product_desc   = desc_strings[STR_PRODUCT];
     uc->usb_desc       = &desc;
     uc->cancel_packet  = usb_uas_cancel_io;
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 10/19] dev-uas: using error_report instead of fprintf

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/dev-uas.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index 884c003..1508064 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -13,6 +13,7 @@
 #include "qemu/option.h"
 #include "qemu/config-file.h"
 #include "trace.h"
+#include "qemu/error-report.h"
 
 #include "hw/usb.h"
 #include "hw/usb/desc.h"
@@ -648,7 +649,7 @@ static void usb_uas_handle_control(USBDevice *dev, USBPacket *p,
     if (ret >= 0) {
         return;
     }
-    fprintf(stderr, "%s: unhandled control request\n", __func__);
+    error_report("%s: unhandled control request", __func__);
     p->status = USB_RET_STALL;
 }
 
@@ -814,8 +815,8 @@ static void usb_uas_handle_data(USBDevice *dev, USBPacket *p)
             usb_uas_task(uas, &iu);
             break;
         default:
-            fprintf(stderr, "%s: unknown command iu: id 0x%x\n",
-                    __func__, iu.hdr.id);
+            error_report("%s: unknown command iu: id 0x%x",
+                         __func__, iu.hdr.id);
             p->status = USB_RET_STALL;
             break;
         }
@@ -861,7 +862,7 @@ static void usb_uas_handle_data(USBDevice *dev, USBPacket *p)
                 p->status = USB_RET_ASYNC;
                 break;
             } else {
-                fprintf(stderr, "%s: no inflight request\n", __func__);
+                error_report("%s: no inflight request", __func__);
                 p->status = USB_RET_STALL;
                 break;
             }
@@ -879,7 +880,7 @@ static void usb_uas_handle_data(USBDevice *dev, USBPacket *p)
         usb_uas_start_next_transfer(uas);
         break;
     default:
-        fprintf(stderr, "%s: invalid endpoint %d\n", __func__, p->ep->nr);
+        error_report("%s: invalid endpoint %d", __func__, p->ep->nr);
         p->status = USB_RET_STALL;
         break;
     }
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 11/19] dev-bluetooth: convert init to realize

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/dev-bluetooth.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/usb/dev-bluetooth.c b/hw/usb/dev-bluetooth.c
index a76e581..7db9921 100644
--- a/hw/usb/dev-bluetooth.c
+++ b/hw/usb/dev-bluetooth.c
@@ -501,7 +501,7 @@ static void usb_bt_handle_destroy(USBDevice *dev)
     s->hci->acl_recv = NULL;
 }
 
-static int usb_bt_initfn(USBDevice *dev)
+static void usb_bt_realize(USBDevice *dev, Error **errp)
 {
     struct USBBtState *s = DO_UPCAST(struct USBBtState, dev, dev);
 
@@ -516,8 +516,6 @@ static int usb_bt_initfn(USBDevice *dev)
     s->hci->acl_recv = usb_bt_out_hci_packet_acl;
     usb_bt_handle_reset(&s->dev);
     s->intr = usb_ep_get(dev, USB_TOKEN_IN, USB_EVT_EP);
-
-    return 0;
 }
 
 static USBDevice *usb_bt_init(USBBus *bus, const char *cmdline)
@@ -560,7 +558,7 @@ static void usb_bt_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
-    uc->init           = usb_bt_initfn;
+    uc->realize           = usb_bt_realize;
     uc->product_desc   = "QEMU BT dongle";
     uc->usb_desc       = &desc_bluetooth;
     uc->handle_reset   = usb_bt_handle_reset;
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 12/19] dev-serial: convert init to realize

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

In this way, all the implementations now use
error_setg instead of error_report for reporting error.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/dev-serial.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index eb1b115..d97339f 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -476,18 +476,17 @@ static void usb_serial_event(void *opaque, int event)
     }
 }
 
-static int usb_serial_initfn(USBDevice *dev)
+static void usb_serial_realize(USBDevice *dev, Error **errp)
 {
     USBSerialState *s = DO_UPCAST(USBSerialState, dev, dev);
-    Error *local_err = NULL;
 
     usb_desc_create_serial(dev);
     usb_desc_init(dev);
     dev->auto_attach = 0;
 
     if (!s->cs) {
-        error_report("Property chardev is required");
-        return -1;
+        error_setg(errp, "Property chardev is required");
+        return;
     }
 
     qemu_chr_add_handlers(s->cs, usb_serial_can_read, usb_serial_read,
@@ -495,13 +494,8 @@ static int usb_serial_initfn(USBDevice *dev)
     usb_serial_handle_reset(dev);
 
     if (s->cs->be_open && !dev->attached) {
-        usb_device_attach(dev, &local_err);
-        if (local_err) {
-            qerror_report_err(local_err);
-            error_free(local_err);
-        }
+        usb_device_attach(dev, errp);
     }
-    return 0;
 }
 
 static USBDevice *usb_serial_init(USBBus *bus, const char *filename)
@@ -592,7 +586,7 @@ static void usb_serial_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
-    uc->init = usb_serial_initfn;
+    uc->realize = usb_serial_realize;
     uc->product_desc   = "QEMU USB Serial";
     uc->usb_desc       = &desc_serial;
     uc->handle_reset   = usb_serial_handle_reset;
@@ -620,7 +614,7 @@ static void usb_braille_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
-    uc->init           = usb_serial_initfn;
+    uc->realize           = usb_serial_realize;
     uc->product_desc   = "QEMU USB Braille";
     uc->usb_desc       = &desc_braille;
     uc->handle_reset   = usb_serial_handle_reset;
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH v2 12/19] dev-serial: convert init to realize

[Paolo Bonzini]

Il 18/09/2014 14:47, arei.gonglei@huawei.com ha scritto:
>      if (s->cs->be_open && !dev->attached) {
> -        usb_device_attach(dev, &local_err);
> -        if (local_err) {
> -            qerror_report_err(local_err);
> -            error_free(local_err);
> -        }
> +        usb_device_attach(dev, errp);
>      }
> -    return 0;

Subtle change: realize now fails if the attach fails, i.e. if the speed
is mismatched.  This is a bugfix, but it doesn't happen if the backend
is closed (e.g. for server socket chardevs).

For a follow-up, the speed check could be extracted to a separate
function usb_check_attach, that is called just once at realize time,
even if !s->cs->be_open.  Then, I think, all calls to usb_device_attach
in dev-serial can pass &error_abort.

Paolo

Re: [Qemu-devel] [PATCH v2 12/19] dev-serial: convert init to realize

[Gonglei (Arei)]

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: Thursday, September 18, 2014 8:59 PM
> Subject: Re: [PATCH v2 12/19] dev-serial: convert init to realize
> 
> Il 18/09/2014 14:47, arei.gonglei@huawei.com ha scritto:
> >      if (s->cs->be_open && !dev->attached) {
> > -        usb_device_attach(dev, &local_err);
> > -        if (local_err) {
> > -            qerror_report_err(local_err);
> > -            error_free(local_err);
> > -        }
> > +        usb_device_attach(dev, errp);
> >      }
> > -    return 0;
> 
> Subtle change: realize now fails if the attach fails, i.e. if the speed
> is mismatched.  This is a bugfix, but it doesn't happen if the backend
> is closed (e.g. for server socket chardevs).
> 
Yes. Good catch. :)

> For a follow-up, the speed check could be extracted to a separate
> function usb_check_attach, that is called just once at realize time,
> even if !s->cs->be_open.  Then, I think, all calls to usb_device_attach
> in dev-serial can pass &error_abort.
> 
Yes, we can. 

Can we use qerror_report_err still in realize function?
Just report this error but not propagate to the caller. Thanks!

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v2 12/19] dev-serial: convert init to realize

[Paolo Bonzini]

Il 18/09/2014 15:26, Gonglei (Arei) ha scritto:
> Yes, we can. 
> 
> Can we use qerror_report_err still in realize function?
> Just report this error but not propagate to the caller. Thanks!

No, I think propagating the error is fine.  In fact I suggested checking
for the error more aggressively, so that it can better propagate to the
caller.

Paolo

Re: [Qemu-devel] [PATCH v2 12/19] dev-serial: convert init to realize

[Gonglei (Arei)]

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: Thursday, September 18, 2014 9:31 PM
> To: Gonglei (Arei); qemu-devel@nongnu.org
> Cc: kraxel@redhat.com; Huangweidong (C); armbru@redhat.com; Huangpeng
> (Peter); Luonengjun
> Subject: Re: [PATCH v2 12/19] dev-serial: convert init to realize
> 
> Il 18/09/2014 15:26, Gonglei (Arei) ha scritto:
> > Yes, we can.
> >
> > Can we use qerror_report_err still in realize function?
> > Just report this error but not propagate to the caller. Thanks!
> 
> No, I think propagating the error is fine.  In fact I suggested checking
> for the error more aggressively, so that it can better propagate to the
> caller.
> 
> Paolo

OK, got it. v3 will be posted shortly. Thanks :)

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v2 12/19] dev-serial: convert init to realize

[Paolo Bonzini]

Il 18/09/2014 15:35, Gonglei (Arei) ha scritto:
>> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
>> Sent: Thursday, September 18, 2014 9:31 PM
>> To: Gonglei (Arei); qemu-devel@nongnu.org
>> Cc: kraxel@redhat.com; Huangweidong (C); armbru@redhat.com; Huangpeng
>> (Peter); Luonengjun
>> Subject: Re: [PATCH v2 12/19] dev-serial: convert init to realize
>>
>> Il 18/09/2014 15:26, Gonglei (Arei) ha scritto:
>>> Yes, we can.
>>>
>>> Can we use qerror_report_err still in realize function?
>>> Just report this error but not propagate to the caller. Thanks!
>>
>> No, I think propagating the error is fine.  In fact I suggested checking
>> for the error more aggressively, so that it can better propagate to the
>> caller.
>>
>> Paolo
> 
> OK, got it. v3 will be posted shortly. Thanks :)

No need for that.  It can be done as a separate series.  This v2 is fine
for me.

Paolo

Re: [Qemu-devel] [PATCH v2 12/19] dev-serial: convert init to realize

[Gonglei (Arei)]

> >>> Can we use qerror_report_err still in realize function?
> >>> Just report this error but not propagate to the caller. Thanks!
> >>
> >> No, I think propagating the error is fine.  In fact I suggested checking
> >> for the error more aggressively, so that it can better propagate to the
> >> caller.
> >>
> >> Paolo
> >
> > OK, got it. v3 will be posted shortly. Thanks :)
> 
> No need for that.  It can be done as a separate series.  This v2 is fine
> for me.
> 
Great! Thanks. :)

Waiting for Gerd's response ~

Best regards,
-Gonglei

[Qemu-devel] [PATCH v2 13/19] usb-ccid: convert init to realize

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/dev-smartcard-reader.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 470e69f..442f487 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1304,7 +1304,7 @@ static int ccid_card_init(DeviceState *qdev)
     return ret;
 }
 
-static int ccid_initfn(USBDevice *dev)
+static void ccid_realize(USBDevice *dev, Error **errp)
 {
     USBCCIDState *s = DO_UPCAST(USBCCIDState, dev, dev);
 
@@ -1332,7 +1332,6 @@ static int ccid_initfn(USBDevice *dev)
     ccid_reset_parameters(s);
     ccid_reset(s);
     s->debug = parse_debug_env("QEMU_CCID_DEBUG", D_VERBOSE, s->debug);
-    return 0;
 }
 
 static int ccid_post_load(void *opaque, int version_id)
@@ -1441,7 +1440,7 @@ static void ccid_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
-    uc->init           = ccid_initfn;
+    uc->realize           = ccid_realize;
     uc->product_desc   = "QEMU USB CCID";
     uc->usb_desc       = &desc_ccid;
     uc->handle_reset   = ccid_handle_reset;
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 14/19] dev-hid: convert init to realize

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

In this way, all the implementations now use
error_setg instead of error_report for reporting error.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/usb/dev-hid.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c
index 67a57f1..05dc03b 100644
--- a/hw/usb/dev-hid.c
+++ b/hw/usb/dev-hid.c
@@ -566,7 +566,7 @@ static void usb_hid_handle_destroy(USBDevice *dev)
     hid_free(&us->hid);
 }
 
-static int usb_hid_initfn(USBDevice *dev, int kind)
+static void usb_hid_initfn(USBDevice *dev, int kind)
 {
     USBHIDState *us = DO_UPCAST(USBHIDState, dev, dev);
 
@@ -579,10 +579,9 @@ static int usb_hid_initfn(USBDevice *dev, int kind)
     if (us->display && us->hid.s) {
         qemu_input_handler_bind(us->hid.s, us->display, us->head, NULL);
     }
-    return 0;
 }
 
-static int usb_tablet_initfn(USBDevice *dev)
+static void usb_tablet_realize(USBDevice *dev, Error **errp)
 {
     USBHIDState *us = DO_UPCAST(USBHIDState, dev, dev);
 
@@ -594,22 +593,22 @@ static int usb_tablet_initfn(USBDevice *dev)
         dev->usb_desc = &desc_tablet2;
         break;
     default:
-        error_report("Invalid usb version %d for usb-tabler (must be 1 or 2)",
-                     us->usb_version);
-        return -1;
+        error_setg(errp, "Invalid usb version %d for usb-tablet "
+                   "(must be 1 or 2)", us->usb_version);
+        return;
     }
 
-    return usb_hid_initfn(dev, HID_TABLET);
+    usb_hid_initfn(dev, HID_TABLET);
 }
 
-static int usb_mouse_initfn(USBDevice *dev)
+static void usb_mouse_realize(USBDevice *dev, Error **errp)
 {
-    return usb_hid_initfn(dev, HID_MOUSE);
+    usb_hid_initfn(dev, HID_MOUSE);
 }
 
-static int usb_keyboard_initfn(USBDevice *dev)
+static void usb_keyboard_realize(USBDevice *dev, Error **errp)
 {
-    return usb_hid_initfn(dev, HID_KEYBOARD);
+    usb_hid_initfn(dev, HID_KEYBOARD);
 }
 
 static int usb_ptr_post_load(void *opaque, int version_id)
@@ -669,7 +668,7 @@ static void usb_tablet_class_initfn(ObjectClass *klass, void *data)
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
     usb_hid_class_initfn(klass, data);
-    uc->init           = usb_tablet_initfn;
+    uc->realize           = usb_tablet_realize;
     uc->product_desc   = "QEMU USB Tablet";
     dc->vmsd = &vmstate_usb_ptr;
     dc->props = usb_tablet_properties;
@@ -689,7 +688,7 @@ static void usb_mouse_class_initfn(ObjectClass *klass, void *data)
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
     usb_hid_class_initfn(klass, data);
-    uc->init           = usb_mouse_initfn;
+    uc->realize           = usb_mouse_realize;
     uc->product_desc   = "QEMU USB Mouse";
     uc->usb_desc       = &desc_mouse;
     dc->vmsd = &vmstate_usb_ptr;
@@ -714,7 +713,7 @@ static void usb_keyboard_class_initfn(ObjectClass *klass, void *data)
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
     usb_hid_class_initfn(klass, data);
-    uc->init           = usb_keyboard_initfn;
+    uc->realize           = usb_keyboard_realize;
     uc->product_desc   = "QEMU USB Keyboard";
     uc->usb_desc       = &desc_keyboard;
     dc->vmsd = &vmstate_usb_kbd;
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 15/19] dev-wacom: convert init to realize

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/dev-wacom.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/usb/dev-wacom.c b/hw/usb/dev-wacom.c
index 1b73fd0..217d48e 100644
--- a/hw/usb/dev-wacom.c
+++ b/hw/usb/dev-wacom.c
@@ -335,14 +335,13 @@ static void usb_wacom_handle_destroy(USBDevice *dev)
     }
 }
 
-static int usb_wacom_initfn(USBDevice *dev)
+static void usb_wacom_realize(USBDevice *dev, Error **errp)
 {
     USBWacomState *s = DO_UPCAST(USBWacomState, dev, dev);
     usb_desc_create_serial(dev);
     usb_desc_init(dev);
     s->intr = usb_ep_get(dev, USB_TOKEN_IN, 1);
     s->changed = 1;
-    return 0;
 }
 
 static const VMStateDescription vmstate_usb_wacom = {
@@ -357,7 +356,7 @@ static void usb_wacom_class_init(ObjectClass *klass, void *data)
 
     uc->product_desc   = "QEMU PenPartner Tablet";
     uc->usb_desc       = &desc_wacom;
-    uc->init           = usb_wacom_initfn;
+    uc->realize           = usb_wacom_realize;
     uc->handle_reset   = usb_wacom_handle_reset;
     uc->handle_control = usb_wacom_handle_control;
     uc->handle_data    = usb_wacom_handle_data;
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH v2 15/19] dev-wacom: convert init to realize

[Gerd Hoffmann]

>  static const VMStateDescription vmstate_usb_wacom = {
> @@ -357,7 +356,7 @@ static void usb_wacom_class_init(ObjectClass *klass, void *data)
>  
>      uc->product_desc   = "QEMU PenPartner Tablet";
>      uc->usb_desc       = &desc_wacom;
> -    uc->init           = usb_wacom_initfn;
> +    uc->realize           = usb_wacom_realize;
>      uc->handle_reset   = usb_wacom_handle_reset;
>      uc->handle_control = usb_wacom_handle_control;
>      uc->handle_data    = usb_wacom_handle_data;

The new line isn't aligned with the others (here and in lots of other
patches too).

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2 15/19] dev-wacom: convert init to realize

[Gonglei (Arei)]

Hi,

> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Friday, September 19, 2014 2:02 PM
> Subject: Re: [PATCH v2 15/19] dev-wacom: convert init to realize
> 
> 
> >  static const VMStateDescription vmstate_usb_wacom = {
> > @@ -357,7 +356,7 @@ static void usb_wacom_class_init(ObjectClass *klass,
> void *data)
> >
> >      uc->product_desc   = "QEMU PenPartner Tablet";
> >      uc->usb_desc       = &desc_wacom;
> > -    uc->init           = usb_wacom_initfn;
> > +    uc->realize           = usb_wacom_realize;
> >      uc->handle_reset   = usb_wacom_handle_reset;
> >      uc->handle_control = usb_wacom_handle_control;
> >      uc->handle_data    = usb_wacom_handle_data;
> 
> The new line isn't aligned with the others (here and in lots of other
> patches too).
> 
OK. I will check and fix them shortly. Thanks!

Best regards,
-Gonglei

> cheers,
>   Gerd
> 

[Qemu-devel] [PATCH v2 16/19] usb-audio: convert init to realize

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/dev-audio.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index 7b9957b..83ebe83 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -628,7 +628,7 @@ static void usb_audio_handle_destroy(USBDevice *dev)
     streambuf_fini(&s->out.buf);
 }
 
-static int usb_audio_initfn(USBDevice *dev)
+static void usb_audio_realize(USBDevice *dev, Error **errp)
 {
     USBAudioState *s = DO_UPCAST(USBAudioState, dev, dev);
 
@@ -651,7 +651,6 @@ static int usb_audio_initfn(USBDevice *dev)
                                 s, output_callback, &s->out.as);
     AUD_set_volume_out(s->out.voice, s->out.mute, s->out.vol[0], s->out.vol[1]);
     AUD_set_active_out(s->out.voice, 0);
-    return 0;
 }
 
 static const VMStateDescription vmstate_usb_audio = {
@@ -676,7 +675,7 @@ static void usb_audio_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_SOUND, dc->categories);
     k->product_desc   = "QEMU USB Audio Interface";
     k->usb_desc       = &desc_audio;
-    k->init           = usb_audio_initfn;
+    k->realize           = usb_audio_realize;
     k->handle_reset   = usb_audio_handle_reset;
     k->handle_control = usb_audio_handle_control;
     k->handle_data    = usb_audio_handle_data;
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 17/19] usb-redir: convert init to realize

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

In this way, all the implementations now use
error_setg instead of qerror_report for reporting error.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/redirect.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 95158b3..e2c9896 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -1361,14 +1361,14 @@ static void usbredir_init_endpoints(USBRedirDevice *dev)
     }
 }
 
-static int usbredir_initfn(USBDevice *udev)
+static void usbredir_realize(USBDevice *udev, Error **errp)
 {
     USBRedirDevice *dev = DO_UPCAST(USBRedirDevice, dev, udev);
     int i;
 
     if (dev->cs == NULL) {
-        qerror_report(QERR_MISSING_PARAMETER, "chardev");
-        return -1;
+        error_set(errp, QERR_MISSING_PARAMETER, "chardev");
+        return;
     }
 
     if (dev->filter_str) {
@@ -1376,9 +1376,9 @@ static int usbredir_initfn(USBDevice *udev)
                                            &dev->filter_rules,
                                            &dev->filter_rules_count);
         if (i) {
-            qerror_report(QERR_INVALID_PARAMETER_VALUE, "filter",
-                          "a usb device filter string");
-            return -1;
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, "filter",
+                      "a usb device filter string");
+            return;
         }
     }
 
@@ -1402,7 +1402,6 @@ static int usbredir_initfn(USBDevice *udev)
 
     qemu_add_vm_change_state_handler(usbredir_vm_state_change, dev);
     add_boot_device_path(dev->bootindex, &udev->qdev, NULL);
-    return 0;
 }
 
 static void usbredir_cleanup_device_queues(USBRedirDevice *dev)
@@ -2481,7 +2480,7 @@ static void usbredir_class_initfn(ObjectClass *klass, void *data)
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    uc->init           = usbredir_initfn;
+    uc->realize        = usbredir_realize;
     uc->product_desc   = "USB Redirection Device";
     uc->handle_destroy = usbredir_handle_destroy;
     uc->cancel_packet  = usbredir_cancel_packet;
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 18/19] usb-mtp: convert init to realize

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/dev-mtp.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 0820046..108ece8 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1060,7 +1060,7 @@ static void usb_mtp_handle_data(USBDevice *dev, USBPacket *p)
     }
 }
 
-static int usb_mtp_initfn(USBDevice *dev)
+static void usb_mtp_realize(USBDevice *dev, Error **errp)
 {
     MTPState *s = DO_UPCAST(MTPState, dev, dev);
 
@@ -1075,7 +1075,6 @@ static int usb_mtp_initfn(USBDevice *dev)
             s->desc = g_strdup("none");
         }
     }
-    return 0;
 }
 
 static const VMStateDescription vmstate_usb_mtp = {
@@ -1100,7 +1099,7 @@ static void usb_mtp_class_initfn(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
 
-    uc->init           = usb_mtp_initfn;
+    uc->realize        = usb_mtp_realize;
     uc->product_desc   = "QEMU USB MTP";
     uc->usb_desc       = &desc;
     uc->cancel_packet  = usb_mtp_cancel_packet;
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 19/19] usb-bus: remove "init" from USBDeviceClass struct

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

All usb-bus devices are realized by realize(),
remove init callback function from USBDeviceClass struct.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/bus.c     | 2 --
 include/hw/usb.h | 2 --
 2 files changed, 4 deletions(-)

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 12881cb..b375293 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -113,8 +113,6 @@ static void usb_device_realize(USBDevice *dev, Error **errp)
 
     if (klass->realize) {
         klass->realize(dev, errp);
-    } else if (klass->init) {
-        klass->init(dev);
     }
 }
 
diff --git a/include/hw/usb.h b/include/hw/usb.h
index 612f09f..8ffbba2 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -273,8 +273,6 @@ typedef void (*USBDeviceUnrealize)(USBDevice *dev, Error **errp);
 typedef struct USBDeviceClass {
     DeviceClass parent_class;
 
-    int (*init)(USBDevice *dev);
-
     USBDeviceRealize realize;
     USBDeviceUnrealize unrealize;
 
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH v2 00/19] usb: convert device init to realize

[Paolo Bonzini]

Il 18/09/2014 14:47, arei.gonglei@huawei.com ha scritto:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> DeviceClass->init is the old interface, let's convert usb
> devices to the new realize API. In this way, all the
> implementations now use error_setg instead of 
> qerror_report/error_report for reporting error.
> 
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> 
> v2 -> v1:
>  - fix PATCH 2, using qerror_report_err print error messages
>   when attach fails (Paolo)
>  - using errp instead of qerror_report_err introduced by
>   fix 1 in PATCH 12 (Paolo)
>  - fix missing return in PATCH 14 (Paolo)
>  - add 'Reviewed-by' tag for other patches
> 
> Thanks a lot for reviewing!
> 
> 
> Gonglei (19):
>   usb-storage: fix possible memory leak and missing error message
>   usb-bus: convert USBDeviceClass init to realize
>   usb-net: convert init to realize
>   libusb: convert init to realize
>   libusb: using error_report instead of fprintf
>   usb-hub: convert init to realize
>   dev-storage: convert init to realize
>   dev-storage: usring error_report instead of fprintf/printf
>   dev-uas: convert init to realize
>   dev-uas: using error_report instead of fprintf
>   dev-bluetooth: convert init to realize
>   dev-serial: convert init to realize
>   usb-ccid: convert init to realize
>   dev-hid: convert init to realize
>   dev-wacom: convert init to realize
>   usb-audio: convert init to realize
>   usb-redir: convert init to realize
>   usb-mtp: convert init to realize
>   usb-bus: remove "init" from USBDeviceClass struct
> 
>  hw/usb/bus.c                  | 79 ++++++++++++++++++++++---------------------
>  hw/usb/dev-audio.c            |  5 ++-
>  hw/usb/dev-bluetooth.c        |  6 ++--
>  hw/usb/dev-hid.c              | 27 +++++++--------
>  hw/usb/dev-hub.c              |  9 +++--
>  hw/usb/dev-mtp.c              |  5 ++-
>  hw/usb/dev-network.c          |  9 +++--
>  hw/usb/dev-serial.c           | 22 +++++++-----
>  hw/usb/dev-smartcard-reader.c |  5 ++-
>  hw/usb/dev-storage.c          | 42 ++++++++++++-----------
>  hw/usb/dev-uas.c              | 17 +++++-----
>  hw/usb/dev-wacom.c            |  5 ++-
>  hw/usb/host-libusb.c          | 33 +++++++++---------
>  hw/usb/redirect.c             | 21 +++++++-----
>  include/hw/usb.h              | 10 ++++--
>  15 files changed, 150 insertions(+), 145 deletions(-)
> 

Looks good, thanks!

Paolo

Re: [Qemu-devel] [PATCH v2 00/19] usb: convert device init to realize

[Markus Armbruster]

<arei.gonglei@huawei.com> writes:

> From: Gonglei <arei.gonglei@huawei.com>
>
> DeviceClass->init is the old interface, let's convert usb
> devices to the new realize API. In this way, all the
> implementations now use error_setg instead of 
> qerror_report/error_report for reporting error.
[...]

Thanks for doing this work.  If you can do a bit more of the same, here
are a few more device model init() methods I'd love to have converted,
because they use qerror_report_err():

hw/char/serial-pci.c            serial_pci_init()
                                multi_serial_pci_init()
hw/i386/kvm/pci-assign.c        assigned_initfn()

Re: [Qemu-devel] [PATCH v2 00/19] usb: convert device init to realize

[Paolo Bonzini]

Il 18/09/2014 19:15, Markus Armbruster ha scritto:
> 
> Thanks for doing this work.  If you can do a bit more of the same, here
> are a few more device model init() methods I'd love to have converted,
> because they use qerror_report_err():
> 
> hw/char/serial-pci.c            serial_pci_init()
>                                 multi_serial_pci_init()
> hw/i386/kvm/pci-assign.c        assigned_initfn()

Converting PCI is Gargantuan, but even something like patch 1 in this
series would be nice to have.

Paolo

Re: [Qemu-devel] [PATCH v2 00/19] usb: convert device init to realize

[Gonglei (Arei)]

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo
> Bonzini
> Sent: Friday, September 19, 2014 2:42 AM
> Subject: Re: [PATCH v2 00/19] usb: convert device init to realize
> 
> Il 18/09/2014 19:15, Markus Armbruster ha scritto:
> >
> > Thanks for doing this work.  If you can do a bit more of the same, here
> > are a few more device model init() methods I'd love to have converted,
> > because they use qerror_report_err():
> >
> > hw/char/serial-pci.c            serial_pci_init()
> >                                 multi_serial_pci_init()
> > hw/i386/kvm/pci-assign.c        assigned_initfn()
> 
> Converting PCI is Gargantuan, 

Yes. AFAICT all PCI devices are using init() at present in QEMU.

> but even something like patch 1 in this
> series would be nice to have.
> 
I think you meant method of PATCH 2, right?
 
Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v2 00/19] usb: convert device init to realize

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 18/09/2014 19:15, Markus Armbruster ha scritto:
>> 
>> Thanks for doing this work.  If you can do a bit more of the same, here
>> are a few more device model init() methods I'd love to have converted,
>> because they use qerror_report_err():
>> 
>> hw/char/serial-pci.c            serial_pci_init()
>>                                 multi_serial_pci_init()
>> hw/i386/kvm/pci-assign.c        assigned_initfn()
>
> Converting PCI is Gargantuan, but even something like patch 1 in this
> series would be nice to have.

Converting all the PCI devices in one go is Gargantuan indeed.  Could we
enable incremental conversion?

Re: [Qemu-devel] [PATCH v2 00/19] usb: convert device init to realize

[Gerd Hoffmann]

On Do, 2014-09-18 at 20:47 +0800, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> DeviceClass->init is the old interface, let's convert usb
> devices to the new realize API. In this way, all the
> implementations now use error_setg instead of 
> qerror_report/error_report for reporting error.

Looks good overall.  One minor style nit, and there is the usb-serial
issue pointed out by paolo, I'd like to see them handled.  I don't care
much whenever you do than with a v3 or 2-3 incremental patches to this
one.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2 00/19] usb: convert device init to realize

[Gonglei (Arei)]

> From: Gerd Hoffmann [mailto:kraxel@redhat.com]
> Sent: Friday, September 19, 2014 2:06 PM
> To: Gonglei (Arei)
> Subject: Re: [PATCH v2 00/19] usb: convert device init to realize
> 
> On Do, 2014-09-18 at 20:47 +0800, arei.gonglei@huawei.com wrote:
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> > DeviceClass->init is the old interface, let's convert usb
> > devices to the new realize API. In this way, all the
> > implementations now use error_setg instead of
> > qerror_report/error_report for reporting error.
> 
> Looks good overall.  One minor style nit, and there is the usb-serial
> issue pointed out by paolo, I'd like to see them handled.  I don't care
> much whenever you do than with a v3 or 2-3 incremental patches to this
> one.
> 
Got it. Thanks. :)

Best regards,
-Gonglei