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@veritas.com>
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@veritas.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- 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.