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@xxxxxxxxxx +1-801-349-2682 > Libvirt virtualization library http://libvirt.org > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list