On 09/27/2017 10:25 AM, Peter Krempa wrote:
> On Tue, Sep 19, 2017 at 21:32:46 -0400, John Ferlan wrote:
>> From: Ashish Mittal <Ashish.Mittal@xxxxxxxxxxx>
>>
>> Alter qemu command line generation in order to possibly add TLS for
>> a suitably configured domain.
>>
>> Sample TLS args generated by libvirt -
>>
>> -object tls-creds-x509,id=objvirtio-disk0_tls0,dir=/etc/pki/qemu,\
>> endpoint=client,verify-peer=yes \
>> -drive file.driver=vxhs,file.tls-creds=objvirtio-disk0_tls0,\
>> file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
>> file.server.type=tcp,file.server.host=192.168.0.1,\
>> file.server.port=9999,format=raw,if=none,\
>> id=drive-virtio-disk0,cache=none \
>> -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\
>> id=virtio-disk0
>>
>> Update the qemuxml2argvtest with a couple of examples. One for a
>> simple case and the other a bit more complex where multiple VxHS disks
>> are added where at least one uses a VxHS that doesn't require TLS
>> credentials and thus sets the domain disk source attribute "tls = 'no'".
>>
>> Update the hotplug to be able to handle processing the tlsAlias whether
>> it's to add the TLS object when hotplugging a disk or to remove the TLS
>> object when hot unplugging a disk. The hot plug/unplug code is largely
>> generic, but the addition code does make the VXHS specific checks only
>> because it needs to grab the correct config directory and generate the
>> object as the command line would do.
>>
>> Signed-off-by: Ashish Mittal <Ashish.Mittal@xxxxxxxxxxx>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/qemu/qemu_block.c | 8 +++
>> src/qemu/qemu_command.c | 33 +++++++++
>> src/qemu/qemu_hotplug.c | 79 ++++++++++++++++++++++
>> ...-disk-drive-network-tlsx509-multidisk-vxhs.args | 43 ++++++++++++
>> ...v-disk-drive-network-tlsx509-multidisk-vxhs.xml | 50 ++++++++++++++
>> ...muxml2argv-disk-drive-network-tlsx509-vxhs.args | 30 ++++++++
>> tests/qemuxml2argvtest.c | 7 ++
>> 7 files changed, 250 insertions(+)
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
>>
>> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
>> index 3437302dd..77ffc6c51 100644
>> --- a/src/qemu/qemu_block.c
>> +++ b/src/qemu/qemu_block.c
>> @@ -529,16 +529,24 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src)
>> return NULL;
>> }
>>
>> + if (src->haveTLS == VIR_TRISTATE_BOOL_YES && !src->tlsAlias) {
>> + virReportError(VIR_ERR_INVALID_ARG, "%s",
>> + _("VxHS disk does not have TLS alias set"));
>> + return NULL;
>> + }
>
> This is not the right place for validation. Also it's really only a
> coding error since callers should make sure to properly configure the
> object.
>
>> +
>> if (!(serv
OK dropped
er = qemuBlockStorageSourceBuildJSONSocketAddress(src->hosts, true)))
>> return NULL;
>>
>
> [...]
>
>
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index 7dd6e5fd9..7751a608d 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -156,6 +156,52 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
>>
>>
>> static int
>> +qemuDomainAddDiskSrcTLSObject(virQEMUDriverPtr driver,
>> + virDomainObjPtr vm,
>> + virStorageSourcePtr src,
>> + const char *srcalias)
>> +{
>> + int ret = -1;
>> + qemuDomainObjPrivatePtr priv = vm->privateData;
>> + virJSONValuePtr tlsProps = NULL;
>> +
>> + /* NB: Initial implementation doesn't require/use a secret to decrypt
>> + * a server certificate, so there's no need to manage a tlsSecAlias
>
> client certificate
>
No it's the server certificate (server-key.pem) that needs the secret in
order to be decrypted.
>> + * and tlsSecProps. See qemuDomainAddChardevTLSObjects for the
>> + * methodology required to add a secret object. */
>> +
>> + /* Create the TLS object using the source tls* settings */
>> + if (qemuDomainGetTLSObjects(priv->qemuCaps, NULL,
>> + src->tlsCertdir,
>> + src->tlsListen,
>> + src->tlsVerify,
>> + srcalias, &tlsProps, &src->tlsAlias,
>> + NULL, NULL) < 0)
>> + goto cleanup;
>> +
>> + if (qemuDomainAddTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE,
>> + NULL, NULL, src->tlsAlias, &tlsProps) < 0)
>> + goto cleanup;
>> +
>> + ret = 0;
>> +
>> + cleanup:
>> + virJSONValueFree(tlsProps);
>> +
>> + return ret;
>> +}
>
>
> [...]
>
>> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>> index bf43beb10..21f057460 100644
>> --- a/tests/qemuxml2argvtest.c
>> +++ b/tests/qemuxml2argvtest.c
>> @@ -934,6 +934,13 @@ mymain(void)
>> DO_TEST("disk-drive-network-rbd-ipv6", NONE);
>> DO_TEST_FAILURE("disk-drive-network-rbd-no-colon", NONE);
>> DO_TEST("disk-drive-network-vxhs", QEMU_CAPS_VXHS);
>> + driver.config->vxhsTLS = 1;
>> + DO_TEST("disk-drive-network-tlsx509-vxhs", QEMU_CAPS_VXHS,
>> + QEMU_CAPS_OBJECT_TLS_CREDS_X509);
>> + DO_TEST("disk-drive-network-tlsx509-multidisk-vxhs", QEMU_CAPS_VXHS,
>> + QEMU_CAPS_OBJECT_TLS_CREDS_X509);
>
> The second test case is a superset of the first, so the first one can be
> dropped.
>
OK I'll remove the former
John
>> + driver.config->vxhsTLS = 0;
>> + VIR_FREE(driver.config->vxhsTLSx509certdir);
>> DO_TEST("disk-drive-no-boot",
>> QEMU_CAPS_BOOTINDEX);
>> DO_TEST_PARSE_ERROR("disk-device-lun-type-invalid",
>
> ACK if you drop the validation and fix the certificate comment.
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
