Re: [Qemu-devel] [PATCH 0/3] add usb_detach and usb_attach (v3)

[Luiz Capitulino <lcapitulino@redhat.com>]

On Fri, 22 Oct 2010 07:55:02 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 10/22/2010 07:48 AM, Luiz Capitulino wrote:
> > On Thu, 21 Oct 2010 15:27:23 +0200
> > Alon Levy<alevy@redhat.com>  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<kraxel@redhat.com>
> >>>>          
> >>> 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.

You and Gerd asked about the purpose of this command, turns out that it's
only useful for developing new USB devices for QEMU, so I thought it would
be better to restrict it, so that people don't start using this the
wrong way or worse, we can't drop/break it because some tool is now using it.

Anyway, maybe a good doc will do, this was just a small suggestion.

> 
> > Objections, Anthony?
> >    
> 
> Not with better docs.
> 
> 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
> >>>>
> >>>>
> >>>>          
> >>>
> >>>        
> >>      
> >    
>