Patchwork usb: Resolve warnings about unassigned bus on usb device creation

login
register
mail settings
Submitter Jan Kiszka
Date Feb. 2, 2012, 5:59 p.m.
Message ID <4F2ACF0F.2000803@siemens.com>
Download mbox | patch
Permalink /patch/139187/
State New
Headers show

Comments

Jan Kiszka - Feb. 2, 2012, 5:59 p.m.
When creating an USB device the old way, there is no way to specify the
target bus. Thus the warning issued by usb_create makes no sense and
rather confuses our users.

Resolve this by passing a bus reference to the usbdevice_init handler
and letting those handlers forward it to usb_create.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/usb-bt.c     |    4 ++--
 hw/usb-bus.c    |   18 ++++--------------
 hw/usb-msd.c    |    4 ++--
 hw/usb-net.c    |    4 ++--
 hw/usb-serial.c |    8 ++++----
 hw/usb.h        |    7 ++++---
 usb-bsd.c       |    4 ++--
 usb-linux.c     |    4 ++--
 vl.c            |    7 ++++---
 9 files changed, 26 insertions(+), 34 deletions(-)
Jan Kiszka - Feb. 27, 2012, 1:57 p.m.
On 2012-02-02 18:59, Jan Kiszka wrote:
> When creating an USB device the old way, there is no way to specify the
> target bus. Thus the warning issued by usb_create makes no sense and
> rather confuses our users.
> 
> Resolve this by passing a bus reference to the usbdevice_init handler
> and letting those handlers forward it to usb_create.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  hw/usb-bt.c     |    4 ++--
>  hw/usb-bus.c    |   18 ++++--------------
>  hw/usb-msd.c    |    4 ++--
>  hw/usb-net.c    |    4 ++--
>  hw/usb-serial.c |    8 ++++----
>  hw/usb.h        |    7 ++++---
>  usb-bsd.c       |    4 ++--
>  usb-linux.c     |    4 ++--
>  vl.c            |    7 ++++---
>  9 files changed, 26 insertions(+), 34 deletions(-)
> 
> diff --git a/hw/usb-bt.c b/hw/usb-bt.c
> index bf8c470..291242f 100644
> --- a/hw/usb-bt.c
> +++ b/hw/usb-bt.c
> @@ -498,14 +498,14 @@ static int usb_bt_initfn(USBDevice *dev)
>      return 0;
>  }
>  
> -USBDevice *usb_bt_init(HCIInfo *hci)
> +USBDevice *usb_bt_init(USBBus *bus, HCIInfo *hci)
>  {
>      USBDevice *dev;
>      struct USBBtState *s;
>  
>      if (!hci)
>          return NULL;
> -    dev = usb_create_simple(NULL /* FIXME */, "usb-bt-dongle");
> +    dev = usb_create_simple(bus, "usb-bt-dongle");
>      if (!dev) {
>          return NULL;
>      }
> diff --git a/hw/usb-bus.c b/hw/usb-bus.c
> index aeef908..aae5b0c 100644
> --- a/hw/usb-bus.c
> +++ b/hw/usb-bus.c
> @@ -203,14 +203,15 @@ typedef struct LegacyUSBFactory
>  {
>      const char *name;
>      const char *usbdevice_name;
> -    USBDevice *(*usbdevice_init)(const char *params);
> +    USBDevice *(*usbdevice_init)(USBBus *bus, const char *params);
>  } LegacyUSBFactory;
>  
>  static GSList *legacy_usb_factory;
>  
>  void usb_qdev_register(DeviceInfo *info,
>                         const char *usbdevice_name,
> -                       USBDevice *(*usbdevice_init)(const char *params))
> +                       USBDevice *(*usbdevice_init)(USBBus *bus,
> +                                                    const char *params))
>  {
>      info->bus_info = &usb_bus_info;
>      info->init     = usb_qdev_init;
> @@ -231,17 +232,6 @@ USBDevice *usb_create(USBBus *bus, const char *name)
>  {
>      DeviceState *dev;
>  
> -#if 1
> -    /* temporary stopgap until all usb is properly qdev-ified */
> -    if (!bus) {
> -        bus = usb_bus_find(-1);
> -        if (!bus)
> -            return NULL;
> -        error_report("%s: no bus specified, using \"%s\" for \"%s\"",
> -                __FUNCTION__, bus->qbus.name, name);
> -    }
> -#endif
> -
>      dev = qdev_create(&bus->qbus, name);
>      return USB_DEVICE(dev);
>  }
> @@ -572,7 +562,7 @@ USBDevice *usbdevice_create(const char *cmdline)
>          }
>          return usb_create_simple(bus, f->name);
>      }
> -    return f->usbdevice_init(params);
> +    return f->usbdevice_init(bus, params);
>  }
>  
>  static TypeInfo usb_device_type_info = {
> diff --git a/hw/usb-msd.c b/hw/usb-msd.c
> index ceb01e0..823f072 100644
> --- a/hw/usb-msd.c
> +++ b/hw/usb-msd.c
> @@ -568,7 +568,7 @@ static int usb_msd_initfn(USBDevice *dev)
>      return 0;
>  }
>  
> -static USBDevice *usb_msd_init(const char *filename)
> +static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
>  {
>      static int nr=0;
>      char id[8];
> @@ -611,7 +611,7 @@ static USBDevice *usb_msd_init(const char *filename)
>      }
>  
>      /* create guest device */
> -    dev = usb_create(NULL /* FIXME */, "usb-storage");
> +    dev = usb_create(bus, "usb-storage");
>      if (!dev) {
>          return NULL;
>      }
> diff --git a/hw/usb-net.c b/hw/usb-net.c
> index 57b58ac..c9884e1 100644
> --- a/hw/usb-net.c
> +++ b/hw/usb-net.c
> @@ -1353,7 +1353,7 @@ static int usb_net_initfn(USBDevice *dev)
>      return 0;
>  }
>  
> -static USBDevice *usb_net_init(const char *cmdline)
> +static USBDevice *usb_net_init(USBBus *bus, const char *cmdline)
>  {
>      USBDevice *dev;
>      QemuOpts *opts;
> @@ -1371,7 +1371,7 @@ static USBDevice *usb_net_init(const char *cmdline)
>          return NULL;
>      }
>  
> -    dev = usb_create(NULL /* FIXME */, "usb-net");
> +    dev = usb_create(bus, "usb-net");
>      if (!dev) {
>          return NULL;
>      }
> diff --git a/hw/usb-serial.c b/hw/usb-serial.c
> index de49607..8c7861d 100644
> --- a/hw/usb-serial.c
> +++ b/hw/usb-serial.c
> @@ -492,7 +492,7 @@ static int usb_serial_initfn(USBDevice *dev)
>      return 0;
>  }
>  
> -static USBDevice *usb_serial_init(const char *filename)
> +static USBDevice *usb_serial_init(USBBus *bus, const char *filename)
>  {
>      USBDevice *dev;
>      CharDriverState *cdrv;
> @@ -535,7 +535,7 @@ static USBDevice *usb_serial_init(const char *filename)
>      if (!cdrv)
>          return NULL;
>  
> -    dev = usb_create(NULL /* FIXME */, "usb-serial");
> +    dev = usb_create(bus, "usb-serial");
>      if (!dev) {
>          return NULL;
>      }
> @@ -549,7 +549,7 @@ static USBDevice *usb_serial_init(const char *filename)
>      return dev;
>  }
>  
> -static USBDevice *usb_braille_init(const char *unused)
> +static USBDevice *usb_braille_init(USBBus *bus, const char *unused)
>  {
>      USBDevice *dev;
>      CharDriverState *cdrv;
> @@ -558,7 +558,7 @@ static USBDevice *usb_braille_init(const char *unused)
>      if (!cdrv)
>          return NULL;
>  
> -    dev = usb_create(NULL /* FIXME */, "usb-braille");
> +    dev = usb_create(bus, "usb-braille");
>      qdev_prop_set_chr(&dev->qdev, "chardev", cdrv);
>      qdev_init_nofail(&dev->qdev);
>  
> diff --git a/hw/usb.h b/hw/usb.h
> index 5b9badb..debc1ab 100644
> --- a/hw/usb.h
> +++ b/hw/usb.h
> @@ -362,12 +362,12 @@ int set_usb_string(uint8_t *buf, const char *str);
>  void usb_send_msg(USBDevice *dev, int msg);
>  
>  /* usb-linux.c */
> -USBDevice *usb_host_device_open(const char *devname);
> +USBDevice *usb_host_device_open(USBBus *bus, const char *devname);
>  int usb_host_device_close(const char *devname);
>  void usb_host_info(Monitor *mon);
>  
>  /* usb-bt.c */
> -USBDevice *usb_bt_init(HCIInfo *hci);
> +USBDevice *usb_bt_init(USBBus *bus, HCIInfo *hci);
>  
>  /* usb ports of the VM */
>  
> @@ -420,7 +420,8 @@ void usb_bus_new(USBBus *bus, USBBusOps *ops, DeviceState *host);
>  USBBus *usb_bus_find(int busnr);
>  void usb_qdev_register(DeviceInfo *info,
>                         const char *usbdevice_name,
> -                       USBDevice *(*usbdevice_init)(const char *params));
> +                       USBDevice *(*usbdevice_init)(USBBus *bus,
> +                                                    const char *params));
>  USBDevice *usb_create(USBBus *bus, const char *name);
>  USBDevice *usb_create_simple(USBBus *bus, const char *name);
>  USBDevice *usbdevice_create(const char *cmdline);
> diff --git a/usb-bsd.c b/usb-bsd.c
> index 2c6afc8..fcd1438 100644
> --- a/usb-bsd.c
> +++ b/usb-bsd.c
> @@ -298,7 +298,7 @@ static int usb_host_initfn(USBDevice *dev)
>      return 0;
>  }
>  
> -USBDevice *usb_host_device_open(const char *devname)
> +USBDevice *usb_host_device_open(USBBus *guest_bus, const char *devname)
>  {
>      struct usb_device_info bus_info, dev_info;
>      USBDevice *d = NULL, *ret = NULL;
> @@ -358,7 +358,7 @@ USBDevice *usb_host_device_open(const char *devname)
>          goto fail_dfd;
>      }
>  
> -    d = usb_create(NULL /* FIXME */, "usb-host");
> +    d = usb_create(guest_bus, "usb-host");
>      dev = DO_UPCAST(USBHostDevice, dev, d);
>  
>      if (dev_info.udi_speed == 1) {
> diff --git a/usb-linux.c b/usb-linux.c
> index 31810f6..7cab488 100644
> --- a/usb-linux.c
> +++ b/usb-linux.c
> @@ -1438,13 +1438,13 @@ static void usb_host_register_devices(void)
>  }
>  device_init(usb_host_register_devices)
>  
> -USBDevice *usb_host_device_open(const char *devname)
> +USBDevice *usb_host_device_open(USBBus *bus, const char *devname)
>  {
>      struct USBAutoFilter filter;
>      USBDevice *dev;
>      char *p;
>  
> -    dev = usb_create(NULL /* FIXME */, "usb-host");
> +    dev = usb_create(bus, "usb-host");
>  
>      if (strstr(devname, "auto:")) {
>          if (parse_filter(devname, &filter) < 0) {
> diff --git a/vl.c b/vl.c
> index 2d464cf..cb8844d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1052,12 +1052,13 @@ static int usb_device_add(const char *devname)
>  #ifndef CONFIG_LINUX
>      /* only the linux version is qdev-ified, usb-bsd still needs this */
>      if (strstart(devname, "host:", &p)) {
> -        dev = usb_host_device_open(p);
> +        dev = usb_host_device_open(usb_bus_find(-1), p);
>      } else
>  #endif
>      if (!strcmp(devname, "bt") || strstart(devname, "bt:", &p)) {
> -        dev = usb_bt_init(devname[2] ? hci_init(p) :
> -                        bt_new_hci(qemu_find_bt_vlan(0)));
> +        dev = usb_bt_init(usb_bus_find(-1),
> +                          devname[2] ? hci_init(p)
> +                                     : bt_new_hci(qemu_find_bt_vlan(0)));
>      } else {
>          return -1;
>      }

Ping.

Jan
Gerd Hoffmann - Feb. 27, 2012, 2:03 p.m.
On 02/27/12 14:57, Jan Kiszka wrote:
> On 2012-02-02 18:59, Jan Kiszka wrote:
>> When creating an USB device the old way, there is no way to specify the
>> target bus. Thus the warning issued by usb_create makes no sense and
>> rather confuses our users.
>>
>> Resolve this by passing a bus reference to the usbdevice_init handler
>> and letting those handlers forward it to usb_create.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Ping.
> 

Somehow slipped through.  Looks good, but needs a rebase.

cheers,
  Gerd

Patch

diff --git a/hw/usb-bt.c b/hw/usb-bt.c
index bf8c470..291242f 100644
--- a/hw/usb-bt.c
+++ b/hw/usb-bt.c
@@ -498,14 +498,14 @@  static int usb_bt_initfn(USBDevice *dev)
     return 0;
 }
 
-USBDevice *usb_bt_init(HCIInfo *hci)
+USBDevice *usb_bt_init(USBBus *bus, HCIInfo *hci)
 {
     USBDevice *dev;
     struct USBBtState *s;
 
     if (!hci)
         return NULL;
-    dev = usb_create_simple(NULL /* FIXME */, "usb-bt-dongle");
+    dev = usb_create_simple(bus, "usb-bt-dongle");
     if (!dev) {
         return NULL;
     }
diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index aeef908..aae5b0c 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -203,14 +203,15 @@  typedef struct LegacyUSBFactory
 {
     const char *name;
     const char *usbdevice_name;
-    USBDevice *(*usbdevice_init)(const char *params);
+    USBDevice *(*usbdevice_init)(USBBus *bus, const char *params);
 } LegacyUSBFactory;
 
 static GSList *legacy_usb_factory;
 
 void usb_qdev_register(DeviceInfo *info,
                        const char *usbdevice_name,
-                       USBDevice *(*usbdevice_init)(const char *params))
+                       USBDevice *(*usbdevice_init)(USBBus *bus,
+                                                    const char *params))
 {
     info->bus_info = &usb_bus_info;
     info->init     = usb_qdev_init;
@@ -231,17 +232,6 @@  USBDevice *usb_create(USBBus *bus, const char *name)
 {
     DeviceState *dev;
 
-#if 1
-    /* temporary stopgap until all usb is properly qdev-ified */
-    if (!bus) {
-        bus = usb_bus_find(-1);
-        if (!bus)
-            return NULL;
-        error_report("%s: no bus specified, using \"%s\" for \"%s\"",
-                __FUNCTION__, bus->qbus.name, name);
-    }
-#endif
-
     dev = qdev_create(&bus->qbus, name);
     return USB_DEVICE(dev);
 }
@@ -572,7 +562,7 @@  USBDevice *usbdevice_create(const char *cmdline)
         }
         return usb_create_simple(bus, f->name);
     }
-    return f->usbdevice_init(params);
+    return f->usbdevice_init(bus, params);
 }
 
 static TypeInfo usb_device_type_info = {
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index ceb01e0..823f072 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -568,7 +568,7 @@  static int usb_msd_initfn(USBDevice *dev)
     return 0;
 }
 
-static USBDevice *usb_msd_init(const char *filename)
+static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
 {
     static int nr=0;
     char id[8];
@@ -611,7 +611,7 @@  static USBDevice *usb_msd_init(const char *filename)
     }
 
     /* create guest device */
-    dev = usb_create(NULL /* FIXME */, "usb-storage");
+    dev = usb_create(bus, "usb-storage");
     if (!dev) {
         return NULL;
     }
diff --git a/hw/usb-net.c b/hw/usb-net.c
index 57b58ac..c9884e1 100644
--- a/hw/usb-net.c
+++ b/hw/usb-net.c
@@ -1353,7 +1353,7 @@  static int usb_net_initfn(USBDevice *dev)
     return 0;
 }
 
-static USBDevice *usb_net_init(const char *cmdline)
+static USBDevice *usb_net_init(USBBus *bus, const char *cmdline)
 {
     USBDevice *dev;
     QemuOpts *opts;
@@ -1371,7 +1371,7 @@  static USBDevice *usb_net_init(const char *cmdline)
         return NULL;
     }
 
-    dev = usb_create(NULL /* FIXME */, "usb-net");
+    dev = usb_create(bus, "usb-net");
     if (!dev) {
         return NULL;
     }
diff --git a/hw/usb-serial.c b/hw/usb-serial.c
index de49607..8c7861d 100644
--- a/hw/usb-serial.c
+++ b/hw/usb-serial.c
@@ -492,7 +492,7 @@  static int usb_serial_initfn(USBDevice *dev)
     return 0;
 }
 
-static USBDevice *usb_serial_init(const char *filename)
+static USBDevice *usb_serial_init(USBBus *bus, const char *filename)
 {
     USBDevice *dev;
     CharDriverState *cdrv;
@@ -535,7 +535,7 @@  static USBDevice *usb_serial_init(const char *filename)
     if (!cdrv)
         return NULL;
 
-    dev = usb_create(NULL /* FIXME */, "usb-serial");
+    dev = usb_create(bus, "usb-serial");
     if (!dev) {
         return NULL;
     }
@@ -549,7 +549,7 @@  static USBDevice *usb_serial_init(const char *filename)
     return dev;
 }
 
-static USBDevice *usb_braille_init(const char *unused)
+static USBDevice *usb_braille_init(USBBus *bus, const char *unused)
 {
     USBDevice *dev;
     CharDriverState *cdrv;
@@ -558,7 +558,7 @@  static USBDevice *usb_braille_init(const char *unused)
     if (!cdrv)
         return NULL;
 
-    dev = usb_create(NULL /* FIXME */, "usb-braille");
+    dev = usb_create(bus, "usb-braille");
     qdev_prop_set_chr(&dev->qdev, "chardev", cdrv);
     qdev_init_nofail(&dev->qdev);
 
diff --git a/hw/usb.h b/hw/usb.h
index 5b9badb..debc1ab 100644
--- a/hw/usb.h
+++ b/hw/usb.h
@@ -362,12 +362,12 @@  int set_usb_string(uint8_t *buf, const char *str);
 void usb_send_msg(USBDevice *dev, int msg);
 
 /* usb-linux.c */
-USBDevice *usb_host_device_open(const char *devname);
+USBDevice *usb_host_device_open(USBBus *bus, const char *devname);
 int usb_host_device_close(const char *devname);
 void usb_host_info(Monitor *mon);
 
 /* usb-bt.c */
-USBDevice *usb_bt_init(HCIInfo *hci);
+USBDevice *usb_bt_init(USBBus *bus, HCIInfo *hci);
 
 /* usb ports of the VM */
 
@@ -420,7 +420,8 @@  void usb_bus_new(USBBus *bus, USBBusOps *ops, DeviceState *host);
 USBBus *usb_bus_find(int busnr);
 void usb_qdev_register(DeviceInfo *info,
                        const char *usbdevice_name,
-                       USBDevice *(*usbdevice_init)(const char *params));
+                       USBDevice *(*usbdevice_init)(USBBus *bus,
+                                                    const char *params));
 USBDevice *usb_create(USBBus *bus, const char *name);
 USBDevice *usb_create_simple(USBBus *bus, const char *name);
 USBDevice *usbdevice_create(const char *cmdline);
diff --git a/usb-bsd.c b/usb-bsd.c
index 2c6afc8..fcd1438 100644
--- a/usb-bsd.c
+++ b/usb-bsd.c
@@ -298,7 +298,7 @@  static int usb_host_initfn(USBDevice *dev)
     return 0;
 }
 
-USBDevice *usb_host_device_open(const char *devname)
+USBDevice *usb_host_device_open(USBBus *guest_bus, const char *devname)
 {
     struct usb_device_info bus_info, dev_info;
     USBDevice *d = NULL, *ret = NULL;
@@ -358,7 +358,7 @@  USBDevice *usb_host_device_open(const char *devname)
         goto fail_dfd;
     }
 
-    d = usb_create(NULL /* FIXME */, "usb-host");
+    d = usb_create(guest_bus, "usb-host");
     dev = DO_UPCAST(USBHostDevice, dev, d);
 
     if (dev_info.udi_speed == 1) {
diff --git a/usb-linux.c b/usb-linux.c
index 31810f6..7cab488 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1438,13 +1438,13 @@  static void usb_host_register_devices(void)
 }
 device_init(usb_host_register_devices)
 
-USBDevice *usb_host_device_open(const char *devname)
+USBDevice *usb_host_device_open(USBBus *bus, const char *devname)
 {
     struct USBAutoFilter filter;
     USBDevice *dev;
     char *p;
 
-    dev = usb_create(NULL /* FIXME */, "usb-host");
+    dev = usb_create(bus, "usb-host");
 
     if (strstr(devname, "auto:")) {
         if (parse_filter(devname, &filter) < 0) {
diff --git a/vl.c b/vl.c
index 2d464cf..cb8844d 100644
--- a/vl.c
+++ b/vl.c
@@ -1052,12 +1052,13 @@  static int usb_device_add(const char *devname)
 #ifndef CONFIG_LINUX
     /* only the linux version is qdev-ified, usb-bsd still needs this */
     if (strstart(devname, "host:", &p)) {
-        dev = usb_host_device_open(p);
+        dev = usb_host_device_open(usb_bus_find(-1), p);
     } else
 #endif
     if (!strcmp(devname, "bt") || strstart(devname, "bt:", &p)) {
-        dev = usb_bt_init(devname[2] ? hci_init(p) :
-                        bt_new_hci(qemu_find_bt_vlan(0)));
+        dev = usb_bt_init(usb_bus_find(-1),
+                          devname[2] ? hci_init(p)
+                                     : bt_new_hci(qemu_find_bt_vlan(0)));
     } else {
         return -1;
     }