On Thu, Jan 13, 2011 at 05:34:37PM -0700, Eric Blake wrote:
Still to go - add .args files to match .xml files in testsuite
* src/qemu/qemu_command.c (qemuBuildCommandLine): Emit smartcard options. (qemuAssignDeviceAliases): Assign an alias for smartcard passthrough. * tests/qemuxml2argvtest.c (mymain): Three new tests. * tests/qemuxml2argvdata/...arg: Three new files. ---
Well, as you can see, the tests/ part isn't done yet.
src/qemu/qemu_command.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++
What is the status of this support wrt upstream QEMU ? Has the smartcard stuff been accepted into GIT yet ? Also there is where the lack of security driver integration will cause problems, because QEMU won't start.
1 files changed, 49 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 205c303..d02241f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -604,6 +604,11 @@ qemuAssignDeviceAliases(virDomainDefPtr def, unsigned long long qemuCmdFlags) if (virAsprintf(&def->channels[i]->info.alias, "channel%d", i) < 0) goto no_memory; } + for (i = 0; i < def->nsmartcards ; i++) { + if (def->smartcards[i]->type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH && + virAsprintf(&def->smartcards[i]->info.alias, "smartcard%d", i) < 0) + goto no_memory; + }
You've assigned device aliases here....
if (def->console) { if (virAsprintf(&def->console->info.alias, "console%d", i) < 0) goto no_memory; @@ -3332,6 +3337,50 @@ qemuBuildCommandLine(virConnectPtr conn, } }
+ if (def->nsmartcards) { + /* Requires -chardev and -device usb-ccid */ + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) || + !(qemuCmdFlags & QEMUD_CMD_FLAG_USB_CCID)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("smartcard requires QEMU to support -usb-ccid")); + goto error; + } + virCommandAddArgList(cmd, "-device", "usb-ccid", NULL);
Ideally this would have an id= field too, eg if=ccid0
+ } + for (i = 0 ; i < def->nsmartcards ; i++) { + virDomainSmartcardDefPtr smartcard = def->smartcards[i]; + char *devstr; + virBuffer smartcard_buf = VIR_BUFFER_INITIALIZER; + int j; +
....But in this following block of code never added the aliases to the -device options:
+ switch (smartcard->type) { + case VIR_DOMAIN_SMARTCARD_TYPE_HOST: + virCommandAddArgList(cmd, "-device", "ccid-card-emulated", NULL); + break; + case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: + virCommandAddArg(cmd, "-device"); + virBufferAddLit(&smartcard_buf, + "ccid-card-emulated,backend=certificates"); + for (j = 0; j < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; j++) + virBufferVSprintf(&smartcard_buf, ",cert%d=%s", j + 1, + smartcard->data.cert.file[j]);
I guess we need some kind of escaping of special chars in the filename here since we append 3 in a row, it might confuse QEMU's parser. Not sure offhand how QEMU expects that to work. Or since we already depend on existance of -device, we might want to use '-set' for setting the filenames and thus avoid need to escape, eg -device ccid-card-emulated,backend=certificates,id=smartcard0 -set ccid-card-emulated.smartcard0.cert0=$FILENAME -set ccid-card-emulated.smartcard0.cert1=$FILENAME -set ccid-card-emulated.smartcard0.cert2=$FILENAME albeit at the cost of much longer command lines.
+ virCommandAddArgBuffer(cmd, &smartcard_buf); + break; + case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&smartcard->data.passthru, + smartcard->info.alias))) + goto error;
The 'id' setting for the chardev should really have a prefix on it, because we won't want it to be the same as the 'id' setting used for the -device option.
+ 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. 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. Regards, Daniel