[libvirt] adding smartcard support to libvirt

[Alon Levy]

On Wed, Jan 05, 2011 at 02:55:55PM -0700, Eric Blake wrote:
> On 01/05/2011 02:09 PM, Alon Levy wrote:
> >> So, I'm thinking that this XML representation matches the spicevmc chardev:
> >>
> >> <devices>
> >>   <channel type='spicevmc'/>
> >>     <source port='5903' tlsPort='5904' autoport='no'
> >> listen='127.0.0.1'/>
> > 
> > I got you until now - but what's with the port/tlsPort - all of that stuff belongs
> > to the spice flag, and I'm pretty sure is already taken care of by some other
> > tag (I guess <spice> probably?).
> 
> Hmm, right now, the only use of spice in XML is under <graphics
> type='spice'>, and it is the <graphics> element that contains port,
> tlsPort, autoport, and listen arguments.  So we may need to revisit
> that, and have some way to use a single location for spice parameters
> shared among all spice-related devices, and a way for both <graphics
> type='spice'> and <smartcard type='passthrough'> to reference that
> rather than repeating it.  Meaning that spicevmc support just got more
> difficult, if it involves tweaking <graphics> rather than just adding a
> new <channel type='spicevmc'> element.

Are you saying that for correctness you want to tie the two elements together?
I mean, that's the only possible reason I see. If you want to tie them
together to prevent an instance of spicevmc without an instance of graphics
of type spice, I understand. I guess (after seeing the patch you sent) there
is a verifier to the xml inputs to libvirt that would be able to deduce invalidity
in that case?
Of course the alternative is to have logic elsewhere, but I see the point in
trying to verify the xml input only at one place.

> 
> > This chardev is totally separate (sure, you need
> > to be using spice for it to make sense, but there is not overlap in parameters, for
> > instance you don't give a port nor a tlsPort to the spicevmc chardev).
> 
> Okay, looking more at existing devices, I agree that <channel
> type='spicevmc'> should only need additional attributes/sub-elements
> corresponding to parameters appearing directly in the -chardev/-device
> argument pair handed to qemu.  So I'm not sure if <source> makes sense
> any more, or if <source> would be what ends up pointing to common
> details about the spice server as shared with the <graphics> element.
> 
> > 
> >>     <target type='smartcard'/>
> > This looks right.
> > 
> >>   </channel>
> >> </devices>
> >>
> >> In looking more at libvirt XML, I don't see any fields that match id
> >> assignments; rather, libvirt auto-assigns unique ids in the form %s%d,
> >> category, count, where category matches <channel> and count matches how
> >> many <channel> devices are present.  That is, the above xml would map to:
> >>
> >> qemu -chardev spicevmc,id=smartcard,name=channel1
> >>
> > I hope you meant id=channel1,name=smartcard ? the id needs to be the same
> > as the ccid-passthru uses.
> 
> Arrgh - yes, more fallout from me swapping id= and name=, and copying
> and pasting from the wrong email.  You already made it clear that id= is
> the unique name referenced in some other -device chardev=id location,
> and that for spicevmc, name= is one of two fixed values (vdagent or
> smartcard).
> 
> > But I guess we determined that it's easiest to
> > let the <smartcard mode="passthru"/> already add the spicevmc chardev
> > itself? the usage of "-chardev spicevmc,id=xyz,type=smartcard" is only
> > for a "-device ccid-card-passthru,chardev=xyz", so one won't appear without
> > the other.
> 
> Looking even more at existing examples (can you tell that I'm trying to
> learn more about existing <devices> at the same time as implementing a
> new one?), I see this good example from
> qemuxml2argvdata/qemuxml2argv-serial-tcp-chardev:
> 
>     <serial type='tcp'>
>       <source mode='connect' host='127.0.0.1' service='9999'/>
>       <protocol type='raw'/>
>       <target port='0'/>
>     </serial>
> 
> maps to:
>  -chardev socket,id=serial0,host=127.0.0.1,port=9999 -device
> isa-serial,chardev=serial0
> 
> It looks like -chardev was added in qemu 0.12, and that earlier versions
> used just -device without having to map things back to a chardev; but
> since smartcard is post-0.12, I only need to worry about the -chardev
> side of things.
> 
> >> Hmm; getting spicevmc to work seems independent enough of getting
> >> <smartcard> implemented by using existing <channel type='tcp'/>, so I'll
> >> try to split my libvirt XML improvements into two batches.  Should I
> >> focus on <channel type='spicevmc'> or on <smartcard> first?
> > 
> > I guess start with the smartcard first? Implement it without dealing
> > with the spicevmc side - i.e. don't implement the passthru type first,
> > or propose it but don't implement it. Then do the spicevmc part.
> 
> Yep, that's pretty much what I'm settling on right now - figure out how
> to map <smartcard> to existing 'tcp' mapping first, then figure out
> about adding spicevmc support including expanding <smartcard> to handle
> spicevmc.
> 
> > I'm not particular on the order - both are required for RHEL 6.1 anyhow,
> > and each is testable without the other (spicevmc with vdagent usage outlined
> > above, and smartcard without spice by using it locally through libvirt).
> 
> So, back to thinking about the new <smartcard> element.  The <smartcard
> mode='host'> and <smartcard mode='host-certificates'> modes are simple,
> since -device ccid-card-emulated has no associated -chardev argument.
> But if we go with the ideas that <smartcard mode='passthrough'> implies
> -device ccid-card-passthru and an associated -chardev, and that the only
> -chardev's worth pairing with a <smartcard> are tcp and spicevmc, then
> it's simpler to just add a type attribute, and make <smartcard
> mode='passthrough' type='tcp'> look a lot like <serial type='tcp'> (no
> <serial> sub-element, but instead have a <source> sub-element directly
> resembling the <source> sub-element of serial).  Meanwhile, <serial>
> allows a <target> sub-element (which virtual serial port the guest will
> access as the chardev), whereas <smartcard> would not (the guest sees
> the smartcard via the emulated card from the usb-ccid bus, and not as a
> raw serial port).  Thus,
> 
> <devices>
>   <smartcard mode='passthrough' type='tcp'>
>     <source host='0.0.0.0' port='2001'/>
>     <protocol type='raw'/>
>   </smartcard>
> </devices>
> 
> maps to:
> -usb -device usb-ccid -chardev
> socket,server,host=0.0.0.0,port=2001,id=smartcard1,nowait -device
> ccid-card-passthru,chardev=smartcard1
> 
> and I think it leaves the door open for:
> 
> <devices>
>   <channel type='spicevmc'>
>     <target type='vdagent'/>
>   </channel>
>   <smartcard mode='passthrough' type='spicevmc'>
>     <target type='smartcard'/>
>   </smartcard>
> </devices>
> 
> to map to:
> -chardev spicevmc,id=channel1,name=vdagent -device vdagent,... -usb
> -device usb-ccid -chardev spicevmc,id=smartcard1,name=smartcard -device
> ccid-card-passthru,chardev=smartcard1
> 
> Hopefully this will make more sense once I actually finish coding up the
> RelaxNG schema validation, and propose a first round patch that
> documents the proposed XML with examples.
> 
> -- 
> Eric Blake   eblake at redhat.com    +1-801-349-2682
> Libvirt virtualization library http://libvirt.org
>