[libvirt] [PATCH] conf: Parse virtio-crypto in the domain XML

[Martin Kletzander]

On Mon, Jan 09, 2017 at 10:15:25AM +0800, Longpeng(Mike) wrote:
>As virtio-crypto has been supported in QEMU 2.9 and
>the frontend driver has been merged in linux 4.10,
>so it's necessary to support virtio-crypto in domain
>XML.
>
>This patch parse the domain XML with virtio-crypto
>support, the virtio-crypto XML looks like this:
>
>    <crypto model='virtio'>
>        <backend type='builtin' queues='2'/>
>    </crypto>
>
>Signed-off-by: Longpeng(Mike) <longpeng2 at huawei.com>
>---
> src/conf/domain_conf.c         | 212 +++++++++++++++++++++++++++++++++++++++++
> src/conf/domain_conf.h         |  32 +++++++
> src/qemu/qemu_alias.c          |  20 ++++
> src/qemu/qemu_alias.h          |   4 +
> src/qemu/qemu_capabilities.c   |   4 +
> src/qemu/qemu_capabilities.h   |   2 +
> src/qemu/qemu_command.c        | 129 +++++++++++++++++++++++++
> src/qemu/qemu_command.h        |   4 +
> src/qemu/qemu_domain_address.c |  17 ++++
> src/qemu/qemu_driver.c         |   6 ++
> src/qemu/qemu_hotplug.c        |   1 +
> 11 files changed, 431 insertions(+)
>

No tests, no documentation, no schema => no acks.

The patch should be split into at least two parts (conf, schema,
qemuxml2xmltest and docs in one; then hypervisor functionality and tests
in the second one).

Also I see no hunk adding anything to qemuDomainAssignDevicePCISlots()
or similar.

>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 9f7b906..fcfccd5 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -237,6 +237,7 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
>               "memballoon",
>               "nvram",
>               "rng",
>+              "crypto",

Why are you adding this in a random place rather than at the end?  That
could be fixed in most of the hunks.

>@@ -21590,6 +21749,51 @@ virDomainRNGDefFree(virDomainRNGDefPtr def)
>
>
> static int
>+virDomainCryptoDefFormat(virBufferPtr buf,
>+                         virDomainCryptoDefPtr def,
>+                         unsigned int flags)
>+{
>+    const char *model = virDomainCryptoModelTypeToString(def->model);
>+    const char *backend = virDomainCryptoBackendTypeToString(def->backend);
>+
>+    virBufferAsprintf(buf, "<crypto model='%s'>\n", model);
>+    virBufferAdjustIndent(buf, 2);
>+    virBufferAsprintf(buf, "<backend type='%s'", backend);
>+
>+    switch ((virDomainCryptoBackend) def->backend) {
>+    case VIR_DOMAIN_CRYPTO_BACKEND_BUILTIN:
>+        if (def->queues)
>+            virBufferAsprintf(buf, " queues='%u'", def->queues);
>+
>+        virBufferAddLit(buf, "/>\n");
>+        break;
>+
>+    case VIR_DOMAIN_CRYPTO_BACKEND_LAST:
>+        break;
>+    }
>+
>+    if (virDomainDeviceInfoNeedsFormat(&def->info, flags)) {

It should always have an address, this is not an old device with
back-compat problems.

>diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>index 2c0b29d..b1b718b 100644
>--- a/src/qemu/qemu_capabilities.c
>+++ b/src/qemu/qemu_capabilities.c
>@@ -337,6 +337,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>               "drive-detect-zeroes",
>
>               "tls-creds-x509", /* 230 */
>+              "cryptodev-backend-builtin",
>+              "virtio-crypto",
>     );
>
>
>diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
>index affb639..98cb817 100644
>--- a/src/qemu/qemu_capabilities.h
>+++ b/src/qemu/qemu_capabilities.h
>@@ -370,6 +370,8 @@ typedef enum {
>
>     /* 230 */
>     QEMU_CAPS_OBJECT_TLS_CREDS_X509, /* -object tls-creds-x509 */
>+    QEMU_CAPS_OBJECT_CRYPTO_BUILTIN,
>+    QEMU_CAPS_DEVICE_VIRTIO_CRYPTO,
>

Base your patch on master, not on some old, private, branch.  Also see
how every capability has a nice comment even if it's very trivial.  It
can help sometimes.

I haven't tested it or read it thoroughly, but the rest looks
reasonable.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170109/717e8bd7/attachment-0001.sig>