From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=35420 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PAIfu-000262-KG for qemu-devel@nongnu.org; Mon, 25 Oct 2010 04:46:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PAIfD-0002rt-0Q for qemu-devel@nongnu.org; Mon, 25 Oct 2010 04:44:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42967) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PAIfC-0002rF-Q6 for qemu-devel@nongnu.org; Mon, 25 Oct 2010 04:44:18 -0400 Date: Mon, 25 Oct 2010 10:44:08 +0200 From: Alon Levy Subject: Re: [Qemu-devel] [PATCH 0/3] add usb_detach and usb_attach (v3) Message-ID: <20101025084408.GA1410@playa.tlv.redhat.com> References: <1287642991-21114-1-git-send-email-alevy@redhat.com> <4CC03A2D.9030105@redhat.com> <4CC03C6F.4020304@codemonkey.ws> <20101021132723.GE22958@playa.tlv.redhat.com> <20101022104822.5f608f13@doriath> <4CC189A6.30807@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4CC189A6.30807@codemonkey.ws> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: qemu-devel@nongnu.org, Gerd Hoffmann , Luiz Capitulino On Fri, Oct 22, 2010 at 07:55:02AM -0500, Anthony Liguori wrote: > On 10/22/2010 07:48 AM, Luiz Capitulino wrote: > >On Thu, 21 Oct 2010 15:27:23 +0200 > >Alon Levy wrote: > > > >>On Thu, Oct 21, 2010 at 08:13:19AM -0500, Anthony Liguori wrote: > >>>On 10/21/2010 08:03 AM, Gerd Hoffmann wrote: > >>>>On 10/21/10 08:36, Alon Levy wrote: > >>>>>v2->v3 changes: > >>>>> * add configure parameter > >>>>> * fix docs > >>>>> > >>>>>v2 message: > >>>>>This patchset uses id like device_del for attaching/detaching usb > >>>>>devices. The first two patches ready the way: > >>>>> 1. makes qdev_find_recursive non static and in qdev.h > >>>>> 2. adds a usb_device_by_id which goes over the usb buses calling > >>>>> qdev_find_recursive > >>>>> 3. adds the commands that use usb_device_by_id > >>>>> > >>>>>Alon Levy (3): > >>>>> qdev: make qdev_find_recursive public > >>>>> usb: add public usb_device_by_id > >>>>> monitor: add usb_attach and usb_detach (v2) > >>>>> > >>>>Acked-by: Gerd Hoffmann > >>>Okay, I am still confused about the use-case for this and I don't > >>>see any further explanation in the commit messages. I've seen > >>>"debugging" but can you be a bit more specific about which cases > >>>it's needed for? > >>To elaborate a little more, when using a certificates based card > >>there is no hardware event (i.e. removing/inserting the physical card) > >>that causes a usb_detach/attach to the card (both in passthru and > >>emulated), but otoh certificates is good for testing since it decouples > >>it from NSS/tcp. So I needed some way to emulate an insert/remove, and > >>I saw usb_del, which was pretty close, and voila. This is not the same > >>as card remove/reinsert, but it is exactly what will happen to the > >>guest when spicec connects/disconnects, since I detach devices on > >>disconnect and attach on connect. > >Looks reasonable to me, specially because this will be protected by > >#ifdef DEBUG. I don't see a big deal in merging this. > > I'd just like to see better documentation. A command isn't useful > for debugging if noone knows how to use it. > > Guarding with an #ifdef isn't necessary. It should be > unconditionally enabled otherwise it will bit rot. > > >Objections, Anthony? > > Not with better docs. > ok, so no QMP and better docs. Where should the docs go? just have more descriptive help messages? or in docs/usb_attach_detach.txt (for example)? > Regards, > > Anthony Liguori > > >>>This is just adding a HMP command. Is that the right approach or > >>>was that an unintentional consequence of rebasing post-HMP/QMP > >>>split? > >I don't think this should be available under QMP, it's more a debugging > >command for USB developers. > > > >>>Regards, > >>> > >>>Anthony Liguori > >>> > >>>>cheers, > >>>> Gerd > >>>> > >>>> > >>> >