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