From: Osier Yang Date: Tue, 4 Mar 2014 03:15:13 +0000 (-0500) Subject: storage: Introduce parentaddr into virStoragePoolSourceAdapter X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=a4bd62adc19c2705f21b277b934f90f4796a4bdf;p=libvirt.git 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:
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:bus: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. --- diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 789701c18c..6bbd61a953 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -86,6 +86,24 @@ </source> ... + 
+        ...
+        <source>
+          <adapter type='scsi_host' name='scsi_host1'/>
+        </source>
+        ...
+ + 
+        ...
+        <source>
+          <adapter type='scsi_host'>
+            <parentaddr unique_id='1'>
+              <address domain='0x0000' bus='0x00' slot='0x1f' addr='0x2'/>
+            </parentaddr>
+          </adapter>
+        </source>
+        ...
+ 
         ...
         <source>
@@ -111,25 +129,109 @@
         Since 0.4.1
       
adapter

       
Provides the source for pools backed by SCSI adapters (pool
-        type scsi). May
-        only occur once. Attribute name 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 type (1.0.5)
-        specifies the adapter type.  Valid values are "fc_host" and "scsi_host".
-        If omitted and the name attribute is specified, then it
-        defaults to "scsi_host". To keep backwards compatibility, the attribute
-        type is optional for the "scsi_host" adapter, but
-        mandatory for the "fc_host" adapter.  Attributes wwnn
-        (Word Wide Node Name) and wwpn (Word Wide Port Name)
-        (1.0.4) 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 parent
-        (1.0.4) specifies the parent device for
-        the "fc_host" adapter.
-        Since 0.6.2

+        type scsi). May only occur once.
+        

+          
name

+          
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 virsh nodedev-list
+            scsi_host command followed by a combination of
+            lspci and virsh nodedev-dumpxml
+            scsi_hostN commands to find the scsi_hostN
+            to be used. Since 0.6.2
+            

+            It is further recommended to utilize the
+            parentaddr element since it's possible to have
+            the path to which the scsi_hostN uses change between system
+            reboots. Since 1.2.7
+            

+          

+        

+        

+          
type

+          
Specifies the adapter type. Valid values are "scsi_host" or
+            "fc_host". If omitted and the name attribute is
+            specified, then it defaults to "scsi_host". To keep backwards
+            compatibility, this attribute is optional only for the
+            "scsi_host" adapter, but is mandatory for the "fc_host" adapter.
+            Since 1.0.5
+            

+        

+        

+          
wwwn and wwpn

+          
The "World Wide Node Name" (wwnn) and "World Wide
+            Port Name" (wwpn) 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.
+            Since 1.0.4
+          

+        

+        

+          
parent

+          
Used by the "fc_host" adapter type to optionally specify the
+            parent scsi_host device defined in the
+            Node Device database as the
+            NPIV
+            virtual Host Bus Adapter (vHBA).
+            Since 1.0.4
+          

+        

+        

+          
parentaddr

+          
Used by the "scsi_host" adapter type instead of the
+            name attribute to more uniquely identify the
+            SCSI host. Using a combination of the unique_id
+            attribute and the address element to formulate
+            a PCI address, a search will be performed of the
+            /sys/class/scsi_host/hostNN links for a
+            matching PCI address with a matching unique_id
+            value in the /sys/class/scsi_host/hostNN/unique_id
+            file. The value in the "unique_id" file will be unique enough
+            for the specific PCI address. The hostNN will be
+            used by libvirt as the basis to define which SCSI host is to
+            be used for the currently booted system.
+            Since 1.2.7
+            

+              
address

+              
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:
+                domain (a 2-byte hex integer, not currently used
+                by qemu), bus (a hex value between 0 and 0xff,
+                inclusive), slot (a hex value between 0x0 and
+                0x1f, inclusive), and function (a value between
+                0 and 7, inclusive). The PCI address can be determined by
+                listing the /sys/bus/pci/devices and the
+                /sys/class/scsi_host 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
+                path element as the basis to formulate the
+                correctly formatted PCI address.
+              

+            

+            

+              
unique_id

+              
Required parentaddr 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
+                unique_id 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
+                find -H /sys/class/scsi_host/host*/unique_id |
+                xargs grep '[0-9]'.
+              

+            

+          

+        

+      
       
host

       
Provides the source for pools backed by storage from a
         remote server (pool types netfs, iscsi,
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 6d357b7340..75d5238f8e 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -361,9 +361,27 @@
               scsi_host
             
           
-          
-            
-          
+          
+            
+              
+                
+              
+            
+            
+              
+                
+                  
+                    
+                      
+                    
+                  
+                  
+                    
+                  
+                
+              
+            
+          
         
         
           
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index e135512789..e33eb1bbae 100644
--- a/src/conf/storage_conf.c
+++ b/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, "\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, "\n");
+                virBufferAdjustIndent(buf, -2);
+                virBufferAddLit(buf, "\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:
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 33784b2f31..ecd0dfe509 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -29,6 +29,7 @@
 # include "virstoragefile.h"
 # include "virbitmap.h"
 # include "virthread.h"
+# include "device_conf.h"
 
 # include 
 
@@ -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;
diff --git a/tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host-stable.xml b/tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host-stable.xml
new file mode 100644
index 0000000000..db13cd05a9
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host-stable.xml
@@ -0,0 +1,19 @@
+
+  hba0
+  e9392370-2917-565e-692b-d057f46512d6
+  
+    
+      
+        

+      
+    
+  
+  
+    /dev/disk/by-path
+    
+      0700
+      0
+      0
+    
+  
+
diff --git a/tests/storagepoolxml2xmlout/pool-scsi-type-scsi-host-stable.xml b/tests/storagepoolxml2xmlout/pool-scsi-type-scsi-host-stable.xml
new file mode 100644
index 0000000000..dd3d87db5c
--- /dev/null
+++ b/tests/storagepoolxml2xmlout/pool-scsi-type-scsi-host-stable.xml
@@ -0,0 +1,22 @@
+
+  hba0
+  e9392370-2917-565e-692b-d057f46512d6
+  0
+  0
+  0
+  
+    
+      
+        

+      
+    
+  
+  
+    /dev/disk/by-path
+    
+      0700
+      0
+      0
+    
+  
+
diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c
index 971fe3bb02..d7ae10b41b 100644
--- a/tests/storagepoolxml2xmltest.c
+++ b/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;
 }