New XML attributes for storage pool source adapter

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-compat reason. However, mandatory
for 'fc_host' adapter and any new future adapter types. Attribute
'parent' is to specify the parent for the fc_host adapter.

* 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. Later patch will
    add the checking, as "adapter.data.name" is only valid for "scsi_host"
    adapter.
* src/storage/storage_backend_scsi.c:
  - Like above
* tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host.xml:
* tests/storagepoolxml2xmlin/pool-scsi-type-fc-host.xml:
  - New test for 'fc_host' and "scsi_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-scsi-type-scsi-host.xml:
* tests/storagepoolxml2xmlout/pool-scsi-type-fc-host.xml:
  - New test
* tests/storagepoolxml2xmltest.c:
  - Include the test

docs/formatstorage.html.in

@@ -75,6 +75,14 @@
         </source>
         ...</pre>
 
+    <pre>
+        ...
+        &lt;source&gt;
+        &lt;source&gt;
+          &lt;adapter type='fc_host' parent='scsi_host5' wwnn='20000000c9831b4b' wwpn='10000000c9831b4b'/&gt;
+        &lt;/source&gt;
+        ...</pre>
+
     <dl>
       <dt><code>device</code></dt>
       <dd>Provides the source for pools backed by physical devices.
@@ -88,8 +96,22 @@
         <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>
+        only occur once. Attribute <code>name</code> is the SCSI adapter
-        which is the SCSI adapter name (ex. "host1").
+        name (ex. "host1"). Attribute <code>type</code>
+        (<span class="since">1.0.5</span>) specifies the adapter type.
+        Valid values are "fc_host" and "scsi_host".  If omitted and
+        the <code>name</code> attribute is specified, then it defaults to
+        "scsi_host". To keep backwards compatibility, the attribute
+        <code>type</code> is optional for the "scsi_host" adapter, but
+        mandatory for the "fc_host" adapter.  Attributes <code>wwnn</code>
+        (Word Wide Node Name) and <code>wwpn</code> (Word Wide Port Name)
+        (<span class="since">1.0.4</span>) are used by the "fc_host" adapter
+        to uniquely identify the device in the Fibre Channel storage fabric
+        (the device can be either a HBA or vHBA). Both wwnn and wwpn should
+        be specified (See command 'virsh nodedev-dumpxml' to known how to get
+        wwnn/wwpn of a (v)HBA). The optional attribute <code>parent</code>
+        (<span class="since">1.0.4</span>) specifies the parent device for
+        the "fc_host" adapter.
         <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

docs/schemas/storagepool.rng

@@ -276,9 +276,36 @@
 
   <define name='sourceinfoadapter'>
     <element name='adapter'>
-      <attribute name='name'>
+      <choice>
-        <text/>
+        <group>
-      </attribute>
+          <!-- To keep back-compat, '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>

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,19 @@ 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);
+        VIR_FREE(adapter.data.fchost.parent);
+    } else if (adapter.type ==
+               VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
+        VIR_FREE(adapter.data.name);
+    }
+}
+
 void
 virStoragePoolSourceClear(virStoragePoolSourcePtr source)
 {
@@ -324,7 +341,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 +506,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
     virStoragePoolOptionsPtr options;
     char *name = NULL;
     char *port = NULL;
+    char *adapter_type = NULL;
     int n;
 
     relnode = ctxt->node;
@@ -580,7 +598,56 @@ 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 {
+        char *wwnn = NULL;
+        char *wwpn = NULL;
+        char *parent = NULL;
+
+        wwnn = virXPathString("string(./adapter/@wwnn)", ctxt);
+        wwpn = virXPathString("string(./adapter/@wwpn)", ctxt);
+        parent = virXPathString("string(./adapter/@parent)", ctxt);
+
+        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 the 'fc_host' adapter 'type'"));
+            goto cleanup;
+        }
+
+        /* To keep back-compat, '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 +685,7 @@ cleanup:
     VIR_FREE(port);
     VIR_FREE(authType);
     VIR_FREE(nodeset);
+    VIR_FREE(adapter_type);
     return ret;
 }
 
@@ -819,11 +887,33 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) {
     }
 
     if (options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) {
-        if (!ret->source.adapter) {
+        if (!ret->source.adapter.type) {
-            virReportError(VIR_ERR_XML_ERROR,
+            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           "%s", _("missing storage pool source adapter name"));
+                           _("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 +1043,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) &&
+    if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER)) {
-        src->adapter)
+        if (src->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST ||
-        virBufferAsprintf(buf,"    <adapter name='%s'/>\n", src->adapter);
+            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 +1960,19 @@ int virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools,
                 matchpool = pool;
             break;
         case VIR_STORAGE_POOL_SCSI:
-            if (STREQ(pool->def->source.adapter, def->source.adapter))
+            if (pool->def->source.adapter.type ==
-                matchpool = pool;
+                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:
         {

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;

src/libvirt_private.syms

@@ -615,6 +615,8 @@ virStoragePoolObjLock;
 virStoragePoolObjRemove;
 virStoragePoolObjSaveDef;
 virStoragePoolObjUnlock;
+virStoragePoolSourceAdapterTypeTypeFromString;
+virStoragePoolSourceAdapterTypeTypeToString;
 virStoragePoolSourceClear;
 virStoragePoolSourceFindDuplicate;
 virStoragePoolSourceFindDuplicateDevices;

src/phyp/phyp_driver.c

@@ -2049,7 +2049,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;
@@ -2271,7 +2271,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;
@@ -2511,7 +2511,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, '\'');
@@ -2752,7 +2752,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;

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;
     }

tests/storagepoolxml2xmlin/pool-scsi-type-fc-host.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>

tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host.xml

@@ -0,0 +1,15 @@
+<pool type="scsi">
+  <name>hba0</name>
+  <uuid>e9392370-2917-565e-692b-d057f46512d6</uuid>
+  <source>
+    <adapter type='scsi_host' name="host0"/>
+  </source>
+  <target>
+    <path>/dev/disk/by-path</path>
+    <permissions>
+      <mode>0700</mode>
+      <owner>0</owner>
+      <group>0</group>
+    </permissions>
+  </target>
+</pool>

tests/storagepoolxml2xmlout/pool-scsi-type-fc-host.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>

tests/storagepoolxml2xmlout/pool-scsi-type-scsi-host.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='scsi_host' name='host0'/>
+  </source>
+  <target>
+    <path>/dev/disk/by-path</path>
+    <permissions>
+      <mode>0700</mode>
+      <owner>0</owner>
+      <group>0</group>
+    </permissions>
+  </target>
+</pool>

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>

tests/storagepoolxml2xmltest.c

@@ -90,6 +90,8 @@ mymain(void)
     DO_TEST("pool-iscsi-auth");
     DO_TEST("pool-netfs");
     DO_TEST("pool-scsi");
+    DO_TEST("pool-scsi-type-scsi-host");
+    DO_TEST("pool-scsi-type-fc-host");
     DO_TEST("pool-mpath");
     DO_TEST("pool-iscsi-multiiqn");
     DO_TEST("pool-iscsi-vendor-product");