Patchwork [3/3] monitor: add usb_attach and usb_detach

login
register
mail settings
Submitter Alon Levy
Date Oct. 19, 2010, 10:33 a.m.
Message ID <1287484411-13611-4-git-send-email-alevy@redhat.com>
Download mbox | patch
Permalink /patch/68311/
State New
Headers show

Comments

Alon Levy - Oct. 19, 2010, 10:33 a.m.
---
 hmp-commands.hx |   34 ++++++++++++++++++++++++++++++++++
 sysemu.h        |    2 ++
 vl.c            |   31 +++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 0 deletions(-)
Gerd Hoffmann - Oct. 19, 2010, 1:35 p.m.
Hi,

> +        .help       = "attach USB device 'bus.addr'",

> +@item usb_attach @var{devname}

/me sees a mismatch here.

There is still the use case question.  Also note that this might have 
unwanted side effects when drivers automagically attach/detach devices 
like usb-host.

Having this purely for debugging/troubleshooting purposes would be fine 
with me, but the documentation should clearly say so.

cheers,
   Gerd
Luiz Capitulino - Oct. 20, 2010, 4:24 p.m.
On Tue, 19 Oct 2010 15:35:01 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

>    Hi,
> 
> > +        .help       = "attach USB device 'bus.addr'",
> 
> > +@item usb_attach @var{devname}
> 
> /me sees a mismatch here.
> 
> There is still the use case question.  Also note that this might have 
> unwanted side effects when drivers automagically attach/detach devices 
> like usb-host.

Alon, did you check this? I hope it doesn't blowup.

> Having this purely for debugging/troubleshooting purposes would be fine 
> with me, but the documentation should clearly say so.

Is this useful in production or only in the development of new devices? If
it's the letter, then I would even prefer enabling them only when DEBUG
is enabled.
Gerd Hoffmann - Oct. 21, 2010, 10 a.m.
On 10/20/10 18:24, Luiz Capitulino wrote:
> On Tue, 19 Oct 2010 15:35:01 +0200
> Gerd Hoffmann<kraxel@redhat.com>  wrote:
>
>>     Hi,
>>
>>> +        .help       = "attach USB device 'bus.addr'",
>>
>>> +@item usb_attach @var{devname}
>>
>> /me sees a mismatch here.
>>
>> There is still the use case question.  Also note that this might have
>> unwanted side effects when drivers automagically attach/detach devices
>> like usb-host.
>
> Alon, did you check this? I hope it doesn't blowup.
>
>> Having this purely for debugging/troubleshooting purposes would be fine
>> with me, but the documentation should clearly say so.
>
> Is this useful in production or only in the development of new devices?

I'd say more a development tool.

> If
> it's the letter, then I would even prefer enabling them only when DEBUG
> is enabled.

Makes sense indeed.

cheers,
   Gerd
Alon Levy - Oct. 21, 2010, 10:58 a.m.
On Thu, Oct 21, 2010 at 12:00:12PM +0200, Gerd Hoffmann wrote:
> On 10/20/10 18:24, Luiz Capitulino wrote:
> >On Tue, 19 Oct 2010 15:35:01 +0200
> >Gerd Hoffmann<kraxel@redhat.com>  wrote:
> >
> >>    Hi,
> >>
> >>>+        .help       = "attach USB device 'bus.addr'",
> >>
> >>>+@item usb_attach @var{devname}
> >>
> >>/me sees a mismatch here.
> >>
> >>There is still the use case question.  Also note that this might have
> >>unwanted side effects when drivers automagically attach/detach devices
> >>like usb-host.
> >
> >Alon, did you check this? I hope it doesn't blowup.
> >
> >>Having this purely for debugging/troubleshooting purposes would be fine
> >>with me, but the documentation should clearly say so.
> >
> >Is this useful in production or only in the development of new devices?
> 
> I'd say more a development tool.
> 
> >If
> >it's the letter, then I would even prefer enabling them only when DEBUG
> >is enabled.
> 
> Makes sense indeed.

I've sent a third version with fixes for the args_type you pointed out and the suggested default disabled behavior (I also added a flag to enable just this command, but made sure --enable-debug enables it too).

> 
> cheers,
>   Gerd
Alon Levy - Oct. 21, 2010, 7:18 p.m.
On Wed, Oct 20, 2010 at 02:24:20PM -0200, Luiz Capitulino wrote:
> On Tue, 19 Oct 2010 15:35:01 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> >    Hi,
> > 
> > > +        .help       = "attach USB device 'bus.addr'",
> > 
> > > +@item usb_attach @var{devname}
> > 
> > /me sees a mismatch here.
> > 
> > There is still the use case question.  Also note that this might have 
> > unwanted side effects when drivers automagically attach/detach devices 
> > like usb-host.
> 
> Alon, did you check this? I hope it doesn't blowup.

I just did, I created
bus
 usb-serial
 usb-hub id=hub1
  usb-serial
  .. (6 more)
  usb-hub id=hub2
   usb-serial
   (6 more)

(verified the guest sees this tree using lsusb -t)

I did usb_detach hub1, and as expected only the first usb-serial remained.
I then did a usb_attach hub1, and no devices appeared. A second usb_attach
gets the expected warning (you tried to attach a device twice).

So It doesn't blow up, but an info qtree showed it's because the rest of
the devices (those below hub1) were still attached. So I think the detach
should take care to detach anything below. If after that attach still won't
cause attach then hub attaches need fixing as well (it should be just causing
attach for devices under it, since the whole protocol for an attach is already
correctly implemented).

> 
> > Having this purely for debugging/troubleshooting purposes would be fine 
> > with me, but the documentation should clearly say so.
> 
> Is this useful in production or only in the development of new devices? If
> it's the letter, then I would even prefer enabling them only when DEBUG
> is enabled.
>

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 81999aa..660205c 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -517,6 +517,40 @@  command @code{info usb} to see the devices you can remove.
 ETEXI
 
     {
+        .name       = "usb_attach",
+        .args_type  = "id:s",
+        .params     = "device",
+        .help       = "attach USB device 'bus.addr'",
+        .mhandler.cmd = do_usb_attach,
+    },
+
+STEXI
+@item usb_attach @var{devname}
+@findex usb_attach
+
+Attach the USB device @var{devname} to 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 attach.
+ETEXI
+
+    {
+        .name       = "usb_detach",
+        .args_type  = "id:s",
+        .params     = "device",
+        .help       = "remove USB device 'bus.addr'",
+        .mhandler.cmd = do_usb_detach,
+    },
+
+STEXI
+@item usb_detach @var{devname}
+@findex 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 b81a70e..1dc0e58 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -182,6 +182,8 @@  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_attach(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 df414ef..35db6c8 100644
--- a/vl.c
+++ b/vl.c
@@ -894,6 +894,37 @@  void do_usb_del(Monitor *mon, const QDict *qdict)
     }
 }
 
+void do_usb_attach(Monitor *mon, const QDict *qdict)
+{
+    const char *id = qdict_get_str(qdict, "id");
+    USBDevice *dev;
+
+    dev = usb_device_by_id(id);
+
+    if (dev == NULL) {
+        error_report("no such USB device '%s'", id);
+        return;
+    }
+    if (usb_device_attach(dev) < 0) {
+        error_report("could not attach USB device '%s'", id);
+    }
+}
+
+void do_usb_detach(Monitor *mon, const QDict *qdict)
+{
+    const char *id = qdict_get_str(qdict, "id");
+    USBDevice *dev;
+
+    dev = usb_device_by_id(id);
+    if (dev == NULL) {
+        error_report("no such USB device '%s'", id);
+        return;
+    }
+    if (usb_device_detach(dev) < 0) {
+        error_report("could not detach USB device '%s'", id);
+    }
+}
+
 /***********************************************************/
 /* PCMCIA/Cardbus */