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

[Anthony Liguori <anthony@codemonkey.ws>]

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.

> 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
>>>>
>>>>
>>>>          
>>>
>>>        
>>      
>