Re: [PATCH v8 11/11] qemu: Add TLS support for Veritas HyperScale (VxHS)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 14, 2017 at 08:51:56 -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                            | 29 +++++++++
>  src/qemu/qemu_hotplug.c                            | 73 ++++++++++++++++++++++
>  ...-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, 240 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 ca6e213b4..458b90d8e 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;
> +    }
> +
>      if (!(server = qemuBlockStorageSourceBuildJSONSocketAddress(src->hosts, true)))
>          return NULL;
>  
>      /* VxHS disk specification example:
>       * { driver:"vxhs",
> +     *   [tls-creds:"objvirtio-disk0_tls0",]

Be careful with square brackets in JSON, they denote an array. 

>       *   vdisk-id:"eb90327c-8302-4725-4e85ed4dc251",
>       *   server:[{type:"tcp", host:"1.2.3.4", port:9999}]}
>       */
>      if (virJSONValueObjectCreate(&ret,
>                                   "s:driver", protocol,
> +                                 "S:tls-creds", src->tlsAlias,
>                                   "s:vdisk-id", src->path,
>                                   "a:server", server, NULL) < 0)
>          virJSONValueFree(server);
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 0a3278510..7b98e1947 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -794,6 +794,32 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
>  }
>  
>  
> +/* qemuBuildDiskTLSx509CommandLine:
> + *
> + * Add TLS object if the disk uses a secure communication channel
> + *
> + * Returns 0 on success, -1 w/ error on some sort of failure.
> + */
> +static int
> +qemuBuildDiskTLSx509CommandLine(virCommandPtr cmd,
> +                                virQEMUDriverConfigPtr cfg,
> +                                virDomainDiskDefPtr disk,
> +                                virQEMUCapsPtr qemuCaps)
> +{
> +    virStorageSourcePtr src = disk->src;
> +
> +    /* other protocols may be added later */
> +    if (src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS &&
> +        disk->src->haveTLS == VIR_TRISTATE_BOOL_YES) {
> +        return qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
> +                                          false, true, false,
> +                                          disk->info.alias, qemuCaps);
> +    }
> +
> +    return 0;
> +}
> +
> +
>  static char *
>  qemuBuildNetworkDriveURI(virStorageSourcePtr src,
>                           qemuDomainSecretInfoPtr secinfo)

[...]

> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index b365078ec..e4174af35 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -152,6 +152,55 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
>  
>  
>  static int
> +qemuDomainAddDiskTLSObject(virQEMUDriverPtr driver,
> +                           virDomainObjPtr vm,
> +                           virDomainDiskDefPtr disk,
> +                           char **tlsAlias)

I don't particularly like this. You set it into the disk object, so
there should be a function which rolls this back for a disk (or
preferably a virStorageSource).

> +{
> +    int ret = -1;
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    virStorageSourcePtr src = disk->src;
> +    virJSONValuePtr tlsProps = NULL;
> +
> +    /* NB: This may alter haveTLS based on cfg */
> +    qemuDomainPrepareDiskSourceTLS(src, disk->info.alias, cfg);

This should be executed in the hotplug API function and not in a helper.

> +
> +    if (src->haveTLS != VIR_TRISTATE_BOOL_YES) {
> +        ret = 0;
> +        goto cleanup;
> +    }
> +
> +    /* Initial implementation doesn't require/use a secret to decrypt
> +     * a server certificate, so there's no need to manage a tlsSecAlias
> +     * and tlsSecProps. See qemuDomainAddChardevTLSObjects for the
> +     * methodology required to add a secret object. */
> +
> +    /* For a VxHS environment, create a TLS object for the client to
> +     * connect to the VxHS server. */
> +    if (src->type == VIR_STORAGE_TYPE_NETWORK &&
> +        src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS &&
> +        qemuDomainGetTLSObjects(priv->qemuCaps, NULL,
> +                                cfg->vxhsTLSx509certdir, false, true,
> +                                disk->info.alias, &tlsProps, tlsAlias,
> +                                NULL, NULL) < 0)

This looks like another place that extracts qemu-specific stuff, which
need to be duplicated for blockdev add. Can't we make the cert dir part
of virStorageSource?


> +        goto cleanup;
> +
> +    if (qemuDomainAddTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE,
> +                                NULL, NULL, *tlsAlias, &tlsProps) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> +
> + cleanup:
> +    virJSONValueFree(tlsProps);
> +    virObjectUnref(cfg);
> +
> +    return ret;
> +}
> +
> +
> +static int
>  qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver,
>                              virDomainObjPtr vm,
>                              virDomainDiskDefPtr disk,

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux