Re: [libvirt] [RFC PATCHv2 5/5] WIP: smartcard: turn on qemu support

On Fri, Jan 14, 2011 at 11:25:36AM -0700, Eric Blake wrote: > On 01/14/2011 05:41 AM, Daniel P. Berrange wrote: > > On Thu, Jan 13, 2011 at 05:34:37PM -0700, Eric Blake wrote: > > > >> + virCommandAddArg(cmd, devstr); > >> + VIR_FREE(devstr); > >> + > >> + virCommandAddArg(cmd, "-device"); > >> + virCommandAddArgFormat(cmd, "ccid-card-passthru,chardev=%s", > >> + smartcard->info.alias); > >> + break; > >> + } > >> + } > >> + > >> if (!def->nserials) { > >> /* If we have -device, then we set -nodefault already */ > >> if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) > > > > We currently only provide a single '-device usb-ccid' device. It looks > > like we're relying on the ccid-card-XXX devices automagically associating > > themselves with that. It would be better to explicitly link them up > > by specifying the 'id' of the 'usb-ccid' device to which they must be > > associated. If QEMU doesn't have such an explicit link, it ought to > > be added. > > That's how Alon implemented it. -device usb-ccid is instantiated > exactly once, and is not tied to any -chardev. -device > ccid-card-emulated automatically uses the services of -device usb-ccid, > and does not need any -chardev. And -device ccid-card-passthru > automatically uses the services of -device usb-ccid, and requires an > associated -chardev (Alon has only tested with a tcp chardev and with a > spicevmc,name=smartcard chardev). At this point, your complaints seem > to be more about whether Alon's qemu command line implementation makes > sense (which, given that the smartcard patches have been proposed but > are not yet upstream, may mean that the final qemu implementation will > change and my libvirt patch would have to equally change to match), > rather than whether I'm correctly mapping to the command line that Alon > documented: > > http://cgit.freedesktop.org/~alon/qemu/commit/?h=usb_ccid.v15&id=024a... Alon's docs are showing the simplified syntax suitable for end users. This doesn't guarentee a stable guest visible ABI. Looking at the code, we need to set the 'slot' parameter on each ccid device we have. This means we need a new address type for smart card devices, and a corresponding <controller> instance. So in the XML we'd get (including libvirt generated aliases and addresses): <devices> <controller type='ccid' index='0'> <alias id='ccid0'/> </controller> <controller type='ccid' index='1'> <alias id='ccid1'/> </controller> <smartcard mode='host'> <alias id='smartcard0'/> <address type='ccid' controller='0' slot='0'/> </smartcard> <smartcard mode='host-certificates'> <certificate>/path/to/cert1</certificate> <certificate>/path/to/cert2</certificate> <certificate>/path/to/cert3</certificate> <alias id='smartcard1'/> <address type='ccid' controller='0' slot='3'/> </smartcard> <smartcard mode='passthrough' type='tcp'> <source mode='connect' host='127.0.0.1' service='2001'/> <protocol type='raw'/> <alias id='smartcard2'/> <address type='ccid' controller='1' slot='0'/> </smartcard> </devices> Which would end up mapping to -device usb-ccid,id=ccid0 -device usb-ccid,id=ccid1 -device ccid-host,id=smartcard0,bus=ccid0,slot=0 -device ccid-certificates,id=smartcard1,bus=ccid0,slot=3... -device ccid-passthrough,id=smartcard2,bus=ccid1,slot=0... In other words a hierarchy USB bus 0 | +- ccid0 | | | +- smartcard0 (ccid slot 0) | +- smartcard1 (ccid slot 3) | +- ccid1 | +- smartcard2 (ccid slot 0) This is a pretty similar setup to the way we link virtio-serial devices and vmchannels > > Also, does the order of '-device ccid-card-XXX' devices matter ie is > > the ordering a guest visible ABI ? If so, then we need to track an > > explicit address against each <smartcard> so that we can selectively > > hotplug/hotunplug devices, and have a stable ABI across migration. > > That I'm not sure about. Alon said that the guest sees the smartcard as > a USB device, but didn't mention anything about whether -device > ccid-card-* can take additional USB addressing parameters to lock down > where the device appears on the guest's USB bus. The 'usb-ccid' is on the USB bus, but we don't have stable addressing for that, because the guest OS controls USB addresses. We do need the stable slots for the <smartcards> though.

Daniel