Re: [libvirt] [PATCH v10 3/5] util: Add TLS attributes to virStorageSource
On 09/28/2017 09:25 AM, Peter Krempa wrote:
On Wed, Sep 27, 2017 at 11:45:53 -0400, John Ferlan wrote:
> From: Ashish Mittal <Ashish.Mittal(a)veritas.com>
>
> Add an optional virTristateBool haveTLS to virStorageSource to
> manage whether a storage source will be using TLS.
>
> Sample XML for a VxHS disk:
>
> <disk type='network' device='disk'>
> <driver name='qemu' type='raw' cache='none'/>
> <source protocol='vxhs'
name='eb90327c-8302-4725-9e1b-4e85ed4dc251' tls='yes'>
> <host name='192.168.0.1' port='9999'/>
> </source>
> <target dev='vda' bus='virtio'/>
> </disk>
>
> Additionally add a tlsFromConfig boolean to control whether the TLS
> setting was due to domain configuration or qemu.conf global setting
> in order to decide whether to Format the haveTLS setting for either
> a live or saved domain configuration file.
I'm still unhappy that you've disregarded my comment and still format
this as an integer. I don't really buy the argument that it should be
this way because it's done the wrong way somewhere else.
Said this ... ACK regardless.
I guess I was going for consistency with the existing model.
I can format and parse as 'true' and 'false' though, would the
following suffice (sorry, I know you dislike cut-n-paste output,
but this was quicker)?
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 87192eb2d..2d8e573c9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8166,13 +8166,8 @@ virDomainDiskSourceParse(xmlNodePtr node,
if ((flags & VIR_DOMAIN_DEF_PARSE_STATUS) &&
(tlsCfg = virXMLPropString(node, "tlsFromConfig"))) {
- if (virStrToLong_i(tlsCfg, NULL, 10, &tlsCfgVal) < 0) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Invalid tlsFromConfig value: %s"),
- tlsCfg);
- goto cleanup;
- }
- src->tlsFromConfig = !!tlsCfgVal;
+ if (tlsCfg && STREQ(tlsCfg, "true"))
+ src->tlsFromConfig = true;
}
/* for historical reasons the volume name for gluster volume is stored
@@ -21729,7 +21724,8 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf,
virBufferAsprintf(attrBuf, " tls='%s'",
virTristateBoolTypeToString(src->haveTLS));
if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS)
- virBufferAsprintf(attrBuf, " tlsFromConfig='%d'",
src->tlsFromConfig);
+ virBufferAsprintf(attrBuf, " tlsFromConfig='%s'",
+ src->tlsFromConfig ? "true" : "false");
for (n = 0; n < src->nhosts; n++) {
virBufferAddLit(childBuf, "<host");