[libvirt] [REPOST 0/8] storage_scsi: Stable SCSI host addressing
Reposting of a series from last month changing only the span version in the docs from 1.2.6 to 1.2.7. Previous posting here: http://www.redhat.com/archives/libvir-list/2014-June/msg00448.html The concept still remains the same - rather than rely on the hostNN numbers for the scsi_host to remain stable and unique across host reboots and/or kernel rebuilds, allow use a combination of the scsi_host's PCI address and the value from the hostNN's 'unique_id' file. John Ferlan (6): getAdapterName: check for SCSI_HOST scsi_backend: Use existing LINUX_SYSFS_SCSI_HOST_PREFIX definition virutil: Introduce virReadSCSIUniqueId Add unique_id to nodedev output scsi_host: Introduce virFindSCSIHostByPCI getAdapterName: Lookup stable scsi_host Osier Yang (2): virStoragePoolSourceAdapter: Refine the SCSI_HOST adapter name storage: Introduce parentaddr into virStoragePoolSourceAdapter docs/formatnode.html.in | 11 + docs/formatstorage.html.in | 143 ++++++++-- docs/schemas/basictypes.rng | 24 +- docs/schemas/nodedev.rng | 6 + src/conf/node_device_conf.c | 23 +- src/conf/node_device_conf.h | 1 + src/conf/storage_conf.c | 111 +++++++- src/conf/storage_conf.h | 8 +- src/libvirt_private.syms | 2 + src/node_device/node_device_linux_sysfs.c | 6 + src/phyp/phyp_driver.c | 8 +- src/storage/storage_backend_scsi.c | 53 +++- src/test/test_driver.c | 5 +- src/util/virutil.c | 154 +++++++++++ src/util/virutil.h | 8 + tests/Makefile.am | 7 + .../pci_8086_27c5_scsi_host_0_unique_id.xml | 8 + tests/nodedevxml2xmltest.c | 1 + tests/scsihosttest.c | 308 +++++++++++++++++++++ .../pool-scsi-type-scsi-host-stable.xml | 19 ++ .../pool-scsi-type-scsi-host-stable.xml | 22 ++ tests/storagepoolxml2xmltest.c | 1 + 22 files changed, 868 insertions(+), 61 deletions(-) create mode 100644 tests/nodedevschemadata/pci_8086_27c5_scsi_host_0_unique_id.xml create mode 100644 tests/scsihosttest.c create mode 100644 tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host-stable.xml create mode 100644 tests/storagepoolxml2xmlout/pool-scsi-type-scsi-host-stable.xml -- 1.9.3
Rather than assume that NOT FC_HOST is SCSI_HOST, let's call them out specifically. Makes it easier to find SCSI_HOST code/structs and ensures something isn't missed in the future Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_scsi.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index f6f3ca2..21c13e5 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -547,18 +547,17 @@ getAdapterName(virStoragePoolSourceAdapter adapter) { char *name = NULL; - if (adapter.type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { ignore_value(VIR_STRDUP(name, adapter.data.name)); - return name; - } - - if (!(name = virGetFCHostNameByWWN(NULL, - adapter.data.fchost.wwnn, - adapter.data.fchost.wwpn))) { - virReportError(VIR_ERR_XML_ERROR, - _("Failed to find SCSI host with wwnn='%s', " - "wwpn='%s'"), adapter.data.fchost.wwnn, - adapter.data.fchost.wwpn); + } else if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + if (!(name = virGetFCHostNameByWWN(NULL, + adapter.data.fchost.wwnn, + adapter.data.fchost.wwpn))) { + virReportError(VIR_ERR_XML_ERROR, + _("Failed to find SCSI host with wwnn='%s', " + "wwpn='%s'"), adapter.data.fchost.wwnn, + adapter.data.fchost.wwpn); + } } return name; -- 1.9.3
On 08.07.2014 13:54, John Ferlan wrote:
Rather than assume that NOT FC_HOST is SCSI_HOST, let's call them out specifically. Makes it easier to find SCSI_HOST code/structs and ensures something isn't missed in the future
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_scsi.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index f6f3ca2..21c13e5 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -547,18 +547,17 @@ getAdapterName(virStoragePoolSourceAdapter adapter) { char *name = NULL;
- if (adapter.type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { ignore_value(VIR_STRDUP(name, adapter.data.name)); - return name; - } - - if (!(name = virGetFCHostNameByWWN(NULL, - adapter.data.fchost.wwnn, - adapter.data.fchost.wwpn))) { - virReportError(VIR_ERR_XML_ERROR, - _("Failed to find SCSI host with wwnn='%s', " - "wwpn='%s'"), adapter.data.fchost.wwnn, - adapter.data.fchost.wwpn); + } else if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { + if (!(name = virGetFCHostNameByWWN(NULL, + adapter.data.fchost.wwnn, + adapter.data.fchost.wwpn))) { + virReportError(VIR_ERR_XML_ERROR, + _("Failed to find SCSI host with wwnn='%s', " + "wwpn='%s'"), adapter.data.fchost.wwnn, + adapter.data.fchost.wwpn); + } }
return name;
Or even better use switch((virStoragePoolSourceAdapterType) adapter.type) {}. But I can live with this version too. Michal
From: Osier Yang <jyang@redhat.com> Preparation for future patches by creating a scsi_host union. For now, just the 'name' will be present. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/storage_conf.c | 15 ++++++++------- src/conf/storage_conf.h | 4 +++- src/phyp/phyp_driver.c | 8 ++++---- src/storage/storage_backend_scsi.c | 2 +- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 9ac5975..ef47dd5 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -344,7 +344,7 @@ virStoragePoolSourceAdapterClear(virStoragePoolSourceAdapter adapter) VIR_FREE(adapter.data.fchost.parent); } else if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { - VIR_FREE(adapter.data.name); + VIR_FREE(adapter.data.scsi_host.name); } } @@ -577,7 +577,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, virXPathString("string(./adapter/@wwpn)", ctxt); } else if (source->adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { - source->adapter.data.name = + source->adapter.data.scsi_host.name = virXPathString("string(./adapter/@name)", ctxt); } } else { @@ -602,7 +602,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, /* To keep back-compat, 'type' is not required to specify * for scsi_host adapter. */ - if ((source->adapter.data.name = + if ((source->adapter.data.scsi_host.name = virXPathString("string(./adapter/@name)", ctxt))) source->adapter.type = VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST; @@ -855,7 +855,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) goto error; } else if (ret->source.adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { - if (!ret->source.adapter.data.name) { + if (!ret->source.adapter.data.scsi_host.name) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing storage pool source adapter name")); goto error; @@ -1022,7 +1022,8 @@ virStoragePoolSourceFormat(virBufferPtr buf, 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); + virBufferAsprintf(buf, " name='%s'/>\n", + src->adapter.data.scsi_host.name); } } @@ -2014,8 +2015,8 @@ 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.name, - def->source.adapter.data.name)) + if (STREQ(pool->def->source.adapter.data.scsi_host.name, + def->source.adapter.data.scsi_host.name)) matchpool = pool; } break; diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 47f769b..31240ca 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -180,7 +180,9 @@ struct _virStoragePoolSourceAdapter { int type; /* virStoragePoolSourceAdapterType */ union { - char *name; + struct { + char *name; + } scsi_host; struct { char *parent; char *wwnn; diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 056d289..ad9f054 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1975,7 +1975,7 @@ phypStorageVolCreateXML(virStoragePoolPtr pool, spdef->source.ndevice = 1; /*XXX source adapter not working properly, should show hdiskX */ - if ((spdef->source.adapter.data.name = + if ((spdef->source.adapter.data.scsi_host.name = phypGetStoragePoolDevice(pool->conn, pool->name)) == NULL) { VIR_ERROR(_("Unable to determine storage pools's source adapter.")); goto err; @@ -2197,7 +2197,7 @@ phypStorageVolGetXMLDesc(virStorageVolPtr vol, unsigned int flags) pool.source.ndevice = 1; - if ((pool.source.adapter.data.name = + if ((pool.source.adapter.data.scsi_host.name = phypGetStoragePoolDevice(sp->conn, sp->name)) == NULL) { VIR_ERROR(_("Unable to determine storage sps's source adapter.")); goto cleanup; @@ -2436,7 +2436,7 @@ phypBuildStoragePool(virConnectPtr conn, virStoragePoolDefPtr def) managed_system, vios_id); virBufferAsprintf(&buf, "mksp -f %schild %s", def->name, - source.adapter.data.name); + source.adapter.data.scsi_host.name); if (system_type == HMC) virBufferAddChar(&buf, '\''); @@ -2667,7 +2667,7 @@ phypStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned int flags) def.source.ndevice = 1; /*XXX source adapter not working properly, should show hdiskX */ - if ((def.source.adapter.data.name = + if ((def.source.adapter.data.scsi_host.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 21c13e5..b5dbe51 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -548,7 +548,7 @@ getAdapterName(virStoragePoolSourceAdapter adapter) char *name = NULL; if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { - ignore_value(VIR_STRDUP(name, adapter.data.name)); + ignore_value(VIR_STRDUP(name, adapter.data.scsi_host.name)); } else if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { if (!(name = virGetFCHostNameByWWN(NULL, adapter.data.fchost.wwnn, -- 1.9.3
From: Osier Yang <jyang@redhat.com> 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@redhat.com> --- docs/formatstorage.html.in | 140 ++++++++++++++++++--- 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, 285 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..d0c111e 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -89,6 +89,24 @@ <pre> ... <source> + <adapter type='scsi_host' name='scsi_host1'/> + </source> + ...</pre> + + <pre> + ... + <source> + <adapter type='scsi_host'> + <parentaddr unique_id='1'> + <address domain='0x0000' bus='0x00' slot='0x1f' addr='0x2'/> + </parentaddr> + </adapter> + </source> + ...</pre> + + <pre> + ... + <source> <adapter type='fc_host' parent='scsi_host5' wwnn='20000000c9831b4b' wwpn='10000000c9831b4b'/> </source> ...</pre> @@ -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>, diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 5fe3a97..222dac3 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 ef47dd5..df6fa9c 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -577,14 +577,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); @@ -595,7 +624,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; } @@ -855,9 +891,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; } } @@ -1021,9 +1067,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"); + } } } @@ -1973,6 +2034,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) @@ -2015,9 +2098,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 31240ca..6239322 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> @@ -182,6 +183,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
Rather than supplying the path again in the formatting of the sysfs scsi_host directory. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_scsi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index b5dbe51..e42778f 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -475,7 +475,8 @@ virStorageBackendSCSITriggerRescan(uint32_t host) VIR_DEBUG("Triggering rescan of host %d", host); - if (virAsprintf(&path, "/sys/class/scsi_host/host%u/scan", host) < 0) { + if (virAsprintf(&path, "%s/host%u/scan", + LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) { retval = -1; goto out; } @@ -663,7 +664,8 @@ virStorageBackendSCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED, if (getHostNumber(name, &host) < 0) goto cleanup; - if (virAsprintf(&path, "/sys/class/scsi_host/host%d", host) < 0) + if (virAsprintf(&path, "%s/host%d", + LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) goto cleanup; *isActive = virFileExists(path); -- 1.9.3
On 08.07.2014 13:54, John Ferlan wrote:
Rather than supplying the path again in the formatting of the sysfs scsi_host directory.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_scsi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index b5dbe51..e42778f 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -475,7 +475,8 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
VIR_DEBUG("Triggering rescan of host %d", host);
- if (virAsprintf(&path, "/sys/class/scsi_host/host%u/scan", host) < 0) { + if (virAsprintf(&path, "%s/host%u/scan", + LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) { retval = -1; goto out; } @@ -663,7 +664,8 @@ virStorageBackendSCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED, if (getHostNumber(name, &host) < 0) goto cleanup;
- if (virAsprintf(&path, "/sys/class/scsi_host/host%d", host) < 0) + if (virAsprintf(&path, "%s/host%d", + LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) goto cleanup;
*isActive = virFileExists(path);
Huh, it's been defined, but not used anywhere till now. :) Michal
Introduce a new function to read the current scsi_host entry and return the value found in the 'unique_id' file. Add a 'scsihosttest' test (similar to the fchosttest, but incorporating some of the concepts of the mocked pci test library) in order to read the unique_id file like would be found in the /sys/class/scsi_host tree. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virutil.c | 53 ++++++++++ src/util/virutil.h | 4 + tests/Makefile.am | 7 ++ tests/scsihosttest.c | 254 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 319 insertions(+) create mode 100644 tests/scsihosttest.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6d7bf41..bf365ac 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2119,6 +2119,7 @@ virParseOwnershipIds; virParseVersionString; virPipeReadUntilEOF; virReadFCHost; +virReadSCSIUniqueId; virScaleInteger; virSetBlocking; virSetCloseExec; diff --git a/src/util/virutil.c b/src/util/virutil.c index 95d1ff9..c73ce06 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1681,6 +1681,50 @@ virGetDeviceUnprivSGIO(const char *path, # define SYSFS_FC_HOST_PATH "/sys/class/fc_host/" # define SYSFS_SCSI_HOST_PATH "/sys/class/scsi_host/" +/* virReadSCSIUniqueId: + * @sysfs_prefix: "scsi_host" sysfs path, defaults to SYSFS_SCSI_HOST_PATH + * @host: Host number, E.g. 5 of "scsi_host/host5" + * @result: Return the entry value as an unsigned int + * + * Read the value of the "scsi_host" unique_id file. + * + * Returns 0 on success, and @result is filled with the unique_id value + * Otherwise returns -1 + */ +int +virReadSCSIUniqueId(const char *sysfs_prefix, + int host, + int *result) +{ + char *sysfs_path = NULL; + char *p = NULL; + int ret = -1; + char *buf = NULL; + int unique_id; + + if (virAsprintf(&sysfs_path, "%s/host%d/unique_id", + sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_HOST_PATH, + host) < 0) + goto cleanup; + + if (virFileReadAll(sysfs_path, 1024, &buf) < 0) + goto cleanup; + + if ((p = strchr(buf, '\n'))) + *p = '\0'; + + if (virStrToLong_i(buf, NULL, 10, &unique_id) < 0) + goto cleanup; + + *result = unique_id; + ret = 0; + + cleanup: + VIR_FREE(sysfs_path); + VIR_FREE(buf); + return ret; +} + /* virReadFCHost: * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH * @host: Host number, E.g. 5 of "fc_host/host5" @@ -2034,6 +2078,15 @@ virFindFCHostCapableVport(const char *sysfs_prefix) } #else int +virReadSCSIUniqueId(const char *sysfs_prefix ATTRIBUTE_UNUSED, + int host ATTRIBUTE_UNUSED, + unsigned int *result ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; +} + +int virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, int host ATTRIBUTE_UNUSED, const char *entry ATTRIBUTE_UNUSED, diff --git a/src/util/virutil.h b/src/util/virutil.h index 2bb74e2..1407dfd 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -164,6 +164,10 @@ int virGetDeviceUnprivSGIO(const char *path, int *unpriv_sgio); char *virGetUnprivSGIOSysfsPath(const char *path, const char *sysfs_dir); +int virReadSCSIUniqueId(const char *sysfs_prefix, + int host, + int *result) + ATTRIBUTE_NONNULL(3); int virReadFCHost(const char *sysfs_prefix, int host, const char *entry, diff --git a/tests/Makefile.am b/tests/Makefile.am index bc1040a..ecb2f34 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -83,6 +83,7 @@ EXTRA_DIST = \ domainsnapshotxml2xmlin \ domainsnapshotxml2xmlout \ fchostdata \ + scsihostdata \ interfaceschemadata \ lxcconf2xmldata \ lxcxml2xmldata \ @@ -188,6 +189,7 @@ endif WITH_REMOTE if WITH_LINUX test_programs += fchosttest +test_programs += scsihosttest endif WITH_LINUX if WITH_LIBVIRTD @@ -1146,8 +1148,13 @@ fchosttest_SOURCES = \ fchosttest.c testutils.h testutils.c fchosttest_LDADD = $(LDADDS) +scsihosttest_SOURCES = \ + scsihosttest.c testutils.h testutils.c +scsihosttest_LDADD = $(LDADDS) + else ! WITH_LINUX EXTRA_DIST += fchosttest.c +EXTRA_DIST += scsihosttest.c endif ! WITH_LINUX if WITH_LINUX diff --git a/tests/scsihosttest.c b/tests/scsihosttest.c new file mode 100644 index 0000000..990fe80 --- /dev/null +++ b/tests/scsihosttest.c @@ -0,0 +1,254 @@ +/* + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include <config.h> + +#include "testutils.h" + +#ifdef __linux__ + +# include <fcntl.h> +# include <sys/stat.h> +# include "virstring.h" +# include "virutil.h" +# include "virerror.h" +# include "virlog.h" + +# define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("tests.scsihosttest"); + +char *scsihost_class_path; +# define TEST_SCSIHOST_CLASS_PATH scsihost_class_path + +/* + * Initialized/create a mock sysfs environment with 4 scsi_host devices + * located on "0000:00:1f.1" and "0000:00:1f.2". Each directory will + * contain 4 unique_id files having the same value. + * + * The environment is: + * + * 4 files: + * + * sys/devices/pci0000:00/0000:00:1f.1/ata1/host0/scsi_host/host0/unique_id + * sys/devices/pci0000:00/0000:00:1f.1/ata2/host1/scsi_host/host1/unique_id + * sys/devices/pci0000:00/0000:00:1f.2/ata1/host0/scsi_host/host0/unique_id + * sys/devices/pci0000:00/0000:00:1f.2/ata2/host1/scsi_host/host1/unique_id + * + * 4 symlinks: + * + * sys/class/scsi_host/host0 -> link to 1f.1 host 0 + * sys/class/scsi_host/host1 -> link to 1f.1 host 1 + * sys/class/scsi_host/host2 -> link to 1f.2 host 0 + * sys/class/scsi_host/host3 -> link to 1f.2 host 1 + * + * The unique_id's for host0 and host2 are set to "1" + * The unique_id's for host1 and host3 are set to "2" + */ + +static int +create_scsihost(const char *fakesysfsdir, const char *devicepath, + const char *unique_id, const char *hostname) +{ + char *unique_id_path = NULL; + char *link_path = NULL; + char *spot; + int ret = -1; + int fd = -1; + + if (virAsprintfQuiet(&unique_id_path, "%s/devices/pci0000:00/%s/unique_id", + fakesysfsdir, devicepath) < 0 || + virAsprintfQuiet(&link_path, "%s/class/scsi_host/%s", + fakesysfsdir, hostname) < 0) { + fprintf(stderr, "Out of memory\n"); + goto cleanup; + } + + /* Rather than create path & file, temporarily snip off the file to + * create the path + */ + if (!(spot = strstr(unique_id_path, "unique_id"))) { + fprintf(stderr, "Did not find unique_id in path\n"); + goto cleanup; + } + spot--; + *spot = '\0'; + if (virFileMakePathWithMode(unique_id_path, 0755) < 0) { + fprintf(stderr, "Unable to make path to '%s'\n", unique_id_path); + goto cleanup; + } + *spot = '/'; + + /* Rather than create path & file, temporarily snip off the file to + * create the path + */ + if (!(spot = strstr(link_path, hostname))) { + fprintf(stderr, "Did not find hostname in path\n"); + goto cleanup; + } + spot--; + *spot = '\0'; + if (virFileMakePathWithMode(link_path, 0755) < 0) { + fprintf(stderr, "Unable to make path to '%s'\n", link_path); + goto cleanup; + } + *spot = '/'; + + if ((fd = open(unique_id_path, O_CREAT|O_WRONLY, 0444)) < 0) { + fprintf(stderr, "Unable to create '%s'\n", unique_id_path); + goto cleanup; + } + + if (safewrite(fd, unique_id, 1) != 1) { + fprintf(stderr, "Unable to write '%s'\n", unique_id); + goto cleanup; + } + VIR_DEBUG("Created unique_id '%s'", unique_id_path); + + /* The link is to the path not the file - so remove the file */ + if (!(spot = strstr(unique_id_path, "unique_id"))) { + fprintf(stderr, "Did not find unique_id in path\n"); + goto cleanup; + } + spot--; + *spot = '\0'; + if (symlink(unique_id_path, link_path) < 0) { + fprintf(stderr, "Unable to create symlink '%s' to '%s'\n", + link_path, unique_id_path); + goto cleanup; + } + VIR_DEBUG("Created symlink '%s'", link_path); + + ret = 0; + + cleanup: + VIR_FORCE_CLOSE(fd); + VIR_FREE(unique_id_path); + VIR_FREE(link_path); + return ret; +} + +static int +init_scsihost_sysfs(const char *fakesysfsdir) +{ + int ret = 0; + + if (create_scsihost(fakesysfsdir, + "0000:00:1f.1/ata1/host0/scsi_host/host0", + "1", "host0") < 0 || + create_scsihost(fakesysfsdir, + "0000:00:1f.1/ata2/host1/scsi_host/host1", + "2", "host1") < 0 || + create_scsihost(fakesysfsdir, + "0000:00:1f.2/ata1/host0/scsi_host/host0", + "1", "host2") < 0 || + create_scsihost(fakesysfsdir, + "0000:00:1f.2/ata2/host1/scsi_host/host1", + "2", "host3") < 0) + ret = -1; + + return ret; +} + +/* Test virReadSCSIUniqueId */ +static int +testVirReadSCSIUniqueId(const void *data ATTRIBUTE_UNUSED) +{ + int hostnum, unique_id; + + for (hostnum = 0; hostnum < 4; hostnum++) { + if (virReadSCSIUniqueId(TEST_SCSIHOST_CLASS_PATH, + hostnum, &unique_id) < 0) { + fprintf(stderr, "Failed to read hostnum=%d unique_id\n", hostnum); + return -1; + } + + /* host0 and host2 have unique_id == 1 + * host1 and host3 have unique_id == 2 + */ + if ((hostnum == 0 || hostnum == 2) && unique_id != 1) { + fprintf(stderr, "The unique_id='%d' for hostnum=%d is wrong\n", + unique_id, hostnum); + return -1; + } else if ((hostnum == 1 || hostnum == 3) && unique_id != 2) { + fprintf(stderr, "The unique_id='%d' for hostnum=%d is wrong\n", + unique_id, hostnum); + return -1; + } + } + + return 0; +} + +# define FAKESYSFSDIRTEMPLATE abs_builddir "/fakesysfsdir-XXXXXX" + +static int +mymain(void) +{ + int ret = -1; + char *fakesysfsdir = NULL; + + if (VIR_STRDUP_QUIET(fakesysfsdir, FAKESYSFSDIRTEMPLATE) < 0) { + fprintf(stderr, "Out of memory\n"); + goto cleanup; + } + + if (!mkdtemp(fakesysfsdir)) { + fprintf(stderr, "Cannot create fakesysfsdir"); + goto cleanup; + } + + setenv("LIBVIRT_FAKE_SYSFS_DIR", fakesysfsdir, 1); + + if (init_scsihost_sysfs(fakesysfsdir) < 0) { + fprintf(stderr, "Failed to create fakesysfs='%s'\n", fakesysfsdir); + goto cleanup; + } + + if (virAsprintfQuiet(&scsihost_class_path, "%s/class/scsi_host", + fakesysfsdir) < 0) { + fprintf(stderr, "Out of memory\n"); + goto cleanup; + } + VIR_DEBUG("Reading from '%s'", scsihost_class_path); + + if (virtTestRun("testVirReadSCSIUniqueId", + testVirReadSCSIUniqueId, NULL) < 0) { + ret = -1; + goto cleanup; + } + + ret = 0; + + cleanup: + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) + virFileDeleteTree(fakesysfsdir); + VIR_FREE(fakesysfsdir); + VIR_FREE(scsihost_class_path); + return ret; +} + +VIRT_TEST_MAIN(mymain) +#else +int +main(void) +{ + return EXIT_AM_SKIP; +} +#endif -- 1.9.3
On 08.07.2014 13:54, John Ferlan wrote:
Introduce a new function to read the current scsi_host entry and return the value found in the 'unique_id' file.
Add a 'scsihosttest' test (similar to the fchosttest, but incorporating some of the concepts of the mocked pci test library) in order to read the unique_id file like would be found in the /sys/class/scsi_host tree.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virutil.c | 53 ++++++++++ src/util/virutil.h | 4 + tests/Makefile.am | 7 ++ tests/scsihosttest.c | 254 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 319 insertions(+) create mode 100644 tests/scsihosttest.c
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6d7bf41..bf365ac 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2119,6 +2119,7 @@ virParseOwnershipIds; virParseVersionString; virPipeReadUntilEOF; virReadFCHost; +virReadSCSIUniqueId; virScaleInteger; virSetBlocking; virSetCloseExec; diff --git a/src/util/virutil.c b/src/util/virutil.c index 95d1ff9..c73ce06 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1681,6 +1681,50 @@ virGetDeviceUnprivSGIO(const char *path, # define SYSFS_FC_HOST_PATH "/sys/class/fc_host/" # define SYSFS_SCSI_HOST_PATH "/sys/class/scsi_host/"
+/* virReadSCSIUniqueId: + * @sysfs_prefix: "scsi_host" sysfs path, defaults to SYSFS_SCSI_HOST_PATH + * @host: Host number, E.g. 5 of "scsi_host/host5" + * @result: Return the entry value as an unsigned int + * + * Read the value of the "scsi_host" unique_id file. + * + * Returns 0 on success, and @result is filled with the unique_id value + * Otherwise returns -1 + */ +int +virReadSCSIUniqueId(const char *sysfs_prefix, + int host, + int *result) +{ + char *sysfs_path = NULL; + char *p = NULL; + int ret = -1; + char *buf = NULL; + int unique_id; + + if (virAsprintf(&sysfs_path, "%s/host%d/unique_id", + sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_HOST_PATH, + host) < 0) + goto cleanup;
This prints a message on error.
+ + if (virFileReadAll(sysfs_path, 1024, &buf) < 0) + goto cleanup;
And so does this.
+ + if ((p = strchr(buf, '\n'))) + *p = '\0'; + + if (virStrToLong_i(buf, NULL, 10, &unique_id) < 0) + goto cleanup;
This, however does not. If the unique_id file didn't contain a number, the caller gets -1 returned but have no clue why. I think: virReportError(VIR_ERR_INTERNAL_ERROR, _(unable to parse unique_id: %s"), buf); will do. (yes, we are misusing the VIR_ERR_INTERNAL_ERROR code soo much).
+ + *result = unique_id; + ret = 0; + + cleanup: + VIR_FREE(sysfs_path); + VIR_FREE(buf); + return ret; +} + /* virReadFCHost: * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH * @host: Host number, E.g. 5 of "fc_host/host5" @@ -2034,6 +2078,15 @@ virFindFCHostCapableVport(const char *sysfs_prefix) } #else int +virReadSCSIUniqueId(const char *sysfs_prefix ATTRIBUTE_UNUSED, + int host ATTRIBUTE_UNUSED, + unsigned int *result ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; +} + +int virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, int host ATTRIBUTE_UNUSED, const char *entry ATTRIBUTE_UNUSED, diff --git a/src/util/virutil.h b/src/util/virutil.h index 2bb74e2..1407dfd 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -164,6 +164,10 @@ int virGetDeviceUnprivSGIO(const char *path, int *unpriv_sgio); char *virGetUnprivSGIOSysfsPath(const char *path, const char *sysfs_dir); +int virReadSCSIUniqueId(const char *sysfs_prefix, + int host, + int *result) + ATTRIBUTE_NONNULL(3); int virReadFCHost(const char *sysfs_prefix, int host, const char *entry, diff --git a/tests/Makefile.am b/tests/Makefile.am index bc1040a..ecb2f34 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -83,6 +83,7 @@ EXTRA_DIST = \ domainsnapshotxml2xmlin \ domainsnapshotxml2xmlout \ fchostdata \ + scsihostdata \ interfaceschemadata \ lxcconf2xmldata \ lxcxml2xmldata \ @@ -188,6 +189,7 @@ endif WITH_REMOTE
if WITH_LINUX test_programs += fchosttest +test_programs += scsihosttest endif WITH_LINUX
if WITH_LIBVIRTD @@ -1146,8 +1148,13 @@ fchosttest_SOURCES = \ fchosttest.c testutils.h testutils.c fchosttest_LDADD = $(LDADDS)
+scsihosttest_SOURCES = \ + scsihosttest.c testutils.h testutils.c +scsihosttest_LDADD = $(LDADDS) + else ! WITH_LINUX EXTRA_DIST += fchosttest.c +EXTRA_DIST += scsihosttest.c endif ! WITH_LINUX
if WITH_LINUX diff --git a/tests/scsihosttest.c b/tests/scsihosttest.c new file mode 100644 index 0000000..990fe80 --- /dev/null +++ b/tests/scsihosttest.c
Nice, new test. Michal
Add an optional unique_id parameter to nodedev. Allows for easier lookup and display of the unique_id value in order to document for use with scsi_host code. Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatnode.html.in | 11 +++++++++++ docs/schemas/nodedev.rng | 6 ++++++ src/conf/node_device_conf.c | 23 ++++++++++++++++------ src/conf/node_device_conf.h | 1 + src/node_device/node_device_linux_sysfs.c | 6 ++++++ src/test/test_driver.c | 5 +++-- .../pci_8086_27c5_scsi_host_0_unique_id.xml | 8 ++++++++ tests/nodedevxml2xmltest.c | 1 + 8 files changed, 53 insertions(+), 8 deletions(-) create mode 100644 tests/nodedevschemadata/pci_8086_27c5_scsi_host_0_unique_id.xml diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in index 5932811..b820a34 100644 --- a/docs/formatnode.html.in +++ b/docs/formatnode.html.in @@ -195,6 +195,17 @@ <dl> <dt><code>host</code></dt> <dd>The SCSI host number.</dd> + <dt><code>unique_id</code></dt> + <dd>On input, this optionally provides the value from the + 'unique_id' file found in the scsi_host's directory. To + view the values of all 'unique_id' files, use <code>find -H + /sys/class/scsi_host/host{0..9}/unique_id | + xargs grep '[0-9]'</code>. On output, if the unique_id + file exists, the value from the file will be displayed. + This can be used in order to help uniquely identify the + scsi_host adapter in a <a href="formatstorage.html"> + Storage Pool</a>. <span class="since">Since 1.2.7</span> + </dd> <dt><code>capability</code></dt> <dd>Current capabilities include "vports_ops" (indicates vport operations are supported) and "fc_host". "vport_ops" diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index b6de201..c4f402c 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -340,6 +340,12 @@ </element> <optional> + <element name='unique_id'> + <ref name='positiveInteger'/> + </element> + </optional> + + <optional> <zeroOrMore> <element name='capability'> <choice> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 30aa477..910a371 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -442,6 +442,9 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def) case VIR_NODE_DEV_CAP_SCSI_HOST: virBufferAsprintf(&buf, "<host>%d</host>\n", data->scsi_host.host); + if (data->scsi_host.unique_id != -1) + virBufferAsprintf(&buf, "<unique_id>%d</unique_id>\n", + data->scsi_host.unique_id); if (data->scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { virBufferAddLit(&buf, "<capability type='fc_host'>\n"); virBufferAdjustIndent(&buf, 2); @@ -826,12 +829,20 @@ virNodeDevCapSCSIHostParseXML(xmlXPathContextPtr ctxt, orignode = ctxt->node; ctxt->node = node; - if (create == EXISTING_DEVICE && - virNodeDevCapsDefParseULong("number(./host[1])", ctxt, - &data->scsi_host.host, def, - _("no SCSI host ID supplied for '%s'"), - _("invalid SCSI host ID supplied for '%s'")) < 0) { - goto out; + if (create == EXISTING_DEVICE) { + if (virNodeDevCapsDefParseULong("number(./host[1])", ctxt, + &data->scsi_host.host, def, + _("no SCSI host ID supplied for '%s'"), + _("invalid SCSI host ID supplied for '%s'")) < 0) { + goto out; + } + /* Optional unique_id value */ + data->scsi_host.unique_id = -1; + if (virNodeDevCapsDefParseIntOptional("number(./unique_id[1])", ctxt, + &data->scsi_host.unique_id, def, + _("invalid unique_id supplied for '%s'")) < 0) { + goto out; + } } if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) < 0) { diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 61d2e19..b5bfb7b 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -171,6 +171,7 @@ struct _virNodeDevCapsDef { } net; struct { unsigned int host; + int unique_id; char *wwnn; char *wwpn; char *fabric_wwn; diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index 6d9726f..be95e51 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -47,6 +47,12 @@ detect_scsi_host_caps(union _virNodeDevCapData *d) char *vports = NULL; int ret = -1; + if (virReadSCSIUniqueId(NULL, d->scsi_host.host, + &d->scsi_host.unique_id) < 0) { + VIR_DEBUG("Failed to read unique_id for host%d", d->scsi_host.host); + d->scsi_host.unique_id = -1; + } + VIR_DEBUG("Checking if host%d is an FC HBA", d->scsi_host.host); if (virIsCapableFCHost(NULL, d->scsi_host.host)) { diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 7bfc88d..11821af 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6087,14 +6087,15 @@ testNodeDeviceCreateXML(virConnectPtr conn, if (VIR_STRDUP(def->name, wwpn) < 0) goto cleanup; - /* Fill in a random 'host' value, since this would also come from - * the backend */ + /* Fill in a random 'host' and 'unique_id' value, + * since this would also come from the backend */ caps = def->caps; while (caps) { if (caps->type != VIR_NODE_DEV_CAP_SCSI_HOST) continue; caps->data.scsi_host.host = virRandomBits(10); + caps->data.scsi_host.unique_id = 2; caps = caps->next; } diff --git a/tests/nodedevschemadata/pci_8086_27c5_scsi_host_0_unique_id.xml b/tests/nodedevschemadata/pci_8086_27c5_scsi_host_0_unique_id.xml new file mode 100644 index 0000000..5428f59 --- /dev/null +++ b/tests/nodedevschemadata/pci_8086_27c5_scsi_host_0_unique_id.xml @@ -0,0 +1,8 @@ +<device> + <name>pci_8086_27c5_scsi_host_0</name> + <parent>pci_8086_27c5</parent> + <capability type='scsi_host'> + <host>1</host> + <unique_id>2</unique_id> + </capability> +</device> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index fe1fd88..a37d729 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -82,6 +82,7 @@ mymain(void) DO_TEST("pci_1002_71c4"); DO_TEST("pci_8086_10c9_sriov_pf"); DO_TEST("pci_8086_27c5_scsi_host_0"); + DO_TEST("pci_8086_27c5_scsi_host_0_unique_id"); DO_TEST("pci_8086_27c5_scsi_host_scsi_device_lun0"); DO_TEST("pci_8086_27c5_scsi_host_scsi_host"); DO_TEST("pci_8086_27c5_scsi_host"); -- 1.9.3
Introduce a new function to parse the provided scsi_host parent address and unique_id value in order to find the /sys/class/scsi_host directory which will allow a stable SCSI host address Add a test to scsihosttest to lookup the host# name by using the PCI address and unique_id value Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatstorage.html.in | 5 ++- src/libvirt_private.syms | 1 + src/util/virutil.c | 101 +++++++++++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 4 ++ tests/scsihosttest.c | 54 ++++++++++++++++++++++++ 5 files changed, 164 insertions(+), 1 deletion(-) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index d0c111e..295c27f 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -226,7 +226,10 @@ 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>. + xargs grep '[0-9]'</code>. Optionally, the + <code>virsh nodedev-dumpxml scsi_hostN</code>' of a + specific scsi_hostN list entry will list the + <code>unique_id</code> value. </dd> </dl> </dd> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bf365ac..4aa41bf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2088,6 +2088,7 @@ virDoubleToStr; virEnumFromString; virEnumToString; virFindFCHostCapableVport; +virFindSCSIHostByPCI; virFormatIntDecimal; virGetDeviceID; virGetDeviceUnprivSGIO; diff --git a/src/util/virutil.c b/src/util/virutil.c index c73ce06..350b6fe 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -1725,6 +1725,98 @@ virReadSCSIUniqueId(const char *sysfs_prefix, return ret; } +/* virFindSCSIHostByPCI: + * @sysfs_prefix: "scsi_host" sysfs path, defaults to SYSFS_SCSI_HOST_PATH + * @parentaddr: string of the PCI address "scsi_host" device to be found + * @unique_id: unique_id value of the to be found "scsi_host" device + * @result: Return the host# of the matching "scsi_host" device + * + * Iterate over the SYSFS_SCSI_HOST_PATH entries looking for a matching + * PCI Address in the expected format (dddd:bb:ss.f, where 'dddd' is the + * 'domain' value, 'bb' is the 'bus' value, 'ss' is the 'slot' value, and + * 'f' is the 'function' value from the PCI address) with a unique_id file + * entry having the value expected. Unlike virReadSCSIUniqueId() we don't + * have a host number yet and that's what we're looking for. + * + * Returns the host name of the "scsi_host" which must be freed by the caller, + * or NULL on failure + */ +char * +virFindSCSIHostByPCI(const char *sysfs_prefix, + const char *parentaddr, + unsigned int unique_id) +{ + const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_HOST_PATH; + struct dirent *entry = NULL; + DIR *dir = NULL; + char *host_link = NULL; + char *host_path = NULL; + char *p = NULL; + char *ret = NULL; + char *buf = NULL; + char *unique_path = NULL; + unsigned int read_unique_id; + + if (!(dir = opendir(prefix))) { + virReportSystemError(errno, + _("Failed to opendir path '%s'"), + prefix); + goto cleanup; + } + + while (virDirRead(dir, &entry, prefix) > 0) { + if (entry->d_name[0] == '.' || !virFileIsLink(entry->d_name)) + continue; + + if (virAsprintf(&host_link, "%s/%s", prefix, entry->d_name) < 0) + goto cleanup; + + if (virFileResolveLink(host_link, &host_path) < 0) + goto cleanup; + + if (!strstr(host_path, parentaddr)) { + VIR_FREE(host_link); + VIR_FREE(host_path); + continue; + } + VIR_FREE(host_link); + VIR_FREE(host_path); + + if (virAsprintf(&unique_path, "%s/%s/unique_id", prefix, + entry->d_name) < 0) + goto cleanup; + + if (!virFileExists(unique_path)) { + VIR_FREE(unique_path); + continue; + } + + if (virFileReadAll(unique_path, 1024, &buf) < 0) + goto cleanup; + + if ((p = strchr(buf, '\n'))) + *p = '\0'; + + if (virStrToLong_ui(buf, NULL, 10, &read_unique_id) < 0) + goto cleanup; + + if (read_unique_id != unique_id) { + VIR_FREE(unique_path); + continue; + } + + ignore_value(VIR_STRDUP(ret, entry->d_name)); + break; + } + + cleanup: + closedir(dir); + VIR_FREE(unique_path); + VIR_FREE(host_link); + VIR_FREE(host_path); + return ret; +} + /* virReadFCHost: * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH * @host: Host number, E.g. 5 of "fc_host/host5" @@ -2086,6 +2178,15 @@ virReadSCSIUniqueId(const char *sysfs_prefix ATTRIBUTE_UNUSED, return -1; } +char * +virFindSCSIHostByPCI(const char *sysfs_prefix ATTRIBUTE_UNUSED, + const char *parentaddr ATTRIBUTE_UNUSED, + unsigned int unique_id ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; +} + int virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED, int host ATTRIBUTE_UNUSED, diff --git a/src/util/virutil.h b/src/util/virutil.h index 1407dfd..e8c1d7c 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -168,6 +168,10 @@ int virReadSCSIUniqueId(const char *sysfs_prefix, int host, int *result) ATTRIBUTE_NONNULL(3); +char * +virFindSCSIHostByPCI(const char *sysfs_prefix, + const char *parentaddr, + unsigned int unique_id); int virReadFCHost(const char *sysfs_prefix, int host, const char *entry, diff --git a/tests/scsihosttest.c b/tests/scsihosttest.c index 990fe80..eecb1c3 100644 --- a/tests/scsihosttest.c +++ b/tests/scsihosttest.c @@ -196,6 +196,54 @@ testVirReadSCSIUniqueId(const void *data ATTRIBUTE_UNUSED) return 0; } +/* Test virFindSCSIHostByPCI */ +static int +testVirFindSCSIHostByPCI(const void *data ATTRIBUTE_UNUSED) +{ + unsigned int unique_id1 = 1; + unsigned int unique_id2 = 2; + const char *pci_addr1 = "0000:00:1f.1"; + const char *pci_addr2 = "0000:00:1f.2"; + char *path_addr = NULL; + char *ret_host = NULL; + int ret = -1; + + if (virAsprintf(&path_addr, "%s/%s", abs_srcdir, + "sysfs/class/scsi_host") < 0) + goto cleanup; + + if (!(ret_host = virFindSCSIHostByPCI(TEST_SCSIHOST_CLASS_PATH, + pci_addr1, unique_id1)) || + STRNEQ(ret_host, "host0")) + goto cleanup; + VIR_FREE(ret_host); + + if (!(ret_host = virFindSCSIHostByPCI(TEST_SCSIHOST_CLASS_PATH, + pci_addr1, unique_id2)) || + STRNEQ(ret_host, "host1")) + goto cleanup; + VIR_FREE(ret_host); + + if (!(ret_host = virFindSCSIHostByPCI(TEST_SCSIHOST_CLASS_PATH, + pci_addr2, unique_id1)) || + STRNEQ(ret_host, "host2")) + goto cleanup; + VIR_FREE(ret_host); + + if (!(ret_host = virFindSCSIHostByPCI(TEST_SCSIHOST_CLASS_PATH, + pci_addr2, unique_id2)) || + STRNEQ(ret_host, "host3")) + goto cleanup; + VIR_FREE(ret_host); + + ret = 0; + + cleanup: + VIR_FREE(ret_host); + VIR_FREE(path_addr); + return ret; +} + # define FAKESYSFSDIRTEMPLATE abs_builddir "/fakesysfsdir-XXXXXX" static int @@ -234,6 +282,12 @@ mymain(void) goto cleanup; } + if (virtTestRun("testVirFindSCSIHostByPCI", + testVirFindSCSIHostByPCI, NULL) < 0) { + ret = -1; + goto cleanup; + } + ret = 0; cleanup: -- 1.9.3
If a parentaddr was provided in the XML, have getAdapterName lookup the stable address. This allows virStorageBackendSCSICheckPool() and virStorageBackendSCSIRefreshPool() to automagically find the scsi_host by its PCI address and unique_id Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_scsi.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index e42778f..7c5044c 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -547,9 +547,29 @@ static char * getAdapterName(virStoragePoolSourceAdapter adapter) { char *name = NULL; + char *parentaddr = NULL; if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { - ignore_value(VIR_STRDUP(name, adapter.data.scsi_host.name)); + if (adapter.data.scsi_host.has_parent) { + unsigned int unique_id = adapter.data.scsi_host.unique_id; + + if (virAsprintf(&parentaddr, "%04x:%02x:%02x.%01x", + adapter.data.scsi_host.parentaddr.domain, + adapter.data.scsi_host.parentaddr.bus, + adapter.data.scsi_host.parentaddr.slot, + adapter.data.scsi_host.parentaddr.function) < 0) + goto cleanup; + if (!(name = virFindSCSIHostByPCI(NULL, parentaddr, + unique_id))) { + virReportError(VIR_ERR_XML_ERROR, + _("Failed to find scsi_host using PCI '%s' " + "and unique_id='%u'"), + parentaddr, unique_id); + goto cleanup; + } + } else { + ignore_value(VIR_STRDUP(name, adapter.data.scsi_host.name)); + } } else if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) { if (!(name = virGetFCHostNameByWWN(NULL, adapter.data.fchost.wwnn, @@ -561,6 +581,8 @@ getAdapterName(virStoragePoolSourceAdapter adapter) } } + cleanup: + VIR_FREE(parentaddr); return name; } -- 1.9.3
On 08.07.2014 13:54, John Ferlan wrote:
Reposting of a series from last month changing only the span version in the docs from 1.2.6 to 1.2.7. Previous posting here:
http://www.redhat.com/archives/libvir-list/2014-June/msg00448.html
The concept still remains the same - rather than rely on the hostNN numbers for the scsi_host to remain stable and unique across host reboots and/or kernel rebuilds, allow use a combination of the scsi_host's PCI address and the value from the hostNN's 'unique_id' file.
John Ferlan (6): getAdapterName: check for SCSI_HOST scsi_backend: Use existing LINUX_SYSFS_SCSI_HOST_PREFIX definition virutil: Introduce virReadSCSIUniqueId Add unique_id to nodedev output scsi_host: Introduce virFindSCSIHostByPCI getAdapterName: Lookup stable scsi_host
Osier Yang (2): virStoragePoolSourceAdapter: Refine the SCSI_HOST adapter name storage: Introduce parentaddr into virStoragePoolSourceAdapter
docs/formatnode.html.in | 11 + docs/formatstorage.html.in | 143 ++++++++-- docs/schemas/basictypes.rng | 24 +- docs/schemas/nodedev.rng | 6 + src/conf/node_device_conf.c | 23 +- src/conf/node_device_conf.h | 1 + src/conf/storage_conf.c | 111 +++++++- src/conf/storage_conf.h | 8 +- src/libvirt_private.syms | 2 + src/node_device/node_device_linux_sysfs.c | 6 + src/phyp/phyp_driver.c | 8 +- src/storage/storage_backend_scsi.c | 53 +++- src/test/test_driver.c | 5 +- src/util/virutil.c | 154 +++++++++++ src/util/virutil.h | 8 + tests/Makefile.am | 7 + .../pci_8086_27c5_scsi_host_0_unique_id.xml | 8 + tests/nodedevxml2xmltest.c | 1 + tests/scsihosttest.c | 308 +++++++++++++++++++++ .../pool-scsi-type-scsi-host-stable.xml | 19 ++ .../pool-scsi-type-scsi-host-stable.xml | 22 ++ tests/storagepoolxml2xmltest.c | 1 + 22 files changed, 868 insertions(+), 61 deletions(-) create mode 100644 tests/nodedevschemadata/pci_8086_27c5_scsi_host_0_unique_id.xml create mode 100644 tests/scsihosttest.c create mode 100644 tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host-stable.xml create mode 100644 tests/storagepoolxml2xmlout/pool-scsi-type-scsi-host-stable.xml
ACK series, but please see my inline comments. Michal
On 07/18/2014 10:30 AM, Michal Privoznik wrote:
On 08.07.2014 13:54, John Ferlan wrote:
Reposting of a series from last month changing only the span version in the docs from 1.2.6 to 1.2.7. Previous posting here:
http://www.redhat.com/archives/libvir-list/2014-June/msg00448.html
The concept still remains the same - rather than rely on the hostNN numbers for the scsi_host to remain stable and unique across host reboots and/or kernel rebuilds, allow use a combination of the scsi_host's PCI address and the value from the hostNN's 'unique_id' file.
John Ferlan (6): getAdapterName: check for SCSI_HOST scsi_backend: Use existing LINUX_SYSFS_SCSI_HOST_PREFIX definition virutil: Introduce virReadSCSIUniqueId Add unique_id to nodedev output scsi_host: Introduce virFindSCSIHostByPCI getAdapterName: Lookup stable scsi_host
Osier Yang (2): virStoragePoolSourceAdapter: Refine the SCSI_HOST adapter name storage: Introduce parentaddr into virStoragePoolSourceAdapter
docs/formatnode.html.in | 11 + docs/formatstorage.html.in | 143 ++++++++-- docs/schemas/basictypes.rng | 24 +- docs/schemas/nodedev.rng | 6 + src/conf/node_device_conf.c | 23 +- src/conf/node_device_conf.h | 1 + src/conf/storage_conf.c | 111 +++++++- src/conf/storage_conf.h | 8 +- src/libvirt_private.syms | 2 + src/node_device/node_device_linux_sysfs.c | 6 + src/phyp/phyp_driver.c | 8 +- src/storage/storage_backend_scsi.c | 53 +++- src/test/test_driver.c | 5 +- src/util/virutil.c | 154 +++++++++++ src/util/virutil.h | 8 + tests/Makefile.am | 7 + .../pci_8086_27c5_scsi_host_0_unique_id.xml | 8 + tests/nodedevxml2xmltest.c | 1 + tests/scsihosttest.c | 308 +++++++++++++++++++++ .../pool-scsi-type-scsi-host-stable.xml | 19 ++ .../pool-scsi-type-scsi-host-stable.xml | 22 ++ tests/storagepoolxml2xmltest.c | 1 + 22 files changed, 868 insertions(+), 61 deletions(-) create mode 100644 tests/nodedevschemadata/pci_8086_27c5_scsi_host_0_unique_id.xml create mode 100644 tests/scsihosttest.c create mode 100644 tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host-stable.xml create mode 100644 tests/storagepoolxml2xmlout/pool-scsi-type-scsi-host-stable.xml
ACK series, but please see my inline comments.
Michal
Thanks for the review. Adjusted to add error message from 5/8... Just pushed (now that the power supply for my fiber connection has been replaced... funny how the kids noticed it first - Dad - the internet is lost...) John
participants (2)
-
John Ferlan -
Michal Privoznik