Re: [Qemu-devel] [PATCH 0/2] usb-ccid device (v2)

[Anthony Liguori <anthony@codemonkey.ws>]

On 10/12/2010 11:03 AM, Alon Levy wrote:
> ----- "Anthony Liguori"<anthony@codemonkey.ws>  wrote:
>
>    
>> On 10/12/2010 07:58 AM, Alon Levy wrote:
>>      
>>> This patch adds a new device, it is described in full in the second
>>>        
>> patch
>>      
>>> intro and also in the documentation in docs. In brief it provides a
>>>        
>> standard
>>      
>>> smart card reader device.
>>>
>>> The first patch is the configure change and docs.
>>> The second patch contains the actual device, I couldn't figure out a
>>>        
>> good
>>      
>>> way to split it to ease review.
>>>
>>> v2 changed:
>>>    * all QSIMPLEQ turned into fixed sized rings
>>>    * all allocated buffers turned into fixed size buffers
>>>    * added migration support
>>>    * added a message to tell client qemu has migrated to ip:port
>>>     * for lack of monitor commands ip:port are 0:0, which causes the
>>>        
>> updated
>>      
>>>      vscclient to connect to one port higher on the same host. will
>>>        
>> add monitor
>>      
>>>      commands in a separate patch. tested with current setup.
>>>
>>>        
>> This is way too much magic to live within a device.  Devices manage
>> reconnecting themselves during migration.  When you create the
>> destination qemu instance, you specify what to connect to.
>>
>> IOW,
>>
>> On the source:
>>
>> qemu -chardev tcp:localhost:1025,id=foo -usbdevice ccid,chardev=foo
>> ...
>>
>> On the destination:
>>
>> qemu -chardev tcp:localhost:1026,id=foo -usbdevice ccid,chardev=foo
>> -incoming tcp:0.0.0.0:1024 ...
>>
>> A connection happens when the device is created.
>>
>> But now I'm even further confused then when I first reviewed it..  If
>>
>> you're now supporting migration, does that mean that you're relying on
>>
>> the daemon to emulate the device?
>>
>>      
> Let me try to clarify this. Nothing has changed since the last patch except
> for what's in the notes, i.e. migration support. The device I'm adding is a
> reader. The reader is just a pipe between smart cards and the guest operating
> system. The smart card logic does live outside of this device, and is available
> in the cac_card sources at http://cgit.freedesktop.org/~alon/cac_card/ (all of
> this is in docs/usb_ccid.txt).
>
> So when I speak of vscclient, I'm talking of an application that emulates a smart
> card and initiates a tcp connection to qemu that connects to the usb-ccid device.
> vscclient is also in the cac_card sources.
>    

Okay, let me be clear.  We shouldn't be doing device emulation outside 
of QEMU's source tree--at least, not in this type of context.  External 
devices present a great deal of challenges and we shouldn't just 
approach it in an ad-hoc fashion.

I'm not opposed to passthrough although I'd prefer QEMU to talk to the 
device directly instead of going through a daemon.

There's a lot of delicate integration between QEMU and a device and if a 
device lives outside of QEMU, it makes it extremely difficult for us to 
influence changes to that device to support QEMU new features.

> Regarding the method of reconnection: You are absolutely right that if I have qemu
> connect to the remote instead of the other way around then I remove the need to inform
> vscclient of the new address. But the way it stands requires the client to know the
> address of the destination qemu. I have to inform it somehow. You are saying that
> devices shouldn't know this information? ok, that's why I talked about monitor commands.
> I come from the world of spice - in spice we use monitor commands for this.

And none of that is upstream.

Regards,

Anthony Liguori

>   I could
> change this to have qemu connect to vscclient, but I don't see the logic in general -
> sometimes you do want to have a chardev that is listening (the fact that it is implemented
> suggests someone found it useful), if you then migrate you have the same problem I'm
> solving.
>
>    
>> Regards,
>>
>> Anthony Liguori
>>
>>      
>>> Alon Levy (2):
>>>     usb-ccid: add CCID device. add configure option.
>>>     usb-ccid: add CCID device (device itself)
>>>
>>>    Makefile.objs      |    1 +
>>>    configure          |   12 +
>>>    docs/usb-ccid.txt  |  115 +++++
>>>    hw/usb-ccid.c      | 1376
>>>        
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>      
>>>    hw/vscard_common.h |  131 +++++
>>>    5 files changed, 1635 insertions(+), 0 deletions(-)
>>>    create mode 100644 docs/usb-ccid.txt
>>>    create mode 100644 hw/usb-ccid.c
>>>    create mode 100644 hw/vscard_common.h
>>>
>>>
>>>