Re: [Qemu-devel] [PATCH] usb: Resolve warnings about unassigned bus on usb device creation

[Jan Kiszka]

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 <address@hidden>
> ---
>  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

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux