From: John Ferlan Date: Tue, 8 Jan 2019 14:28:03 +0000 (-0500) Subject: rbd: Utilize storage pool namespace to manage config options X-Git-Tag: v5.1.0-rc1~324 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ab6ca812763b539e5380d8d6c4fa9da939125814;p=thirdparty%2Flibvirt.git rbd: Utilize storage pool namespace to manage config options Allow for adjustment of RBD configuration options via Storage Pool XML Namespace adjustments. When namespace arguments are used to start the pool, add a VIR_WARN to indicate that the startup was tainted by custom config_opts. Based off original patch/concept: https://www.redhat.com/archives/libvir-list/2014-May/msg00940.html Signed-off-by: John Ferlan Reviewed-by: Daniel P. Berrangé --- diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 7a79ec82d8..d19bc579a4 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -516,7 +516,8 @@ XML syntax targeted solely for the needs of the specific pool type which is not otherwise supported in standard XML. For the "fs" and "netfs" pool types this provides a mechanism to provide additional - mount options on the command line. + mount options on the command line. For the "rbd" pool this provides + a mechanism to override default settings for RBD configuration options.

Usage of namespaces comes with no support guarantees. It is intended @@ -569,6 +570,55 @@ Since 5.1.0. +

rbd:config_opts
+
Provides an XML namespace mechanism to optionally utilize + specifically named options for the RBD configuration options + via the rados_conf_set API for the rbd type + storage pools. In order to designate that the Storage Pool + will be using the mechanism, the pool element + must be modified to provide the XML namespace attribute + syntax as follows: + +

+ xmlns:rbd='http://libvirt.org/schemas/storagepool/source/rbd/1.0' +

+ +

+ The rbd:config_opts defines the configuration options + by specifying multiple rbd:option subelements with + the attribute name specifying the configuration option + to be added and value 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. +

+ + The following XML snippet shows the syntax required in order to + utilize + 
+<pool type="rbd" xmlns:rbd='http://libvirt.org/schemas/storagepool/source/rbd/1.0'>
+  <name>myrbdpool</name>
+...
+  <source>
+...
+  </source>
+...
+  <target>
+...
+  </target>
+...
+  <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>
+</pool>
+
+ + Since 5.1.0.
+

Storage volume XML

diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 0b359669af..e1944ff8e1 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -156,6 +156,9 @@ + + + @@ -705,4 +708,24 @@ + + + + + + + + + + + + + + + + + diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 24dd1349ae..2348f80146 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 #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -50,6 +51,138 @@ struct _virStorageBackendRBDState { typedef struct _virStorageBackendRBDState virStorageBackendRBDState; typedef virStorageBackendRBDState *virStorageBackendRBDStatePtr; +typedef struct _virStoragePoolRBDConfigOptionsDef virStoragePoolRBDConfigOptionsDef; +typedef virStoragePoolRBDConfigOptionsDef *virStoragePoolRBDConfigOptionsDefPtr; +struct _virStoragePoolRBDConfigOptionsDef { + 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) +{ + virStoragePoolRBDConfigOptionsDefPtr 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]); + } + VIR_FREE(cmdopts->names); + VIR_FREE(cmdopts->values); + + VIR_FREE(cmdopts); +} + + +static int +virStoragePoolDefRBDNamespaceParse(xmlXPathContextPtr ctxt, + void **data) +{ + virStoragePoolRBDConfigOptionsDefPtr cmdopts = NULL; + xmlNodePtr *nodes = NULL; + int nnodes; + size_t i; + int ret = -1; + + if (xmlXPathRegisterNs(ctxt, BAD_CAST "rbd", + BAD_CAST STORAGE_POOL_RBD_NAMESPACE_HREF) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to register xml namespace '%s'"), + STORAGE_POOL_RBD_NAMESPACE_HREF); + return -1; + } + + nnodes = virXPathNodeSet("./rbd:config_opts/rbd:option", ctxt, &nodes); + if (nnodes < 0) + return -1; + + if (nnodes == 0) + return 0; + + if (VIR_ALLOC(cmdopts) < 0) + goto cleanup; + + if (VIR_ALLOC_N(cmdopts->names, nnodes) < 0 || + VIR_ALLOC_N(cmdopts->values, nnodes) < 0) + goto cleanup; + + for (i = 0; i < nnodes; i++) { + if (!(cmdopts->names[cmdopts->noptions] = + virXMLPropString(nodes[i], "name"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("no rbd option name specified")); + goto cleanup; + } + if (*cmdopts->names[cmdopts->noptions] == '\0') { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("empty rbd option name specified")); + goto cleanup; + } + if (!(cmdopts->values[cmdopts->noptions] = + virXMLPropString(nodes[i], "value"))) { + virReportError(VIR_ERR_XML_ERROR, + _("no rbd option value specified for name '%s'"), + cmdopts->names[cmdopts->noptions]); + goto cleanup; + } + if (*cmdopts->values[cmdopts->noptions] == '\0') { + virReportError(VIR_ERR_XML_ERROR, + _("empty rbd option value specified for name '%s'"), + cmdopts->names[cmdopts->noptions]); + goto cleanup; + } + cmdopts->noptions++; + } + + VIR_STEAL_PTR(*data, cmdopts); + ret = 0; + + cleanup: + VIR_FREE(nodes); + virStoragePoolDefRBDNamespaceFree(cmdopts); + return ret; +} + + +static int +virStoragePoolDefRBDNamespaceFormatXML(virBufferPtr buf, + void *nsdata) +{ + size_t i; + virStoragePoolRBDConfigOptionsDefPtr def = nsdata; + + if (!def) + return 0; + + virBufferAddLit(buf, "\n"); + virBufferAdjustIndent(buf, 2); + + for (i = 0; i < def->noptions; i++) { + virBufferEscapeString(buf, "names[i]); + virBufferEscapeString(buf, "value='%s'/>\n", def->values[i]); + } + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "\n"); + + return 0; +} + + +static const char * +virStoragePoolDefRBDNamespaceHref(void) +{ + return "xmlns:rbd='" STORAGE_POOL_RBD_NAMESPACE_HREF "'"; +} + + static int virStorageBackendRBDRADOSConfSet(rados_t cluster, const char *option, @@ -69,10 +202,11 @@ virStorageBackendRBDRADOSConfSet(rados_t cluster, static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, - virStoragePoolSourcePtr source) + virStoragePoolDefPtr def) { int ret = -1; int r = 0; + virStoragePoolSourcePtr source = &def->source; virStorageAuthDefPtr authdef = source->auth; unsigned char *secret_value = NULL; size_t secret_value_size = 0; @@ -183,6 +317,22 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, rbd_default_format) < 0) goto cleanup; + if (def->namespaceData) { + virStoragePoolRBDConfigOptionsDefPtr cmdopts = def->namespaceData; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + for (i = 0; i < cmdopts->noptions; i++) { + if (virStorageBackendRBDRADOSConfSet(ptr->cluster, + cmdopts->names[i], + cmdopts->values[i]) < 0) + goto cleanup; + } + + virUUIDFormat(def->uuid, uuidstr); + VIR_WARN("Storage Pool name='%s' uuid='%s' is tainted by custom " + "config_opts from XML", def->name, uuidstr); + } + ptr->starttime = time(0); if ((r = rados_connect(ptr->cluster)) < 0) { virReportSystemError(-r, _("failed to connect to the RADOS monitor on: %s"), @@ -256,7 +406,7 @@ virStorageBackendRBDNewState(virStoragePoolObjPtr pool) if (VIR_ALLOC(ptr) < 0) return NULL; - if (virStorageBackendRBDOpenRADOSConn(ptr, &def->source) < 0) + if (virStorageBackendRBDOpenRADOSConn(ptr, def) < 0) goto error; if (virStorageBackendRBDOpenIoCTX(ptr, pool) < 0) @@ -1277,6 +1427,7 @@ virStorageBackendRBDVolWipe(virStoragePoolObjPtr pool, return ret; } + virStorageBackend virStorageBackendRBD = { .type = VIR_STORAGE_POOL_RBD, @@ -1291,8 +1442,20 @@ virStorageBackend virStorageBackendRBD = { }; +static virStoragePoolXMLNamespace virStoragePoolRBDXMLNamespace = { + .parse = virStoragePoolDefRBDNamespaceParse, + .free = virStoragePoolDefRBDNamespaceFree, + .format = virStoragePoolDefRBDNamespaceFormatXML, + .href = virStoragePoolDefRBDNamespaceHref, +}; + + int virStorageBackendRBDRegister(void) { - return virStorageBackendRegister(&virStorageBackendRBD); + if (virStorageBackendRegister(&virStorageBackendRBD) < 0) + return -1; + + return virStorageBackendNamespaceInit(VIR_STORAGE_POOL_RBD, + &virStoragePoolRBDXMLNamespace); } diff --git a/tests/storagepoolxml2xmlin/pool-rbd-ns-configopts.xml b/tests/storagepoolxml2xmlin/pool-rbd-ns-configopts.xml new file mode 100644 index 0000000000..6b62aa6f7e --- /dev/null +++ b/tests/storagepoolxml2xmlin/pool-rbd-ns-configopts.xml @@ -0,0 +1,17 @@ + + ceph + 47c1faee-0207-e741-f5ae-d9b019b98fe2 + + rbd + + + + + + + + + + + + diff --git a/tests/storagepoolxml2xmlout/pool-rbd-ns-configopts.xml b/tests/storagepoolxml2xmlout/pool-rbd-ns-configopts.xml new file mode 100644 index 0000000000..342a0ff74a --- /dev/null +++ b/tests/storagepoolxml2xmlout/pool-rbd-ns-configopts.xml @@ -0,0 +1,20 @@ + + ceph + 47c1faee-0207-e741-f5ae-d9b019b98fe2 + 0 + 0 + 0 + + + + rbd + + + + + + + + + + diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c index aff9ff160c..90d00a8d9e 100644 --- a/tests/storagepoolxml2xmltest.c +++ b/tests/storagepoolxml2xmltest.c @@ -106,6 +106,7 @@ mymain(void) DO_TEST("pool-zfs"); DO_TEST("pool-zfs-sourcedev"); DO_TEST("pool-rbd"); + DO_TEST("pool-rbd-ns-configopts"); DO_TEST("pool-vstorage"); DO_TEST("pool-iscsi-direct-auth"); DO_TEST("pool-iscsi-direct");