[libvirt] [PATCH v2 6/6] rbd: Utilize storage pool namespace to manage config options

[Michal Privoznik]

On 1/8/19 6:52 PM, John Ferlan wrote:
> Allow for adjustment of RBD configuration options via Storage
> Pool XML Namespace adjustments.
> 
> Based off original patch/concept:
> 
> https://www.redhat.com/archives/libvir-list/2014-May/msg00940.html
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  docs/formatstorage.html.in                    |  45 +++++
>  docs/schemas/storagepool.rng                  |  23 +++
>  src/storage/storage_backend_rbd.c             | 159 +++++++++++++++++-
>  .../pool-rbd-configopts.xml                   |  17 ++
>  .../pool-rbd-configopts.xml                   |  20 +++
>  tests/storagepoolxml2xmltest.c                |   1 +
>  6 files changed, 264 insertions(+), 1 deletion(-)
>  create mode 100644 tests/storagepoolxml2xmlin/pool-rbd-configopts.xml
>  create mode 100644 tests/storagepoolxml2xmlout/pool-rbd-configopts.xml
> 
> diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
> index 308b94f5e5..4e0fe0a981 100644
> --- a/docs/formatstorage.html.in
> +++ b/docs/formatstorage.html.in
> @@ -437,6 +437,51 @@
>  
>        <span class="since">Since 5.0.0.</span></dd>
>  
> +      <dt><code>rbd:config_opts</code></dt>
> +      <dd>Provides an XML namespace mechanism to optionally utilize
> +        specifically named options for the RBD configuration options
> +        via the rados_conf_set API for the <code>rbd</code> type
> +        storage pools. In order to designate that the Storage Pool
> +        will be using the mechanism, the <code>pool</code> element
> +        must be modified to provide the XML namespace attribute
> +        syntax as follows:
> +
> +        <p>
> +        xmlns:rbd='http://libvirt.org/schemas/storagepool/source/rbd/1.0'
> +        </p>
> +
> +        <p>
> +        The <code>rbd:config_opts</code> defines the configuration options
> +        by specifying multiple <code>rbd:option</code> subelements with
> +        the attribute <code>name</code> specifying the configuration option
> +        to be added and <code>value</code> specifying the configuration
> +        option value. The name and value for each option is only checked
> +        to be not empty. The name and value provided are not checked since
> +        it's possible options don't exist on all distributions. It is
> +        expected that proper and valid options will be supplied for the
> +        target host.
> +        </p>
> +
> +        The following XML snippet shows the syntax required in order to
> +        utilize
> +      <pre>
> +<pool type="rbd" xmlns:rbd='http://libvirt.org/schemas/storagepool/source/rbd/1.0'>
> +  <name>myrbdpool</name>
> +...
> +  <source>
> +    <name>rbdpool</name>
> +    <host name='1.2.3.4'/>
> +    <host name='my.ceph.monitor'/>
> +    <host name='third.ceph.monitor' port='6789'/>
> +    <rbd:config_opts>
> +      <rbd:option name='client_mount_timeout' value='45'/>
> +      <rbd:option name='rados_mon_op_timeout' value='20'/>
> +      <rbd:option name='rados_osd_op_timeout' value='10'/>
> +    </rbd:config_opts>
> +  </source>
> +...</pre>
> +
> +      <span class="since">Since 5.0.0.</span></dd>
>      </dl>
>  
>      <h3><a id="StoragePoolTarget">Target elements</a></h3>
> diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
> index 20c7ae5744..54fa584828 100644
> --- a/docs/schemas/storagepool.rng
> +++ b/docs/schemas/storagepool.rng
> @@ -647,6 +647,9 @@
>          <optional>
>            <ref name='sourceinfoauth'/>
>          </optional>
> +        <optional>
> +          <ref name='rbd_config_opts'/>
> +        </optional>
>        </interleave>
>      </element>
>    </define>
> @@ -695,4 +698,24 @@
>      </element>
>    </define>
>  
> +  <!--
> +       Optional storage pool extensions in their own namespace:
> +         RBD
> +    -->
> +
> +  <define name="rbd_config_opts">
> +    <element name="config_opts" ns="http://libvirt.org/schemas/storagepool/source/rbd/1.0">
> +      <zeroOrMore>
> +        <element name="option">
> +          <attribute name='name'>
> +            <text/>
> +          </attribute>
> +          <attribute name='value'>
> +            <text/>
> +          </attribute>
> +        </element>
> +      </zeroOrMore>
> +    </element>
> +  </define>
> +
>  </grammar>
> diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
> index 24dd1349ae..c419b12e2d 100644
> --- a/src/storage/storage_backend_rbd.c
> +++ b/src/storage/storage_backend_rbd.c
> @@ -36,6 +36,7 @@
>  #include "rbd/librbd.h"
>  #include "secret_util.h"
>  #include "storage_util.h"
> +#include <libxml/xpathInternals.h>
>  
>  #define VIR_FROM_THIS VIR_FROM_STORAGE
>  
> @@ -50,6 +51,138 @@ struct _virStorageBackendRBDState {
>  typedef struct _virStorageBackendRBDState virStorageBackendRBDState;
>  typedef virStorageBackendRBDState *virStorageBackendRBDStatePtr;
>  
> +/* NetFS Storage Pool Namespace options to share w/ storage_backend_fs.c and
> + * the virStorageBackendFileSystemMountCmd method */
> +typedef struct _virStoragePoolRBDMountOptionsDef virStoragePoolRBDMountOptionsDef;
> +typedef virStoragePoolRBDMountOptionsDef *virStoragePoolRBDMountOptionsDefPtr;
> +struct _virStoragePoolRBDMountOptionsDef {
> +    size_t noptions;
> +    char **names;
> +    char **values;
> +};
> +
> +#define STORAGE_POOL_RBD_NAMESPACE_HREF "http://libvirt.org/schemas/storagepool/source/rbd/1.0"
> +
> +static void
> +virStoragePoolDefRBDNamespaceFree(void *nsdata)
> +{
> +    virStoragePoolRBDMountOptionsDefPtr cmdopts = nsdata;
> +    size_t i;
> +
> +    if (!cmdopts)
> +        return;
> +
> +    for (i = 0; i < cmdopts->noptions; i++) {
> +        VIR_FREE(cmdopts->names[i]);
> +        VIR_FREE(cmdopts->values[i]);
> +    }
> +

Again, VIR_FREE(cmdopts->names) and VIR_FREE(cmdopts->values);

> +    VIR_FREE(cmdopts);
> +}


Michal