Re: [Qemu-devel] [PATCH] monitor: add usb_detach

[Luiz Capitulino]

On Tue, 5 Oct 2010 16:40:29 +0200
Alon Levy <address@hidden> wrote:

> Signed-off-by: Alon Levy <address@hidden>

This needs to be rebased as it now conflicts with my last series merged
on master.

I have some minor comments (below), but in general seems ok to me. However,
I'd like to get an ACK from someone familiar with QEMU's USB before
applying to the monitor's queue.

Anthony? Gerd?

> 
> ---
>  qemu-monitor.hx |   17 +++++++++++++++++
>  sysemu.h        |    1 +
>  vl.c            |   41 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 59 insertions(+), 0 deletions(-)
> 
> diff --git a/qemu-monitor.hx b/qemu-monitor.hx
> index 49bcd8d..9c792a9 100644
> --- a/qemu-monitor.hx
> +++ b/qemu-monitor.hx
> @@ -711,6 +711,23 @@ command @code{info usb} to see the devices you can 
> remove.
>  ETEXI
>  
>      {
> +        .name       = "usb_detach",
> +        .args_type  = "devname:s",
> +        .params     = "device",
> +        .help       = "detach USB device 'bus.addr'",
> +        .mhandler.cmd = do_usb_detach,
> +    },
> +
> +STEXI
> address@hidden usb_detach @var{devname}
> address@hidden usb_detach
> +
> +Detach the USB device @var{devname} from the QEMU virtual USB
> +hub. @var{devname} has the syntax @code{bus.addr}. Use the monitor
> +command @code{info usb} to see the devices you can detach.
> +ETEXI
> +
> +    {
>          .name       = "device_add",
>          .args_type  = "device:O",
>          .params     = "driver[,prop=value][,...]",
> diff --git a/sysemu.h b/sysemu.h
> index a1f6466..ac68863 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -183,6 +183,7 @@ extern struct soundhw soundhw[];
>  
>  void do_usb_add(Monitor *mon, const QDict *qdict);
>  void do_usb_del(Monitor *mon, const QDict *qdict);
> +void do_usb_detach(Monitor *mon, const QDict *qdict);
>  void usb_info(Monitor *mon);
>  
>  void rtc_change_mon_event(struct tm *tm);
> diff --git a/vl.c b/vl.c
> index d352d18..6cfa009 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -891,6 +891,47 @@ void do_usb_del(Monitor *mon, const QDict *qdict)
>      }
>  }
>  
> +static USBDevice *usb_device_from_bus_dot_addr(const char *devname)
> +{
> +    int bus_num, addr;
> +    const char *p;
> +    USBBus *bus;
> +    USBPort *port;
> +
> +    if (!usb_enabled) {
> +        return NULL;
> +    }
> +    p = strchr(devname, '.');
> +    if (!p) {
> +        return NULL;
> +    }
> +    bus_num = strtoul(devname, NULL, 0);
> +    addr = strtoul(p + 1, NULL, 0);

Checking stroul()'s return would avoid nasty surprises.

> +    bus = usb_bus_find(bus_num);
> +    if (!bus) {
> +        return NULL;
> +    }
> +    QTAILQ_FOREACH(port, &bus->used, next) {
> +        if (port->dev->addr == addr) {
> +            break;
> +        }
> +    }
> +    if (!port) {
> +        return NULL;
> +    }
> +    return port->dev;
> +}
> +
> +void do_usb_detach(Monitor *mon, const QDict *qdict)
> +{
> +    const char *devname = qdict_get_str(qdict, "devname");
> +    USBDevice *dev = usb_device_from_bus_dot_addr(devname);
> +
> +    if (dev == NULL || usb_device_detach(dev) < 0) {
> +        error_report("could not detach USB device '%s'", devname);
> +    }

You can improve the error message if you check for dev and detach
separately. Minor.

> +}
> +
>  /***********************************************************/
>  /* PCMCIA/Cardbus */
>