On Fri, Jan 14, 2011 at 07:23:17PM +0000, Daniel P. Berrange wrote:
> 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=024a37bf696ae3a8d148a2ecbecdd3d92d390b2a
>
> 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.
Ok, that saves me some code searching - was trying to figure out how to
specify addresses from command line for usb devices :)
Regarding usb-ccid bus:
1) the id is currently done by the qdev code when supplied a NULL bus parameter,
i.e. it takes the device id and appends ".0", so you get for instance:
-device usb-ccid,id=ccid0 -device usb-ccid,id=ccid1
-device ccid-card-emulated,bus=ccid0.0
-device ccid-card-emulated,bus=ccid1.0
Becomes:
bus: ccid1.0
type ccid-bus
dev: ccid-card-emulated, id ""
dev-prop: backend = "nss-emulated"
dev-prop: cert1 = <null>
dev-prop: cert2 = <null>
dev-prop: cert3 = <null>
dev-prop: db = <null>
dev-prop: debug = 0
bus-prop: slot = 0
dev: usb-ccid, id "ccid0"
dev-prop: debug = 0
addr 0.1, speed 12, name QEMU USB CCID, attached
bus: ccid0.0
type ccid-bus
dev: ccid-card-emulated, id ""
dev-prop: backend = "nss-emulated"
dev-prop: cert1 = <null>
dev-prop: cert2 = <null>
dev-prop: cert3 = <null>
dev-prop: db = <null>
dev-prop: debug = 0
bus-prop: slot = 0
Tell me if this needs to be changed - for instance I could just
reuse the id as the bus name, so it will lose the ".0" suffix.
2) usb-ccid device doesn't support more then one slot atm. This
may be changed later, but atm to create two slots you need two
busses. Also, this is a bug, two emulated slots will probably
fail (haven't tested), since I'm pretty sure I'm doing all
the initialization twice in that case, and even if not, my
event quering loop doesn't have a concept of a target, so
some events will end up at the first, some at the second - chaos.
The good news is that this is totally opaque to libvirt, just
an implementation bug.
>
> Daniel
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list