Hi Martin,
On 2017/1/9 16:44, Martin Kletzander wrote:
> 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@xxxxxxxxxx>
>> ---
>> 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).
>
OK, I will split the patch and add some testcases in V2.
> Also I see no hunk adding anything to qemuDomainAssignDevicePCISlots()
> or similar.
>
I added in fact :)
@@ -1236,6 +1242,17 @@ 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 (virDomainPCIAddressReserveNextSlot(addrs,
+ &def->cryptos[i]->info, flags) < 0)
+ goto error;
+ }
+
>> 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.
>
OK, I will fix it in V2.
>> @@ -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.
>
All right, thanks
>> 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 got it, thanks :)
> I haven't tested it or read it thoroughly, but the rest looks
> reasonable.
>
I had tested this patch and 'make syntax-check' before sent it. I will rework
this patch according to your suggestion and send V2 in these days :)
> Martin
--
Regards,
Longpeng(Mike)
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
