[libvirt] [PATCH v3 08/18] conf: Extract SCSI adapter type processing into their own helpers

John Ferlan jferlan at redhat.com
Fri Mar 10 21:10:40 UTC 2017


Rather than have lots of ugly inline code create helpers to try and
make things more readable. While creating the helpers realign the code
as necessary.

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 src/conf/storage_adapter_conf.c | 239 +++++++++++++++++++++++-----------------
 1 file changed, 138 insertions(+), 101 deletions(-)

diff --git a/src/conf/storage_adapter_conf.c b/src/conf/storage_adapter_conf.c
index a2d4a3a..a361c34 100644
--- a/src/conf/storage_adapter_conf.c
+++ b/src/conf/storage_adapter_conf.c
@@ -52,12 +52,11 @@ virStorageAdapterFCHostClear(virStoragePoolSourceAdapterPtr adapter)
 void
 virStorageAdapterClear(virStoragePoolSourceAdapterPtr adapter)
 {
-    if (adapter->type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+    if (adapter->type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST)
         virStorageAdapterFCHostClear(adapter);
-    } else if (adapter->type ==
-               VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
+
+    if (adapter->type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST)
         VIR_FREE(adapter->data.scsi_host.name);
-    }
 }
 
 
@@ -95,6 +94,83 @@ virStorageAdapterFCHostParseXML(xmlNodePtr node,
 }
 
 
+static int
+virStorageAdapterSCSIHostParseXML(xmlNodePtr node,
+                                  xmlXPathContextPtr ctxt,
+                                  virStoragePoolSourcePtr source)
+{
+    source->adapter.data.scsi_host.name =
+        virXMLPropString(node, "name");
+    if (virXPathNode("./parentaddr", ctxt)) {
+        xmlNodePtr addrnode = virXPathNode("./parentaddr/address", ctxt);
+        virPCIDeviceAddressPtr addr =
+            &source->adapter.data.scsi_host.parentaddr;
+
+        if (!addrnode) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("Missing scsi_host PCI address element"));
+            return -1;
+        }
+        source->adapter.data.scsi_host.has_parent = true;
+        if (virPCIDeviceAddressParseXML(addrnode, addr) < 0)
+            return -1;
+        if ((virXPathInt("string(./parentaddr/@unique_id)",
+                         ctxt,
+                         &source->adapter.data.scsi_host.unique_id) < 0) ||
+            (source->adapter.data.scsi_host.unique_id < 0)) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("Missing or invalid scsi adapter "
+                             "'unique_id' value"));
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+
+static int
+virStorageAdapterLegacyParseXML(xmlNodePtr node,
+                                xmlXPathContextPtr ctxt,
+                                virStoragePoolSourcePtr source)
+{
+    char *wwnn = virXMLPropString(node, "wwnn");
+    char *wwpn = virXMLPropString(node, "wwpn");
+    char *parent = virXMLPropString(node, "parent");
+
+    /* "type" was not specified in the XML, so we must verify that
+     * "wwnn", "wwpn", "parent", or "parentaddr" are also not in the
+     * XML. If any are found, then we cannot just use "name" alone".
+     */
+    if (wwnn || wwpn || parent) {
+        VIR_FREE(wwnn);
+        VIR_FREE(wwpn);
+        VIR_FREE(parent);
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("Use of 'wwnn', 'wwpn', and 'parent' attributes "
+                         "requires use of the adapter 'type'"));
+        return -1;
+    }
+
+    if (virXPathNode("./parentaddr", ctxt)) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("Use of 'parent' element requires use "
+                         "of the adapter 'type'"));
+        return -1;
+    }
+
+    /* To keep back-compat, 'type' is not required to specify
+     * for scsi_host adapter.
+     */
+    if ((source->adapter.data.scsi_host.name =
+         virXMLPropString(node, "name")))
+        source->adapter.type =
+            VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST;
+
+    return 0;
+}
+
+
 int
 virStorageAdapterParseXML(virStoragePoolSourcePtr source,
                           xmlNodePtr node,
@@ -121,68 +197,13 @@ virStorageAdapterParseXML(virStoragePoolSourcePtr source,
                 goto cleanup;
         } else if (source->adapter.type ==
                    VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
+            if (virStorageAdapterSCSIHostParseXML(node, ctxt, source) < 0)
+                goto cleanup;
 
-            source->adapter.data.scsi_host.name =
-                virXMLPropString(node, "name");
-            if (virXPathNode("./parentaddr", ctxt)) {
-                xmlNodePtr addrnode = virXPathNode("./parentaddr/address",
-                                                   ctxt);
-                virPCIDeviceAddressPtr addr =
-                    &source->adapter.data.scsi_host.parentaddr;
-
-                if (!addrnode) {
-                    virReportError(VIR_ERR_XML_ERROR, "%s",
-                                   _("Missing scsi_host PCI address element"));
-                    goto cleanup;
-                }
-                source->adapter.data.scsi_host.has_parent = true;
-                if (virPCIDeviceAddressParseXML(addrnode, addr) < 0)
-                    goto cleanup;
-                if ((virXPathInt("string(./parentaddr/@unique_id)",
-                                 ctxt,
-                                 &source->adapter.data.scsi_host.unique_id) < 0) ||
-                    (source->adapter.data.scsi_host.unique_id < 0)) {
-                    virReportError(VIR_ERR_XML_ERROR, "%s",
-                                   _("Missing or invalid scsi adapter "
-                                     "'unique_id' value"));
-                    goto cleanup;
-                }
-            }
         }
     } else {
-        char *wwnn = virXMLPropString(node, "wwnn");
-        char *wwpn = virXMLPropString(node, "wwpn");
-        char *parent = virXMLPropString(node, "parent");
-
-        /* "type" was not specified in the XML, so we must verify that
-         * "wwnn", "wwpn", "parent", or "parentaddr" are also not in the
-         * XML. If any are found, then we cannot just use "name" alone".
-         */
-
-        if (wwnn || wwpn || parent) {
-            VIR_FREE(wwnn);
-            VIR_FREE(wwpn);
-            VIR_FREE(parent);
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("Use of 'wwnn', 'wwpn', and 'parent' attributes "
-                             "requires use of the adapter 'type'"));
+        if (virStorageAdapterLegacyParseXML(node, ctxt, source) < 0)
             goto cleanup;
-        }
-
-        if (virXPathNode("./parentaddr", ctxt)) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("Use of 'parent' element requires use "
-                             "of the adapter 'type'"));
-            goto cleanup;
-        }
-
-        /* To keep back-compat, 'type' is not required to specify
-         * for scsi_host adapter.
-         */
-        if ((source->adapter.data.scsi_host.name =
-             virXMLPropString(node, "name")))
-            source->adapter.type =
-                VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST;
     }
 
     ret = 0;
@@ -235,6 +256,29 @@ virStorageAdapterFCHostParseValidate(virStoragePoolDefPtr ret)
 }
 
 
+static int
+virStorageAdapterSCSIHostParseValidate(virStoragePoolDefPtr ret)
+{
+    if (!ret->source.adapter.data.scsi_host.name &&
+        !ret->source.adapter.data.scsi_host.has_parent) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("Either 'name' or 'parent' must be specified "
+                         "for the 'scsi_host' adapter"));
+        return -1;
+    }
+
+    if (ret->source.adapter.data.scsi_host.name &&
+        ret->source.adapter.data.scsi_host.has_parent) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("Both 'name' and 'parent' cannot be specified "
+                         "for the 'scsi_host' adapter"));
+        return -1;
+    }
+
+    return 0;
+}
+
+
 int
 virStorageAdapterParseValidate(virStoragePoolDefPtr ret)
 {
@@ -245,26 +289,12 @@ virStorageAdapterParseValidate(virStoragePoolDefPtr ret)
     }
 
     if (ret->source.adapter.type ==
-        VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+        VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST)
         return virStorageAdapterFCHostParseValidate(ret);
-    } else if (ret->source.adapter.type ==
-               VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
-        if (!ret->source.adapter.data.scsi_host.name &&
-            !ret->source.adapter.data.scsi_host.has_parent) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("Either 'name' or 'parent' must be specified "
-                             "for the 'scsi_host' adapter"));
-            return -1;
-        }
 
-        if (ret->source.adapter.data.scsi_host.name &&
-            ret->source.adapter.data.scsi_host.has_parent) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("Both 'name' and 'parent' cannot be specified "
-                             "for the 'scsi_host' adapter"));
-            return -1;
-        }
-    }
+    if (ret->source.adapter.type ==
+        VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST)
+        return virStorageAdapterSCSIHostParseValidate(ret);
 
     return 0;
 }
@@ -292,6 +322,30 @@ virStorageAdapterFCHostFormat(virBufferPtr buf,
 }
 
 
+static void
+virStorageAdapterSCSIHostFormat(virBufferPtr buf,
+                                virStoragePoolSourcePtr src)
+{
+    if (src->adapter.data.scsi_host.name) {
+        virBufferAsprintf(buf, " name='%s'/>\n",
+                          src->adapter.data.scsi_host.name);
+    } else {
+        virPCIDeviceAddress addr;
+        virBufferAddLit(buf, ">\n");
+        virBufferAdjustIndent(buf, 2);
+        virBufferAsprintf(buf, "<parentaddr unique_id='%d'>\n",
+                          src->adapter.data.scsi_host.unique_id);
+        virBufferAdjustIndent(buf, 2);
+        addr = src->adapter.data.scsi_host.parentaddr;
+        ignore_value(virPCIDeviceAddressFormat(buf, addr, false));
+        virBufferAdjustIndent(buf, -2);
+        virBufferAddLit(buf, "</parentaddr>\n");
+        virBufferAdjustIndent(buf, -2);
+        virBufferAddLit(buf, "</adapter>\n");
+    }
+}
+
+
 void
 virStorageAdapterFormat(virBufferPtr buf,
                         virStoragePoolSourcePtr src)
@@ -299,26 +353,9 @@ virStorageAdapterFormat(virBufferPtr buf,
     virBufferAsprintf(buf, "<adapter type='%s'",
                       virStoragePoolSourceAdapterTypeToString(src->adapter.type));
 
-    if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+    if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST)
         virStorageAdapterFCHostFormat(buf, src);
-    } else if (src->adapter.type ==
-               VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
-        if (src->adapter.data.scsi_host.name) {
-            virBufferAsprintf(buf, " name='%s'/>\n",
-                              src->adapter.data.scsi_host.name);
-        } else {
-            virPCIDeviceAddress addr;
-            virBufferAddLit(buf, ">\n");
-            virBufferAdjustIndent(buf, 2);
-            virBufferAsprintf(buf, "<parentaddr unique_id='%d'>\n",
-                              src->adapter.data.scsi_host.unique_id);
-            virBufferAdjustIndent(buf, 2);
-            addr = src->adapter.data.scsi_host.parentaddr;
-            ignore_value(virPCIDeviceAddressFormat(buf, addr, false));
-            virBufferAdjustIndent(buf, -2);
-            virBufferAddLit(buf, "</parentaddr>\n");
-            virBufferAdjustIndent(buf, -2);
-            virBufferAddLit(buf, "</adapter>\n");
-        }
-    }
+
+    if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST)
+        virStorageAdapterSCSIHostFormat(buf, src);
 }
-- 
2.9.3




More information about the libvir-list mailing list