src/conf/domain_conf.c | 183 ++++++++++++++++++++++++++----------------------- 1 file changed, 99 insertions(+), 84 deletions(-)
virDomainDiskSourceParse got to the point of being an ugly spaghetti
mess by adding more and more stuff into it. Split out parsing of network
disk information into a separate function so that it stays contained.
---
I've had this patch on a branch that makes virDomainDiskSourceParse
uglier, but with the recent VxHS patches I've got a merge conflict, thus
I'm sending it now.
Note: this patch was generated with the "patience" diff algorithm
src/conf/domain_conf.c | 183 ++++++++++++++++++++++++++-----------------------
1 file changed, 99 insertions(+), 84 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 87192eb2d..98f5666ef 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8106,18 +8106,109 @@ virDomainDiskSourcePoolDefParse(xmlNodePtr node,
}
-int
-virDomainDiskSourceParse(xmlNodePtr node,
- xmlXPathContextPtr ctxt,
- virStorageSourcePtr src,
- unsigned int flags)
+static int
+virDomainDiskSourceNetworkParse(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ virStorageSourcePtr src,
+ unsigned int flags)
{
- int ret = -1;
char *protocol = NULL;
- xmlNodePtr saveNode = ctxt->node;
char *haveTLS = NULL;
char *tlsCfg = NULL;
int tlsCfgVal;
+ int ret = -1;
+
+ if (!(protocol = virXMLPropString(node, "protocol"))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing network source protocol type"));
+ goto cleanup;
+ }
+
+ if ((src->protocol = virStorageNetProtocolTypeFromString(protocol)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown protocol type '%s'"), protocol);
+ goto cleanup;
+ }
+
+ if (!(src->path = virXMLPropString(node, "name")) &&
+ src->protocol != VIR_STORAGE_NET_PROTOCOL_NBD) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing name for disk source"));
+ 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;
+ }
+ }
+
+ 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;
+ }
+
+ /* 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 */
+ if (src->path && src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) {
+ char *tmp;
+ if (!(tmp = strchr(src->path, '/')) ||
+ tmp == src->path) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("missing volume name or file name in "
+ "gluster source path '%s'"), src->path);
+ goto cleanup;
+ }
+
+ src->volume = src->path;
+
+ if (VIR_STRDUP(src->path, tmp) < 0)
+ goto cleanup;
+
+ tmp[0] = '\0';
+ }
+
+ /* snapshot currently works only for remote disks */
+ src->snapshot = virXPathString("string(./snapshot/@name)", ctxt);
+
+ /* config file currently only works with remote disks */
+ src->configFile = virXPathString("string(./config/@file)", ctxt);
+
+ if (virDomainStorageNetworkParseHosts(node, &src->hosts, &src->nhosts) < 0)
+ goto cleanup;
+
+ virStorageSourceNetworkAssignDefaultPorts(src);
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(protocol);
+ return ret;
+}
+
+
+int
+virDomainDiskSourceParse(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ virStorageSourcePtr src,
+ unsigned int flags)
+{
+ int ret = -1;
+ xmlNodePtr saveNode = ctxt->node;
ctxt->node = node;
@@ -8132,81 +8223,8 @@ virDomainDiskSourceParse(xmlNodePtr node,
src->path = virXMLPropString(node, "dir");
break;
case VIR_STORAGE_TYPE_NETWORK:
- if (!(protocol = virXMLPropString(node, "protocol"))) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing network source protocol type"));
+ if (virDomainDiskSourceNetworkParse(node, ctxt, src, flags) < 0)
goto cleanup;
- }
-
- if ((src->protocol = virStorageNetProtocolTypeFromString(protocol)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown protocol type '%s'"), protocol);
- goto cleanup;
- }
-
- if (!(src->path = virXMLPropString(node, "name")) &&
- src->protocol != VIR_STORAGE_NET_PROTOCOL_NBD) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing name for disk source"));
- 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;
- }
- }
-
- 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;
- }
-
- /* 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 */
- if (src->path && src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) {
- char *tmp;
- if (!(tmp = strchr(src->path, '/')) ||
- tmp == src->path) {
- virReportError(VIR_ERR_XML_ERROR,
- _("missing volume name or file name in "
- "gluster source path '%s'"), src->path);
- goto cleanup;
- }
-
- src->volume = src->path;
-
- if (VIR_STRDUP(src->path, tmp) < 0)
- goto cleanup;
-
- tmp[0] = '\0';
- }
-
- /* snapshot currently works only for remote disks */
- src->snapshot = virXPathString("string(./snapshot/@name)", ctxt);
-
- /* config file currently only works with remote disks */
- src->configFile = virXPathString("string(./config/@file)", ctxt);
-
- if (virDomainStorageNetworkParseHosts(node, &src->hosts, &src->nhosts) < 0)
- goto cleanup;
-
- virStorageSourceNetworkAssignDefaultPorts(src);
-
break;
case VIR_STORAGE_TYPE_VOLUME:
if (virDomainDiskSourcePoolDefParse(node, &src->srcpool) < 0)
@@ -8229,9 +8247,6 @@ virDomainDiskSourceParse(xmlNodePtr node,
ret = 0;
cleanup:
- VIR_FREE(protocol);
- VIR_FREE(haveTLS);
- VIR_FREE(tlsCfg);
ctxt->node = saveNode;
return ret;
}
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/29/2017 05:02 AM, Peter Krempa wrote: > virDomainDiskSourceParse got to the point of being an ugly spaghetti > mess by adding more and more stuff into it. Split out parsing of network > disk information into a separate function so that it stays contained. > --- > I've had this patch on a branch that makes virDomainDiskSourceParse > uglier, but with the recent VxHS patches I've got a merge conflict, thus > I'm sending it now. > > Note: this patch was generated with the "patience" diff algorithm > > > src/conf/domain_conf.c | 183 ++++++++++++++++++++++++++----------------------- > 1 file changed, 99 insertions(+), 84 deletions(-) > Reviewed-by: John Ferlan <jferlan@redhat.com> Seems "safe" to me too. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Sep 29, 2017 at 09:12:34AM -0400, John Ferlan wrote: > > >On 09/29/2017 05:02 AM, Peter Krempa wrote: >> virDomainDiskSourceParse got to the point of being an ugly spaghetti >> mess by adding more and more stuff into it. Split out parsing of network >> disk information into a separate function so that it stays contained. >> --- >> I've had this patch on a branch that makes virDomainDiskSourceParse >> uglier, but with the recent VxHS patches I've got a merge conflict, thus >> I'm sending it now. >> >> Note: this patch was generated with the "patience" diff algorithm >> >> >> src/conf/domain_conf.c | 183 ++++++++++++++++++++++++++----------------------- >> 1 file changed, 99 insertions(+), 84 deletions(-) >> > >Reviewed-by: John Ferlan <jferlan@redhat.com> > >Seems "safe" to me too. > Please don't push refactors or even pure code movement during the freeze. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.