4:45 p.m.
https://bugzilla.redhat.com/show_bug.cgi?id=1160565
The existing code assumed that the configuration of a 'parent' attribute
was correct for the createVport path. As it turns out, that may not be
the case which leads errors during the deleteVport path because the
wwnn/wwpn isn't associated with the parent.
With this change the following is reported:
error: Failed to start pool fc_pool_host3
error: XML error: Parent attribute 'scsi_host4' does not match parent
'scsi_host3' determined for the 'scsi_host16' wwnn/wwpn lookup.
for XML as follows:
<pool type='scsi'>
<name>fc_pool</name>
<source>
<adapter type='fc_host' parent='scsi_host4'
wwnn='5001a4aaf3ca174b' wwpn='5001a4a77192b864'/>
</source>
Where 'nodedev-dumpxml scsi_host16' provides:
<device>
<name>scsi_host16</name>
<path>/sys/devices/pci0000:00/0000:00:04.0/0000:10:00.0/host3/vport-3:0-11/host16</path>
<parent>scsi_host3</parent>
<capability type='scsi_host'>
<host>16</host>
<unique_id>13</unique_id>
<capability type='fc_host'>
<wwnn>5001a4aaf3ca174b</wwnn>
<wwpn>5001a4a77192b864</wwpn>
...
The patch also adjusts the description of the storage pool to describe the
restrictions.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
docs/formatstorage.html.in | 15 ++++-
src/storage/storage_backend_scsi.c | 118 +++++++++++++++++++++++++++++++++++--
2 files changed, 126 insertions(+), 7 deletions(-)
diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index d3e6f05..9d77c45 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -166,7 +166,13 @@
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.
+ how to set the values for the wwnn/wwpn of a (v)HBA. The wwnn and
+ wwpn have very specific numerical format requirements based on the
+ hypervisor being used, thus care should be taken if you decide to
+ generate your own to follow the standards; otherwise, the pool
+ will fail to start with an opaque error message indicating failure
+ to write to the vport_create file during vport create/delete due
+ to "No such file or directory".
<span class="since">Since 1.0.4</span>
</dd>
</dl>
@@ -176,7 +182,12 @@
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).
+ virtual Host Bus Adapter (vHBA). The value provided must be
+ a vport capable scsi_host. The value is not the scsi_host of
+ the vHBA created by 'virsh nodedev-create', rather it is
+ the parent of that vHBA. If the value is not provided, libvirt
+ will determine the parent based either finding the wwnn,wwpn
+ defined for an existing scsi_host or by creating a vHBA.
<span class="since">Since 1.0.4</span>
</dd>
</dl>
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index 549d8db..5220292 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -34,6 +34,7 @@
#include "virlog.h"
#include "virfile.h"
#include "vircommand.h"
+#include "viraccessapicheck.h"
#include "virstring.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -547,8 +548,103 @@ getAdapterName(virStoragePoolSourceAdapter adapter)
return name;
}
+/*
+ * Using the host# name found via wwnn/wwpn lookup in the fc_host
+ * sysfs tree to get the parent 'scsi_host#'. On entry we need 'conn'
+ * set. We won't get here from the autostart path since the caller
+ * will return true before calling this function. For the shutdown
+ * path we won't be able to delete the vport.
+ */
+static char * ATTRIBUTE_NONNULL(1)
+getVhbaSCSIHostParent(virConnectPtr conn,
+ const char *name)
+{
+ char *nodedev_name = NULL;
+ virNodeDevicePtr device = NULL;
+ char *xml = NULL;
+ virNodeDeviceDefPtr def = NULL;
+ char *vhba_parent = NULL;
+ virErrorPtr savedError = NULL;
+
+ VIR_DEBUG("conn=%p, name=%s", conn, name);
+
+ /* We get passed "host#" from the return from virGetFCHostNameByWWN,
+ * so we need to adjust that to what the nodedev lookup expects
+ */
+ if (virAsprintf(&nodedev_name, "scsi_%s", name) < 0)
+ goto cleanup;
+
+ /* Compare the scsi_host for the name with the provided parent
+ * if not the same, then fail
+ */
+ if (!(device = virNodeDeviceLookupByName(conn, nodedev_name))) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Cannot find '%s' in node device database"),
+ nodedev_name);
+ goto cleanup;
+ }
+
+ if (!(xml = virNodeDeviceGetXMLDesc(device, 0)))
+ goto cleanup;
+
+ if (!(def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL)))
+ goto cleanup;
+
+ ignore_value(VIR_STRDUP(vhba_parent, def->parent));
+
+ cleanup:
+ if (!vhba_parent)
+ savedError = virSaveLastError();
+ VIR_FREE(nodedev_name);
+ virNodeDeviceDefFree(def);
+ VIR_FREE(xml);
+ virNodeDeviceFree(device);
+ if (savedError) {
+ virSetError(savedError);
+ virFreeError(savedError);
+ }
+ return vhba_parent;
+}
+
+/*
+ * Using the host# name found via wwnn/wwpn lookup in the fc_host
+ * sysfs tree to get the parent 'scsi_host#' to ensure it matches.
+ */
+static bool
+checkVhbaSCSIHostParent(virConnectPtr conn,
+ const char *name,
+ const char *parent_name)
+{
+ char *vhba_parent = NULL;
+ bool retval = false;
+
+ VIR_DEBUG("conn=%p, name=%s, parent_name=%s", conn, name, parent_name);
+
+ /* autostarted pool - assume we're OK */
+ if (!conn)
+ return true;
+
+ if (!(vhba_parent = getVhbaSCSIHostParent(conn, name)))
+ goto cleanup;
+
+ if (STRNEQ(parent_name, vhba_parent)) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Parent attribute '%s' does not match parent
'%s' "
+ "determined for the '%s' wwnn/wwpn lookup."),
+ parent_name, vhba_parent, name);
+ goto cleanup;
+ }
+
+ retval = true;
+
+ cleanup:
+ VIR_FREE(vhba_parent);
+ return retval;
+}
+
static int
-createVport(virStoragePoolSourceAdapter adapter)
+createVport(virConnectPtr conn,
+ virStoragePoolSourceAdapter adapter)
{
unsigned int parent_host;
char *name = NULL;
@@ -570,11 +666,23 @@ createVport(virStoragePoolSourceAdapter adapter)
}
}
- /* This filters either HBA or already created vHBA */
+ /* If we find an existing HBA/vHBA within the fc_host sysfs
+ * using the wwnn/wwpn, then a nodedev is already created for
+ * this pool and we don't have to create the vHBA
+ */
if ((name = virGetFCHostNameByWWN(NULL, adapter.data.fchost.wwnn,
adapter.data.fchost.wwpn))) {
+ int retval = 0;
+
+ /* If a parent was provided, let's make sure the 'name' we've
+ * retrieved has the same parent
+ */
+ if (adapter.data.fchost.parent &&
+ !checkVhbaSCSIHostParent(conn, name, adapter.data.fchost.parent))
+ retval = -1;
+
VIR_FREE(name);
- return 0;
+ return retval;
}
if (!adapter.data.fchost.parent &&
@@ -704,11 +812,11 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn
ATTRIBUTE_UNUSED,
}
static int
-virStorageBackendSCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED,
+virStorageBackendSCSIStartPool(virConnectPtr conn,
virStoragePoolObjPtr pool)
{
virStoragePoolSourceAdapter adapter = pool->def->source.adapter;
- return createVport(adapter);
+ return createVport(conn, adapter);
}
static int
--
1.9.3