[libvirt] [PATCH v2 3/4] qemu: Implement support for 'builtin' backend for virtio-crypto
Martin Kletzander
mkletzan at redhat.com
Tue Feb 7 12:36:47 UTC 2017
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 at huawei.com>
>---
> 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 at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170207/3feda03e/attachment-0001.sig>
More information about the libvir-list
mailing list