On 09/27/2017 09:02 AM, Peter Krempa wrote:
> On Tue, Sep 19, 2017 at 21:32:44 -0400, John Ferlan wrote:
>> From: Ashish Mittal <Ashish.Mittal@xxxxxxxxxxx>
>>
>> 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.
>>
>> Update the qemuxml2xmltest in order to add a test to show the proper
>> parsing.
>>
>> Also update the docs to describe the tls attribute plus clean up the
>> description in the surrounding area to make the information a bit more
>> readable rather than one winding paragraph.
>>
>> Signed-off-by: Ashish Mittal <Ashish.Mittal@xxxxxxxxxxx>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> docs/formatdomain.html.in | 40 ++++++++++++++++------
>> docs/schemas/domaincommon.rng | 5 +++
>> src/conf/domain_conf.c | 29 ++++++++++++++--
>> src/util/virstoragefile.c | 2 ++
>> src/util/virstoragefile.h | 7 ++++
>> ...emuxml2argv-disk-drive-network-tlsx509-vxhs.xml | 32 +++++++++++++++++
>> ...uxml2xmlout-disk-drive-network-tlsx509-vxhs.xml | 34 ++++++++++++++++++
>> tests/qemuxml2xmltest.c | 1 +
>> 8 files changed, 138 insertions(+), 12 deletions(-)
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml
>> create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-tlsx509-vxhs.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index 9ce4620c6..3e10213b5 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -2514,19 +2514,39 @@
>> <dd>
>> The <code>protocol</code> attribute specifies the protocol to
>> access to the requested image. Possible values are "nbd",
>> - "iscsi", "rbd", "sheepdog", "gluster" or "vxhs". If the
>> - <code>protocol</code> attribute is "rbd", "sheepdog", "gluster"
>> - or "vxhs", an additional attribute <code>name</code> is
>> - mandatory to specify which volume/image will be used. For "nbd",
>> - the <code>name</code> attribute is optional. For "iscsi"
>> - (<span class="since">since 1.0.4</span>), the <code>name</code>
>> - attribute may include a logical unit number, separated from the
>> - target's name by a slash (e.g.,
>> + "iscsi", "rbd", "sheepdog", "gluster" or "vxhs".
>> +
>> + <p>If the <code>protocol</code> attribute is "rbd", "sheepdog",
>> + "gluster", or "vxhs", an additional attribute <code>name</code>
>> + is mandatory to specify which volume/image will be used.
>> + </p>
>> +
>> + <p>For "nbd", the <code>name</code> attribute is optional.
>> + </p>
>> +
>> + <p>For "iscsi" (<span class="since">since 1.0.4</span>), the
>> + <code>name</code> attribute may include a logical unit number,
>> + separated from the target's name by a slash (e.g.,
>> <code>iqn.2013-07.com.example:iscsi-pool/1</code>). If not
>> specified, the default LUN is zero.
>> - For "vxhs" (<span class="since">since 3.8.0</span>), the
>> + </p>
>> +
>> + <p>For "vxhs" (<span class="since">since 3.8.0</span>), the
>> <code>name</code> is the UUID of the volume, assigned by the
>> - HyperScale server.
>
> Everything above is not really relevant to adding TLS to VxHS and thus
> deserves a separate patch.
>
I can separate - not a problem. The reason it was done the way it was is
because as I was editing the section it became painfully obvious that
the text was becoming impossible to read.
I did note that in the commit message BTW.
>> + HyperScale server. Additionally, an optional attribute
>> + <code>tls</code> (QEMU only) can be used to control whether a
>> + VxHS block device would utilize a hypervisor configured TLS
>> + X.509 certificate environment in order to encrypt the data
>> + channel. For the QEMU hypervisor, usage of a TLS environment can
>> + also be globally controlled on the host by the
>> + <code>vxhs_tls</code> and <code>vxhs_tls_x509_cert_dir</code> or
>> + <code>default_tls_x509_cert_dir</code> settings in the file
>> + /etc/libvirt/qemu.conf. If <code>vxhs_tls</code> is enabled,
>> + then unless the domain <code>tls</code> attribute is set to "no",
>> + libvirt will use the host configured TLS environment. If the
>> + <code>tls</code> attribute is set to "yes", then regardless of
>> + the qemu.conf setting, TLS authentication will be attempted.
>> + </p>
>> <span class="since">Since 0.8.7</span>
>> </dd>
>> <dt><code>volume</code></dt>
>
> [...]
>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index cc5e79b70..a568d9140 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>
> [...]
>
>> @@ -8147,6 +8148,19 @@ virDomainDiskSourceParse(xmlNodePtr node,
>> goto cleanup;
>> }
>>
>> + /* Check tls=yes|no domain setting for the block device
>> + * At present only VxHS. Other block devices may be added later */
>> + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS &&
>> + (haveTLS = virXMLPropString(node, "tls"))) {
>> + if ((src->haveTLS =
>> + virTristateBoolTypeFromString(haveTLS)) <= 0) {
>> + virReportError(VIR_ERR_XML_ERROR,
>> + _("unknown disk source 'tls' setting '%s'"),
>> + haveTLS);
>> + goto cleanup;
>> + }
>> + }
>> +
>> /* for historical reasons the volume name for gluster volume is stored
>> * as a part of the path. This is hard to work with when dealing with
>> * relative names. Split out the volume into a separate variable */
>
> [...]
>
>> @@ -21690,6 +21706,14 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf,
>>
>> VIR_FREE(path);
>>
>> + if (src->haveTLS != VIR_TRISTATE_BOOL_ABSENT &&
>> + !(flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE &&
>> + src->tlsFromConfig))
>> + virBufferAsprintf(attrBuf, " tls='%s'",
>> + virTristateBoolTypeToString(src->haveTLS));
>> + if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS)
>> + virBufferAsprintf(attrBuf, " tlsFromConfig='%d'", src->tlsFromConfig);
>
> Why is this attribute formatted as a number? Also you don't really seem
> to parse it.
>
It follows what was done for <chardev>...
As for the parsing, strange it was there in my recollection, but it's
not there in reality. I wonder where it went. Oh it gets really ugly to
try and squash in something since virDomainDiskSourceParse will need
@flags <sigh>
Tks -
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
