[libvirt] [PATCH v6 10/13] qemu: Introduce qemuDomainPrepareDiskSource

[Peter Krempa]

On Wed, Aug 30, 2017 at 18:46:10 -0400, John Ferlan wrote:
> Introduce a function to setup any TLS needs for a disk source.
> 
> If there's a configuration or other error setting up the disk source
> for TLS, then cause the domain startup to fail.
> 
> For VxHS, if cfg->haveTLS is set, then TLS creds will be added
> automatically to every VxHS disk that didn't specify "tls='no'"
> in the domain XML. Additionally, if the domain XML has "tls='yes'",
> but cfg->haveTLS is not set, then issue a configuration error.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
> 
>  This is NEW from v5. This patch adds the infrastructure to the
>  qemuDomainPrepare* family of functions in order to walk the disk
>  list looking for disks that need to be set up properly for TLS.
>  The code is very similar to the chardev model - except that it
>  can cause a failure to start the domain in the event that the
>  vxhsTLS = 0, but domain XML haveTLS = yes. Essentially pulling
>  in the code from v5 patch5 qemuBuildDiskVxHSTLSinfoCommandLine.
> 
> 
>  src/qemu/qemu_domain.c  | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_domain.h  |  5 +++++
>  src/qemu/qemu_process.c |  4 ++++
>  3 files changed, 67 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index cbee151..c3eadf3 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -7520,6 +7520,64 @@ qemuDomainPrepareChardevSource(virDomainDefPtr def,
>  }
>  
>  
> +/* qemuProcessPrepareDiskSourceTLS:
> + * @source: pointer to host interface data for disk device
> + * @cfg: driver configuration
> + *
> + * Updates host interface TLS encryption setting based on qemu.conf
> + * for disk devices.  This will be presented as "tls='yes|no'" in
> + * live XML of a guest.
> + *
> + * Returns 0 on success, -1 on bad config/failure
> + */
> +static int
> +qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src,
> +                               virQEMUDriverConfigPtr cfg)
> +{
> +
> +    /* VxHS doesn't utilize a password protected server certificate,
> +     * so no need to add a secinfo for a secret UUID. */
> +    if (src->type == VIR_STORAGE_TYPE_NETWORK &&
> +        src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS) {
> +
> +        if (src->haveTLS == VIR_TRISTATE_BOOL_YES && !cfg->vxhsTLS) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("VxHS TLS protocol is configured in XML, but "
> +                             "TLS for VxHS is disabled in qemu.conf"));
> +            return -1;
> +        }

As pointed out in the patch adding the TLS stuff, I disagree with this
condition.


> +
> +        if (src->haveTLS == VIR_TRISTATE_BOOL_ABSENT && cfg->vxhsTLS)
> +            src->haveTLS = VIR_TRISTATE_BOOL_YES;
> +    }
> +
> +    return 0;
> +}
> +
> +
> +/* qemuProcessPrepareDiskSource:
> + * @def: live domain definition
> + * @driver: qemu driver
> + *
> + * Iterate through all disk devices to setup/check any that would be
> + * using TLS.
> + *
> + * Returns 0 on success, -1 on failure
> + */
> +int
> +qemuDomainPrepareDiskSource(virDomainDefPtr def,
> +                            virQEMUDriverConfigPtr cfg)
> +{
> +    size_t i;
> +
> +    for (i = 0; i < def->ndisks; i++) {
> +        if (qemuDomainPrepareDiskSourceTLS(def->disks[i]->src, cfg) < 0)
> +            return -1;
> +    }
> +
> +    return 0;
> +}
> +
>  
>  int
>  qemuDomainPrepareShmemChardev(virDomainShmemDefPtr shmem)
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index f93b09b..f701287 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -841,6 +841,11 @@ void qemuDomainPrepareChardevSourceTLS(virDomainChrSourceDefPtr source,
>                                         virQEMUDriverConfigPtr cfg)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>  
> +int
> +qemuDomainPrepareDiskSource(virDomainDefPtr def,
> +                            virQEMUDriverConfigPtr cfg)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> +
>  void qemuDomainPrepareChardevSource(virDomainDefPtr def,
>                                      virQEMUDriverPtr driver)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 364c359..7978ef3 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5326,6 +5326,10 @@ qemuProcessPrepareDomain(virConnectPtr conn,
>      if (qemuDomainMasterKeyCreate(vm) < 0)
>          goto cleanup;
>  
> +    VIR_DEBUG("Prepare disk source backends for TLS");
> +    if (qemuDomainPrepareDiskSource(vm->def, cfg) < 0)
> +        goto cleanup;
> +
>      VIR_DEBUG("Prepare chardev source backends for TLS");
>      qemuDomainPrepareChardevSource(vm->def, driver);


I think I have a very similar patch somewhere on my branches. So ACK to
the rest.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170831/c16c02d6/attachment-0001.sig>