Re: [libvirt] [PATCH v9 4/4] qemu: Add TLS support for Veritas HyperScale (VxHS)

On Tue, Sep 19, 2017 at 21:32:46 -0400, John Ferlan wrote: > From: Ashish Mittal <Ashish.Mittal(a)veritas.com&gt; > > 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(a)veritas.com&gt; > Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; > --- > 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

> 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

> + * 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.

> + 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.