storage: Introduce parentaddr into virStoragePoolSourceAdapter

Between reboots and kernel reloads, the SCSI host number used for SCSI
storage pools may change requiring modification to the storage pool XML
in order to use a specific SCSI host adapter.

This patch introduces the "parentaddr" element and "unique_id" attribute
for the SCSI host adapter in order to uniquely identify the adapter
between reboots and kernel reloads. For now the goal is to only parse
and format the XML. Both will be required to be provided in order to
uniquely identify the desired SCSI host.

The new XML is expected to be as follows:

  <adapter type='scsi_host'>
    <parentaddr unique_id='3'>
      <address domain='0x0000' bus='0x00' slot='0x1f' func='0x2'/>
    </parentaddr>
  </adapter>

where "parentaddr" is the parent device of the SCSI host using the PCI
address on which the device resides and the value from the unique_id file
for the device. Both the PCI address and unique_id values will be used
to traverse the /sys/class/scsi_host/ directories looking at each link
to match the PCI address reformatted to the directory link format where
"domain🚌slot:function" is found.  Then for each matching directory
the unique_id file for the scsi_host will be used to match the unique_id
value in the xml.

For a PCI address listed above, this will be formatted to "0000:00:1f.2"
and the links in /sys/class/scsi_host will be used to find the host#
to be used for the 'scsi_host' device. Each entry is a link to the
/sys/bus/pci/devices directories, e.g.:

%  ls -al /sys/class/scsi_host/host2
lrwxrwxrwx. 1 root root 0 Jun  1 00:22 /sys/class/scsi_host/host2 -> ../../devices/pci0000:00/0000:00:1f.2/ata3/host2/scsi_host/host2

% cat /sys/class/scsi_host/host2/unique_id
3

The "parentaddr" and "name" attributes are mutually exclusive to identify
the SCSI host number. Use of the "parentaddr" element will be the preferred
mechanism.

This patch only supports to parse and format the XMLs. Later patches will
add code to find out the scsi host number.

docs/formatstorage.html.in

@@ -86,6 +86,24 @@
         </source>
         ...</pre>
 
+    <pre>
+        ...
+        &lt;source&gt;
+          &lt;adapter type='scsi_host' name='scsi_host1'/&gt;
+        &lt;/source&gt;
+        ...</pre>
+
+    <pre>
+        ...
+        &lt;source&gt;
+          &lt;adapter type='scsi_host'&gt;
+            &lt;parentaddr unique_id='1'&gt;
+              &lt;address domain='0x0000' bus='0x00' slot='0x1f' addr='0x2'/&gt;
+            &lt;/parentaddr&gt;
+          &lt;/adapter&gt;
+        &lt;/source&gt;
+        ...</pre>
+
     <pre>
         ...
         &lt;source&gt;
@@ -111,25 +129,109 @@
         <span class="since">Since 0.4.1</span></dd>
       <dt><code>adapter</code></dt>
       <dd>Provides the source for pools backed by SCSI adapters (pool
-        type <code>scsi</code>). May
-        only occur once. Attribute <code>name</code> is the SCSI adapter
-        name (ex. "scsi_host1".  NB, although a name such as "host1" is
-        still supported for backwards compatibility, it is not recommended).
-        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>
+        type <code>scsi</code>). May only occur once.
+        <dl>
+          <dt><code>name</code></dt>
+          <dd>The SCSI adapter name (e.g. "scsi_host1", although a name
+            such as "host1" is still supported for backwards compatibility,
+            it is not recommended). The scsi_host name to be used can be
+            determined from the output of a <code>virsh nodedev-list
+            scsi_host</code> command followed by a combination of
+            <code>lspci</code> and <code>virsh nodedev-dumpxml
+            scsi_hostN</code> commands to find the <code>scsi_hostN</code>
+            to be used. <span class="since">Since 0.6.2</span>
+            <p>
+            It is further recommended to utilize the
+            <code>parentaddr</code> element since it's possible to have
+            the path to which the scsi_hostN uses change between system
+            reboots. <span class="since">Since 1.2.7</span>
+            </p>
+          </dd>
+        </dl>
+        <dl>
+          <dt><code>type</code></dt>
+          <dd>Specifies the adapter type. Valid values are "scsi_host" or
+            "fc_host". If omitted and the <code>name</code> attribute is
+            specified, then it defaults to "scsi_host". To keep backwards
+            compatibility, this attribute is optional <b>only</b> for the
+            "scsi_host" adapter, but is mandatory for the "fc_host" adapter.
+            <span class="since">Since 1.0.5</span>
+            </dd>
+        </dl>
+        <dl>
+          <dt><code>wwwn</code> and <code>wwpn</code></dt>
+          <dd>The "World Wide Node Name" (<code>wwnn</code>) and "World Wide
+            Port Name" (<code>wwpn</code>) 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. Use the command 'virsh nodedev-dumpxml' to determine
+            how to set the values for the wwnn/wwpn of a (v)HBA.
+            <span class="since">Since 1.0.4</span>
+          </dd>
+        </dl>
+        <dl>
+          <dt><code>parent</code></dt>
+          <dd>Used by the "fc_host" adapter type to optionally specify the
+            parent scsi_host device defined in the
+            <a href="formatnode.html">Node Device</a> database as the
+            <a href="http://wiki.libvirt.org/page/NPIV_in_libvirt">NPIV</a>
+            virtual Host Bus Adapter (vHBA).
+            <span class="since">Since 1.0.4</span>
+          </dd>
+        </dl>
+        <dl>
+          <dt><code>parentaddr</code></dt>
+          <dd>Used by the "scsi_host" adapter type instead of the
+            <code>name</code> attribute to more uniquely identify the
+            SCSI host. Using a combination of the <code>unique_id</code>
+            attribute and the <code>address</code> element to formulate
+            a PCI address, a search will be performed of the
+            <code>/sys/class/scsi_host/hostNN</code> links for a
+            matching PCI address with a matching <code>unique_id</code>
+            value in the <code>/sys/class/scsi_host/hostNN/unique_id</code>
+            file. The value in the "unique_id" file will be unique enough
+            for the specific PCI address. The <code>hostNN</code> will be
+            used by libvirt as the basis to define which SCSI host is to
+            be used for the currently booted system.
+            <span class="since">Since 1.2.7</span>
+            <dl>
+              <dt><code>address</code></dt>
+              <dd>The PCI address of the scsi_host device to be used. Using
+                a PCI address provides consistent naming across system reboots
+                and kernel reloads. The address will have four attributes:
+                <code>domain</code> (a 2-byte hex integer, not currently used
+                by qemu), <code>bus</code> (a hex value between 0 and 0xff,
+                inclusive), <code>slot</code> (a hex value between 0x0 and
+                0x1f, inclusive), and <code>function</code> (a value between
+                0 and 7, inclusive). The PCI address can be determined by
+                listing the <code>/sys/bus/pci/devices</code> and the
+                <code>/sys/class/scsi_host</code> directories in order to
+                find the expected scsi_host device. The address will be
+                provided in a format such as "0000:00:1f:2" which can be
+                used to generate the expected PCI address
+                "domain='0x0000' bus='0x00' slot='0x1f' function='0x0'".
+                Optionally, using the combination of the commands 'virsh
+                nodedev-list scsi_host' and 'virsh nodedev-dumpxml' for a
+                specific list entry and converting the resulting
+                <code>path</code> element as the basis to formulate the
+                correctly formatted PCI address.
+              </dd>
+            </dl>
+            <dl>
+              <dt><code>unique_id</code></dt>
+              <dd>Required <code>parentaddr</code> attribute used to determine
+                which of the scsi_host adapters for the provided PCI address
+                should be used. The value is determine by contents of the
+                <code>unique_id</code> file for the specific scsi_host adapter.
+                For a PCI address of "0000:00:1f:2", the unique identifer files
+                can be found using the command
+                <code>find -H /sys/class/scsi_host/host*/unique_id |
+                xargs grep '[0-9]'</code>.
+              </dd>
+            </dl>
+          </dd>
+        </dl>
+      </dd>
       <dt><code>host</code></dt>
       <dd>Provides the source for pools backed by storage from a
         remote server (pool types <code>netfs</code>, <code>iscsi</code>,

docs/schemas/basictypes.rng

@@ -361,9 +361,27 @@
               <value>scsi_host</value>
             </attribute>
           </optional>
-          <attribute name='name'>
-            <text/>
-          </attribute>
+          <choice>
+            <group>
+              <attribute name='name'>
+                <text/>
+              </attribute>
+            </group>
+            <group>
+              <interleave>
+                <element name="parentaddr">
+                  <optional>
+                    <attribute name='unique_id'>
+                      <ref name='positiveInteger'/>
+                    </attribute>
+                  </optional>
+                  <element name="address">
+                    <ref name="pciaddress"/>
+                  </element>
+                </element>
+              </interleave>
+            </group>
+          </choice>
         </group>
         <group>
           <attribute name='type'>

src/conf/storage_conf.c

@@ -576,14 +576,43 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
                 virXPathString("string(./adapter/@wwpn)", ctxt);
         } else if (source->adapter.type ==
                    VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
+
             source->adapter.data.scsi_host.name =
                 virXPathString("string(./adapter/@name)", ctxt);
+            if (virXPathNode("./adapter/parentaddr", ctxt)) {
+                xmlNodePtr addrnode = virXPathNode("./adapter/parentaddr/address",
+                                                   ctxt);
+                virDevicePCIAddressPtr 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 (virDevicePCIAddressParseXML(addrnode, addr) < 0)
+                    goto cleanup;
+                if ((virXPathInt("string(./adapter/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 = NULL;
         char *wwpn = NULL;
         char *parent = NULL;
 
+        /* "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".
+         */
         wwnn = virXPathString("string(./adapter/@wwnn)", ctxt);
         wwpn = virXPathString("string(./adapter/@wwpn)", ctxt);
         parent = virXPathString("string(./adapter/@parent)", ctxt);
@@ -594,7 +623,14 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
             VIR_FREE(parent);
             virReportError(VIR_ERR_XML_ERROR, "%s",
                            _("Use of 'wwnn', 'wwpn', and 'parent' attributes "
-                             "requires the 'fc_host' adapter 'type'"));
+                             "requires use of the adapter 'type'"));
+            goto cleanup;
+        }
+
+        if (virXPathNode("./adapter/parentaddr", ctxt)) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("Use of 'parent' element requires use "
+                             "of the adapter 'type'"));
             goto cleanup;
         }
 
@@ -854,9 +890,19 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
                 goto error;
         } else if (ret->source.adapter.type ==
                    VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
-            if (!ret->source.adapter.data.scsi_host.name) {
+            if (!ret->source.adapter.data.scsi_host.name &&
+                !ret->source.adapter.data.scsi_host.has_parent) {
                 virReportError(VIR_ERR_XML_ERROR, "%s",
-                               _("missing storage pool source adapter name"));
+                               _("Either 'name' or 'parent' must be specified "
+                                 "for the 'scsi_host' adapter"));
+                goto error;
+            }
+
+            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"));
                 goto error;
             }
         }
@@ -1020,9 +1066,24 @@ virStoragePoolSourceFormat(virBufferPtr buf,
                               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.scsi_host.name);
+                   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 {
+                virDevicePCIAddress 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(virDevicePCIAddressFormat(buf, addr, false));
+                virBufferAdjustIndent(buf, -2);
+                virBufferAddLit(buf, "</parentaddr>\n");
+                virBufferAdjustIndent(buf, -2);
+                virBufferAddLit(buf, "</adapter>\n");
+            }
         }
     }
 
@@ -1987,6 +2048,28 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools,
     return ret;
 }
 
+static bool
+matchSCSIAdapterParent(virStoragePoolObjPtr pool,
+                       virStoragePoolDefPtr def)
+{
+    virDevicePCIAddressPtr pooladdr =
+        &pool->def->source.adapter.data.scsi_host.parentaddr;
+    virDevicePCIAddressPtr defaddr =
+        &def->source.adapter.data.scsi_host.parentaddr;
+    int pool_unique_id =
+        pool->def->source.adapter.data.scsi_host.unique_id;
+    int def_unique_id =
+        def->source.adapter.data.scsi_host.unique_id;
+    if (pooladdr->domain == defaddr->domain &&
+        pooladdr->bus == defaddr->bus &&
+        pooladdr->slot == defaddr->slot &&
+        pooladdr->function == defaddr->function &&
+        pool_unique_id == def_unique_id) {
+        return true;
+    }
+    return false;
+}
+
 int
 virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools,
                                   virStoragePoolDefPtr def)
@@ -2029,9 +2112,14 @@ virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools,
                     matchpool = pool;
             } else if (pool->def->source.adapter.type ==
                        VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST){
-                if (STREQ(pool->def->source.adapter.data.scsi_host.name,
-                          def->source.adapter.data.scsi_host.name))
-                    matchpool = pool;
+                if (pool->def->source.adapter.data.scsi_host.name) {
+                    if (STREQ(pool->def->source.adapter.data.scsi_host.name,
+                              def->source.adapter.data.scsi_host.name))
+                        matchpool = pool;
+                } else {
+                    if (matchSCSIAdapterParent(pool, def))
+                        matchpool = pool;
+                }
             }
             break;
         case VIR_STORAGE_POOL_ISCSI:

src/conf/storage_conf.h

@@ -29,6 +29,7 @@
 # include "virstoragefile.h"
 # include "virbitmap.h"
 # include "virthread.h"
+# include "device_conf.h"
 
 # include <libxml/tree.h>
 
@@ -181,6 +182,9 @@ struct _virStoragePoolSourceAdapter {
     union {
         struct {
             char *name;
+            virDevicePCIAddress parentaddr; /* host address */
+            int unique_id;
+            bool has_parent;
         } scsi_host;
         struct {
             char *parent;

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

@@ -0,0 +1,19 @@
+<pool type="scsi">
+  <name>hba0</name>
+  <uuid>e9392370-2917-565e-692b-d057f46512d6</uuid>
+  <source>
+    <adapter type='scsi_host'>
+      <parentaddr unique_id='5'>
+        <address domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
+      </parentaddr>
+    </adapter>
+  </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-stable.xml

@@ -0,0 +1,22 @@
+<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'>
+      <parentaddr unique_id='5'>
+        <address domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
+      </parentaddr>
+    </adapter>
+  </source>
+  <target>
+    <path>/dev/disk/by-path</path>
+    <permissions>
+      <mode>0700</mode>
+      <owner>0</owner>
+      <group>0</group>
+    </permissions>
+  </target>
+</pool>

tests/storagepoolxml2xmltest.c

@@ -104,6 +104,7 @@ mymain(void)
     DO_TEST("pool-sheepdog");
     DO_TEST("pool-gluster");
     DO_TEST("pool-gluster-sub");
+    DO_TEST("pool-scsi-type-scsi-host-stable");
 
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }