[libvirt] [PATCH 1/8] New XML attributes for storage pool source adapter

Osier Yang jyang at redhat.com
Mon Feb 4 13:31:58 UTC 2013


This introduces 4 new attributes for storage pool source adapter.
E.g.

<adapter type='fc_host' parent='scsi_host5' wwnn='20000000c9831b4b' wwpn='10000000c9831b4b'/>

Attribute 'type' can be either 'scsi_host' or 'fc_host', and defaults
to 'scsi_host' if attribute 'name' is specified. I.e. It's optional
for 'scsi_host' adapter, for back-compact reason. However, it's mandatory
for 'fc_host' adapter and any new future adapter types. Attribute 'parent'
is required for vHBA, but optional for HBA.

* docs/formatstorage.html.in:
  - Add documents for the 4 new attrs
* docs/schemas/storagepool.rng:
  - Add RNG schema
* src/conf/storage_conf.c:
  - Parse and format the new XMLs
* src/conf/storage_conf.h:
  - New struct virStoragePoolSourceAdapter, replace "char *adapter" with it;
  - New enum virStoragePoolSourceAdapterType
* src/libvirt_private.syms:
  - Export TypeToString and TypeFromString
* src/phyp/phyp_driver.c:
  - Replace "adapter" with "adapter.data.name", which is member of the union
    of the new struct virStoragePoolSourceAdapter now
* src/storage/storage_backend_scsi.c:
  - Like above
* tests/storagepoolxml2xmlin/pool-scsi1.xml:
  - New test for 'fc_host' adapter
* tests/storagepoolxml2xmlout/pool-scsi.xml:
  - Change the expected output, as the 'type' defaults to 'scsi_host' if 'name"
    specified now
* tests/storagepoolxml2xmlout/pool-scsi1.xml:
  - New test
* tests/storagepoolxml2xmltest.c:
  - Include the test
---
 docs/formatstorage.html.in                 |   13 +++-
 docs/schemas/storagepool.rng               |   33 +++++++-
 src/conf/storage_conf.c                    |  123 +++++++++++++++++++++++++--
 src/conf/storage_conf.h                    |   23 +++++-
 src/libvirt_private.syms                   |    2 +
 src/phyp/phyp_driver.c                     |    8 +-
 src/storage/storage_backend_scsi.c         |    6 +-
 tests/storagepoolxml2xmlin/pool-scsi1.xml  |   15 ++++
 tests/storagepoolxml2xmlout/pool-scsi.xml  |    2 +-
 tests/storagepoolxml2xmlout/pool-scsi1.xml |   18 ++++
 tests/storagepoolxml2xmltest.c             |    1 +
 11 files changed, 220 insertions(+), 24 deletions(-)
 create mode 100644 tests/storagepoolxml2xmlin/pool-scsi1.xml
 create mode 100644 tests/storagepoolxml2xmlout/pool-scsi1.xml

diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 9f93db8..0849618 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -88,8 +88,17 @@
         <span class="since">Since 0.4.1</span></dd>
       <dt><code>adapter</code></dt>
       <dd>Provides the source for pools backed by SCSI adapters. May
-        only occur once. Contains a single attribute <code>name</code>
-        which is the SCSI adapter name (ex. "host1").
+        only occur once. Attribute <code>name</code> is the SCSI adapter
+        name (ex. "host1"). Attribute <code>type</code>
+        (<span class="since">1.0.2</span>) specifies the adapter type,
+        valid value is "fc_host" or "scsi_host", defaults to "scsi_host"
+        if <code>name</code> is specified. To keep back-compact,
+        <code>type</code> is optional for "scsi_host" adapter, but
+        mandatory for "fc_host" adapter.  Attribute <code>wwnn</code> and
+        <code>wwpn</code> (<span class="since">1.0.2</span>) indicates
+        the (v)HBA. The optional attribute <code>parent</code>
+        (<span class="since">1.0.2</span>) specifies the parent device of
+        the (v)HBA. It's optional for HBA, but required for vHBA.
         <span class="since">Since 0.6.2</span></dd>
       <dt><code>host</code></dt>
       <dd>Provides the source for pools backed by storage from a
diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index 165e276..a3204ec 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -275,9 +275,36 @@
 
   <define name='sourceinfoadapter'>
     <element name='adapter'>
-      <attribute name='name'>
-        <text/>
-      </attribute>
+      <choice>
+        <group>
+          <!-- To keep back-compact, 'type' is not mandatory for
+           scsi_host adapter -->
+          <optional>
+            <attribute name='type'>
+              <value>scsi_host</value>
+            </attribute>
+          </optional>
+          <attribute name='name'>
+            <text/>
+          </attribute>
+        </group>
+        <group>
+          <attribute name='type'>
+            <value>fc_host</value>
+          </attribute>
+          <optional>
+            <attribute name='parent'>
+              <text/>
+            </attribute>
+          </optional>
+          <attribute name='wwnn'>
+            <ref name='wwn'/>
+          </attribute>
+          <attribute name='wwpn'>
+            <ref name='wwn'/>
+          </attribute>
+        </group>
+      </choice>
       <empty/>
     </element>
   </define>
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 7a39998..ddb2945 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -90,6 +90,10 @@ VIR_ENUM_IMPL(virStoragePartedFsType,
               "ext2", "ext2",
               "extended")
 
+VIR_ENUM_IMPL(virStoragePoolSourceAdapterType,
+              VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_LAST,
+              "default", "scsi_host", "fc_host")
+
 typedef const char *(*virStorageVolFormatToString)(int format);
 typedef int (*virStorageVolFormatFromString)(const char *format);
 
@@ -304,6 +308,18 @@ virStorageVolDefFree(virStorageVolDefPtr def) {
     VIR_FREE(def);
 }
 
+static void
+virStoragePoolSourceAdapterClear(virStoragePoolSourceAdapter adapter)
+{
+    if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+        VIR_FREE(adapter.data.fchost.wwnn);
+        VIR_FREE(adapter.data.fchost.wwpn);
+    } else if (adapter.type ==
+               VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
+        VIR_FREE(adapter.data.name);
+    }
+}
+
 void
 virStoragePoolSourceClear(virStoragePoolSourcePtr source)
 {
@@ -324,7 +340,7 @@ virStoragePoolSourceClear(virStoragePoolSourcePtr source)
     VIR_FREE(source->devices);
     VIR_FREE(source->dir);
     VIR_FREE(source->name);
-    VIR_FREE(source->adapter);
+    virStoragePoolSourceAdapterClear(source->adapter);
     VIR_FREE(source->initiator.iqn);
     VIR_FREE(source->vendor);
     VIR_FREE(source->product);
@@ -489,6 +505,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
     virStoragePoolOptionsPtr options;
     char *name = NULL;
     char *port = NULL;
+    char *adapter_type = NULL;
     int n;
 
     relnode = ctxt->node;
@@ -580,7 +597,45 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
     }
 
     source->dir = virXPathString("string(./dir/@path)", ctxt);
-    source->adapter = virXPathString("string(./adapter/@name)", ctxt);
+
+    if ((adapter_type = virXPathString("string(./adapter/@type)", ctxt))) {
+        if ((source->adapter.type =
+             virStoragePoolSourceAdapterTypeTypeFromString(adapter_type)) <= 0) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("Unknown pool adapter type '%s'"),
+                           adapter_type);
+            goto cleanup;
+        }
+
+        if (source->adapter.type ==
+            VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+            source->adapter.data.fchost.parent =
+                virXPathString("string(./adapter/@parent)", ctxt);
+            source->adapter.data.fchost.wwnn =
+                virXPathString("string(./adapter/@wwnn)", ctxt);
+            source->adapter.data.fchost.wwpn =
+                virXPathString("string(./adapter/@wwpn)", ctxt);
+        } else if (source->adapter.type ==
+                   VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
+            source->adapter.data.name =
+                virXPathString("string(./adapter/@name)", ctxt);
+        }
+    } else {
+        if (virXPathString("string(./adapter/@wwnn)", ctxt) ||
+            virXPathString("string(./adapter/@wwpn)", ctxt)) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("'type' for 'fc_host' adapter must be specified"));
+            goto cleanup;
+        }
+
+        /* To keep back-compact, 'type' is not required to specify
+         * for scsi_host adapter.
+         */
+        if ((source->adapter.data.name =
+             virXPathString("string(./adapter/@name)", ctxt)))
+            source->adapter.type =
+                VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST;
+    }
 
     authType = virXPathString("string(./auth/@type)", ctxt);
     if (authType == NULL) {
@@ -618,6 +673,7 @@ cleanup:
     VIR_FREE(port);
     VIR_FREE(authType);
     VIR_FREE(nodeset);
+    VIR_FREE(adapter_type);
     return ret;
 }
 
@@ -819,11 +875,33 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) {
     }
 
     if (options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) {
-        if (!ret->source.adapter) {
-            virReportError(VIR_ERR_XML_ERROR,
-                           "%s", _("missing storage pool source adapter name"));
+        if (!ret->source.adapter.type) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("missing storage pool source adapter"));
             goto cleanup;
         }
+
+        if (ret->source.adapter.type ==
+            VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+            if (!ret->source.adapter.data.fchost.wwnn ||
+                !ret->source.adapter.data.fchost.wwpn) {
+                virReportError(VIR_ERR_XML_ERROR, "%s",
+                               _("'wwnn' and 'wwpn' must be specified for adapter "
+                                 "type 'fchost'"));
+                goto cleanup;
+            }
+
+            if (!virValidateWWN(ret->source.adapter.data.fchost.wwnn) ||
+                !virValidateWWN(ret->source.adapter.data.fchost.wwpn))
+                goto cleanup;
+        } else if (ret->source.adapter.type ==
+                   VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
+            if (!ret->source.adapter.data.name) {
+                virReportError(VIR_ERR_XML_ERROR,
+                               "%s", _("missing storage pool source adapter name"));
+                goto cleanup;
+            }
+        }
     }
 
     /* If DEVICE is the only source type, then its required */
@@ -953,9 +1031,23 @@ virStoragePoolSourceFormat(virBufferPtr buf,
     if ((options->flags & VIR_STORAGE_POOL_SOURCE_DIR) &&
         src->dir)
         virBufferAsprintf(buf,"    <dir path='%s'/>\n", src->dir);
-    if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) &&
-        src->adapter)
-        virBufferAsprintf(buf,"    <adapter name='%s'/>\n", src->adapter);
+    if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER)) {
+        if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST ||
+            src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST)
+            virBufferAsprintf(buf, "    <adapter type='%s'",
+                              virStoragePoolSourceAdapterTypeTypeToString(src->adapter.type));
+
+        if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+            virBufferEscapeString(buf, " parent='%s'",
+                                  src->adapter.data.fchost.parent);
+            virBufferAsprintf(buf," wwnn='%s' wwpn='%s'/>\n",
+                              src->adapter.data.fchost.wwnn,
+                              src->adapter.data.fchost.wwpn);
+        } else if (src->adapter.type ==
+                 VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
+            virBufferAsprintf(buf," name='%s'/>\n", src->adapter.data.name);
+        }
+    }
     if ((options->flags & VIR_STORAGE_POOL_SOURCE_NAME) &&
         src->name)
         virBufferAsprintf(buf,"    <name>%s</name>\n", src->name);
@@ -1856,8 +1948,19 @@ int virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools,
                 matchpool = pool;
             break;
         case VIR_STORAGE_POOL_SCSI:
-            if (STREQ(pool->def->source.adapter, def->source.adapter))
-                matchpool = pool;
+            if (pool->def->source.adapter.type ==
+                VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+                if (STREQ(pool->def->source.adapter.data.fchost.wwnn,
+                          def->source.adapter.data.fchost.wwnn) &&
+                    STREQ(pool->def->source.adapter.data.fchost.wwpn,
+                          def->source.adapter.data.fchost.wwpn))
+                    matchpool = pool;
+            } else if (pool->def->source.adapter.type ==
+                       VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST){
+                if (STREQ(pool->def->source.adapter.data.name,
+                          def->source.adapter.data.name))
+                    matchpool = pool;
+            }
             break;
         case VIR_STORAGE_POOL_ISCSI:
         {
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index ad16eca..60b8034 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -233,7 +233,28 @@ struct _virStoragePoolSourceDevice {
     } geometry;
 };
 
+enum virStoragePoolSourceAdapterType {
+    VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_DEFAULT = 0,
+    VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST,
+    VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST,
 
+    VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_LAST,
+};
+VIR_ENUM_DECL(virStoragePoolSourceAdapterType)
+
+typedef struct _virStoragePoolSourceAdapter virStoragePoolSourceAdapter;
+struct _virStoragePoolSourceAdapter {
+    int type; /* enum virStoragePoolSourceAdapterType */
+
+    union {
+        char *name;
+        struct {
+            char *parent;
+            char *wwnn;
+            char *wwpn;
+        } fchost;
+    } data;
+};
 
 typedef struct _virStoragePoolSource virStoragePoolSource;
 typedef virStoragePoolSource *virStoragePoolSourcePtr;
@@ -250,7 +271,7 @@ struct _virStoragePoolSource {
     char *dir;
 
     /* Or an adapter */
-    char *adapter;
+    virStoragePoolSourceAdapter adapter;
 
     /* Or a name */
     char *name;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4a84b2b..c0c6b91 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -777,6 +777,8 @@ virStoragePoolObjLock;
 virStoragePoolObjRemove;
 virStoragePoolObjSaveDef;
 virStoragePoolObjUnlock;
+virStoragePoolSourceAdapterTypeTypeFromString;
+virStoragePoolSourceAdapterTypeTypeToString;
 virStoragePoolSourceClear;
 virStoragePoolSourceFindDuplicate;
 virStoragePoolSourceFindDuplicateDevices;
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 74f04ff..a12a430 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -2058,7 +2058,7 @@ phypStorageVolCreateXML(virStoragePoolPtr pool,
     spdef->source.ndevice = 1;
 
     /*XXX source adapter not working properly, should show hdiskX */
-    if ((spdef->source.adapter =
+    if ((spdef->source.adapter.data.name =
          phypGetStoragePoolDevice(pool->conn, pool->name)) == NULL) {
         VIR_ERROR(_("Unable to determine storage pools's source adapter."));
         goto err;
@@ -2280,7 +2280,7 @@ phypVolumeGetXMLDesc(virStorageVolPtr vol, unsigned int flags)
 
     pool.source.ndevice = 1;
 
-    if ((pool.source.adapter =
+    if ((pool.source.adapter.data.name =
          phypGetStoragePoolDevice(sp->conn, sp->name)) == NULL) {
         VIR_ERROR(_("Unable to determine storage sps's source adapter."));
         goto cleanup;
@@ -2520,7 +2520,7 @@ phypBuildStoragePool(virConnectPtr conn, virStoragePoolDefPtr def)
                           managed_system, vios_id);
 
     virBufferAsprintf(&buf, "mksp -f %schild %s", def->name,
-                      source.adapter);
+                      source.adapter.data.name);
 
     if (system_type == HMC)
         virBufferAddChar(&buf, '\'');
@@ -2761,7 +2761,7 @@ phypGetStoragePoolXMLDesc(virStoragePoolPtr pool, unsigned int flags)
     def.source.ndevice = 1;
 
     /*XXX source adapter not working properly, should show hdiskX */
-    if ((def.source.adapter =
+    if ((def.source.adapter.data.name =
          phypGetStoragePoolDevice(pool->conn, pool->name)) == NULL) {
         VIR_ERROR(_("Unable to determine storage pools's source adapter."));
         goto err;
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index 90bbf59..c1c163e 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -639,7 +639,7 @@ virStorageBackendSCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
     char *path;
 
     *isActive = false;
-    if (virAsprintf(&path, "/sys/class/scsi_host/%s", pool->def->source.adapter) < 0) {
+    if (virAsprintf(&path, "/sys/class/scsi_host/%s", pool->def->source.adapter.data.name) < 0) {
         virReportOOMError();
         return -1;
     }
@@ -661,9 +661,9 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
 
     pool->def->allocation = pool->def->capacity = pool->def->available = 0;
 
-    if (sscanf(pool->def->source.adapter, "host%u", &host) != 1) {
+    if (sscanf(pool->def->source.adapter.data.name, "host%u", &host) != 1) {
         VIR_DEBUG("Failed to get host number from '%s'",
-                    pool->def->source.adapter);
+                    pool->def->source.adapter.data.name);
         retval = -1;
         goto out;
     }
diff --git a/tests/storagepoolxml2xmlin/pool-scsi1.xml b/tests/storagepoolxml2xmlin/pool-scsi1.xml
new file mode 100644
index 0000000..1e9826d
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-scsi1.xml
@@ -0,0 +1,15 @@
+<pool type='scsi'>
+  <name>hba0</name>
+  <uuid>e9392370-2917-565e-692b-d057f46512d6</uuid>
+  <source>
+    <adapter type='fc_host' parent='scsi_host5' wwnn='20000000c9831b4b' wwpn='10000000c9831b4b'/>
+  </source>
+  <target>
+    <path>/dev/disk/by-path</path>
+    <permissions>
+      <mode>0700</mode>
+      <owner>0</owner>
+      <group>0</group>
+    </permissions>
+  </target>
+</pool>
diff --git a/tests/storagepoolxml2xmlout/pool-scsi.xml b/tests/storagepoolxml2xmlout/pool-scsi.xml
index 321dc2b..faf5342 100644
--- a/tests/storagepoolxml2xmlout/pool-scsi.xml
+++ b/tests/storagepoolxml2xmlout/pool-scsi.xml
@@ -5,7 +5,7 @@
   <allocation unit='bytes'>0</allocation>
   <available unit='bytes'>0</available>
   <source>
-    <adapter name='host0'/>
+    <adapter type='scsi_host' name='host0'/>
   </source>
   <target>
     <path>/dev/disk/by-path</path>
diff --git a/tests/storagepoolxml2xmlout/pool-scsi1.xml b/tests/storagepoolxml2xmlout/pool-scsi1.xml
new file mode 100644
index 0000000..fb1079f
--- /dev/null
+++ b/tests/storagepoolxml2xmlout/pool-scsi1.xml
@@ -0,0 +1,18 @@
+<pool type='scsi'>
+  <name>hba0</name>
+  <uuid>e9392370-2917-565e-692b-d057f46512d6</uuid>
+  <capacity unit='bytes'>0</capacity>
+  <allocation unit='bytes'>0</allocation>
+  <available unit='bytes'>0</available>
+  <source>
+    <adapter type='fc_host' parent='scsi_host5' wwnn='20000000c9831b4b' wwpn='10000000c9831b4b'/>
+  </source>
+  <target>
+    <path>/dev/disk/by-path</path>
+    <permissions>
+      <mode>0700</mode>
+      <owner>0</owner>
+      <group>0</group>
+    </permissions>
+  </target>
+</pool>
diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c
index 8cac978..c04eb69 100644
--- a/tests/storagepoolxml2xmltest.c
+++ b/tests/storagepoolxml2xmltest.c
@@ -90,6 +90,7 @@ mymain(void)
     DO_TEST("pool-iscsi-auth");
     DO_TEST("pool-netfs");
     DO_TEST("pool-scsi");
+    DO_TEST("pool-scsi1");
     DO_TEST("pool-mpath");
     DO_TEST("pool-iscsi-multiiqn");
     DO_TEST("pool-iscsi-vendor-product");
-- 
1.7.7.6




More information about the libvir-list mailing list