[libvirt] [PATCH 3/8] storage: Introduce parentaddr into virStoragePoolSourceAdapter

From: Osier Yang <jyang(a)redhat.com&gt; 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: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. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- docs/formatstorage.html.in | 128 ++++++++++++++++++--- docs/schemas/basictypes.rng | 24 +++- src/conf/storage_conf.c | 106 +++++++++++++++-- src/conf/storage_conf.h | 4 + .../pool-scsi-type-scsi-host-stable.xml | 19 +++ .../pool-scsi-type-scsi-host-stable.xml | 22 ++++ tests/storagepoolxml2xmltest.c | 1 + 7 files changed, 273 insertions(+), 31 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host-stable.xml create mode 100644 tests/storagepoolxml2xmlout/pool-scsi-type-scsi-host-stable.xml diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 1cd82b4..0e43fd8 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -89,6 +89,24 @@ <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; &lt;adapter type='fc_host' parent='scsi_host5' wwnn='20000000c9831b4b' wwpn='10000000c9831b4b'/&gt; &lt;/source&gt; ...</pre> @@ -111,25 +129,97 @@ <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). It is further recommended to utilize + the <code>parentaddr</code> element since the <code>name</code> + may change between system reboots and kernel reloads if the + hardware configuration is modified. + <span class="since">Since 0.6.2</span> + </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<... + 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 uniquely identify the SCSI host + adapter's parent PCI address. Using a combination of the + <code>unique_id</code> attribute and the <code>address</code> + element, a search will be peformed of the system filesystem for + a match. + <span class="since">Since 1.2.6</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 commands: <code> + find -H /sys/bus/pci/devices/0000:00:1f.2 -name unique_id | + xargs grep '[0-9]'</code> or <code> + grep '[0-9]' /sys/class/scsi_host/host{0..9}/unique_id</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>, diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 34ef613..29eece3 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -355,9 +355,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'> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 2e71c64..31648cb 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -674,14 +674,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); @@ -692,7 +721,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; } @@ -942,9 +978,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; } } @@ -1109,9 +1155,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"); + } } } @@ -2087,6 +2148,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) @@ -2129,9 +2212,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 7a92a47..4659231 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 <libxml/tree.h> @@ -213,6 +214,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 0000000..db13cd0 --- /dev/null +++ b/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> 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 0000000..dd3d87d --- /dev/null +++ b/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> diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c index 971fe3b..d7ae10b 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; } -- 1.9.3