On Wed, Jan 11, 2017 at 04:28:25PM +0800, Longpeng(Mike) wrote:
This patch implements support for the virtio-crypto-pci device and the builtin backend in qemu. Two capabilities bits are added to track support for those: QEMU_CAPS_DEVICE_VIRTIO_CRYPTO - for the device support and QEMU_CAPS_OBJECT_CRYPTO_BUILTIN - for the backend support. qemu is invoked with these additional parameters if the device id enabled: (to add the backend) -object cryptodev-backend-builtin,id=objcrypto0,queues=1 (to add the device) -device virtio-crypto-pci,cryptodev=objcrypto0,id=crypto0 Signed-off-by: Longpeng(Mike) <longpeng2@xxxxxxxxxx> --- src/conf/domain_conf.c | 4 +- src/qemu/qemu_alias.c | 20 +++++++ src/qemu/qemu_alias.h | 3 + src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 132 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 3 + src/qemu/qemu_domain_address.c | 26 +++++++- 8 files changed, 191 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ef44930..cf77af5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12595,7 +12595,7 @@ virDomainCryptoDefParseXML(xmlNodePtr node, if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; -cleanup: + cleanup: VIR_FREE(model); VIR_FREE(backend); VIR_FREE(queues); @@ -12603,7 +12603,7 @@ cleanup: ctxt->node = save; return def; -error: + error:
Make sure you run make check and make syntax check after *every* patch. Otherwise we end up with hunks like this that clearly don't belong here.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d459f8e..afebe69 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5787,6 +5787,135 @@ qemuBuildRNGCommandLine(virLogManagerPtr logManager, static char * +qemuBuildCryptoBackendStr(virDomainCryptoDefPtr crypto, + virQEMUCapsPtr qemuCaps) +{ + const char *type = NULL; + char *alias = NULL; + char *queue = NULL; + char *ret = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (virAsprintf(&alias, "obj%s", crypto->info.alias) < 0) + goto cleanup; + + if (crypto->queues > 0) { + if (virAsprintf(&queue, "queues=%u", crypto->queues) < 0) + goto cleanup; + } + + switch ((virDomainCryptoBackend)crypto->backend) { + case VIR_DOMAIN_CRYPTO_BACKEND_BUILTIN: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_CRYPTO_BUILTIN)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this qemu doesn't support the builtin backend")); + goto cleanup; + } + + type = "cryptodev-backend-builtin"; + break; + + case VIR_DOMAIN_CRYPTO_BACKEND_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unknown crypto backend")); + goto cleanup; + } + + if (queue) + virBufferAsprintf(&buf, "%s,id=%s,%s", type, alias, queue); + else + virBufferAsprintf(&buf, "%s,id=%s", type, alias); + + ret = virBufferContentAndReset(&buf);
The advantage of using buffer is that you can act on it multiple times. If you just want one asprintf, you can just do virAsprintf instead. But in this case I would just append every parameter as you go.
+ + cleanup: + VIR_FREE(alias); + return ret; +} + + +char * +qemuBuildCryptoDevStr(const virDomainDef *def, + virDomainCryptoDefPtr dev, + virQEMUCapsPtr qemuCaps) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (dev->model != VIR_DOMAIN_CRYPTO_MODEL_VIRTIO || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_CRYPTO)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("this qemu doesn't support crypto device model '%s'"), + virDomainRNGModelTypeToString(dev->model)); + goto error; + } + + if (dev->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported address type %s for virtio crypto device"), + virDomainDeviceAddressTypeToString(dev->info.type)); + goto error; + } + + virBufferAsprintf(&buf, "virtio-crypto-pci,cryptodev=obj%s,id=%s", + dev->info.alias, dev->info.alias); + + if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0) + goto error; + + if (virBufferCheckError(&buf) < 0) + goto error; +
This ^^ is pointless since this vv will return NULL and not leak either.
+ return virBufferContentAndReset(&buf); + + error: + virBufferFreeAndReset(&buf); + return NULL; +} + + +static int +qemuBuildCryptoCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + for (i = 0; i < def->ncryptos; i++) { + virDomainCryptoDefPtr crypto = def->cryptos[i]; + char *tmp; + + if (qemuAssignDeviceCryptoAlias(def, crypto)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("crypto device assign alias faile")); + return -1; + } + + if (!crypto->info.alias) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("crypto device is missing alias")); + return -1; + } + + /* add the Crypto backend */
I see you keep calling it Crypto (with uppercase 'C'). It's OK here, but shouldn't be in the middle of the sentence in a documentation, for example.
+ if (!(tmp = qemuBuildCryptoBackendStr(crypto, qemuCaps))) + return -1; + + virCommandAddArgList(cmd, "-object", tmp, NULL); + VIR_FREE(tmp); + + /* add the device */ + if (!(tmp = qemuBuildCryptoDevStr(def, crypto, qemuCaps))) + return -1; + + virCommandAddArgList(cmd, "-device", tmp, NULL); + VIR_FREE(tmp); + } + + return 0; +} + + +static char * qemuBuildSmbiosBiosStr(virSysinfoBIOSDefPtr def) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -9793,6 +9922,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildRNGCommandLine(logManager, cmd, cfg, def, qemuCaps) < 0) goto error; + if (qemuBuildCryptoCommandLine(cmd, def, qemuCaps) < 0) + goto error; + if (qemuBuildNVRAMCommandLine(cmd, def, qemuCaps) < 0) goto error; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 3bcfdc6..9b49ccd 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -201,6 +201,9 @@ char *qemuBuildShmemDevStr(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +char *qemuBuildCryptoDevStr(const virDomainDef *def, + virDomainCryptoDefPtr dev, + virQEMUCapsPtr qemuCaps); #endif /* __QEMU_COMMAND_H__*/ diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index e17476a..62bbd1c 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -331,6 +331,12 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, def->rngs[i]->info.type = type; } + for (i = 0; i < def->ncryptos; i++) { + if (def->cryptos[i]->model == VIR_DOMAIN_CRYPTO_MODEL_VIRTIO && + def->cryptos[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + def->cryptos[i]->info.type = type; + } + if (type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { for (i = 0; i < def->nfss; i++) { if (def->fss[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) @@ -727,6 +733,15 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, return 0; } + case VIR_DOMAIN_DEVICE_CRYPTO: + switch ((virDomainCryptoModel) dev->data.crypto->model) { + case VIR_DOMAIN_CRYPTO_MODEL_VIRTIO: + return virtioFlags; + + case VIR_DOMAIN_RNG_MODEL_LAST: + return 0; + } + case VIR_DOMAIN_DEVICE_VIDEO: switch ((virDomainVideoType) dev->data.video->type) { case VIR_DOMAIN_VIDEO_TYPE_VIRTIO: @@ -784,7 +799,6 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_GRAPHICS: case VIR_DOMAIN_DEVICE_IOMMU: - case VIR_DOMAIN_DEVICE_CRYPTO: case VIR_DOMAIN_DEVICE_LAST: case VIR_DOMAIN_DEVICE_NONE: return 0;
Oh, here it is, why didn't you put it here right away? Oh, because now this patch implements this device. Well, that's a way to do it, OK.
@@ -1770,6 +1784,16 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def, goto error; } + /* VirtIO CRYPTO */ + for (i = 0; i < def->ncryptos; i++) { + if (def->cryptos[i]->model != VIR_DOMAIN_CRYPTO_MODEL_VIRTIO || + !virDeviceInfoPCIAddressWanted(&def->cryptos[i]->info)) + continue; + + if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->cryptos[i]->info) < 0) + goto error; + } + /* A watchdog - check if it is a PCI device */ if (def->watchdog && def->watchdog->model == VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB && -- 1.8.3.1 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list