[libvirt] [PATCH v2 1/3] storage: Fix existing parent check for vHBA creation

[John Ferlan]

https://bugzilla.redhat.com/show_bug.cgi?id=1472277

Commit id '106930aaa' altered the order of checking for an existing
vHBA (e.g something created via nodedev-create functionality outside
of the storage pool logic) which inadvertantly broke the code to
decide whether to alter/force the fchost->managed field to be 'yes'
because the storage pool will be managing the created vHBA in order
to ensure when the storage pool is destroyed that the vHBA is also
destroyed.

This patch moves the check (and checkParent helper) for an existing
vHBA back into the createVport in storage_backend_scsi. It also
restores the logic back to what commit id '79ab0935' had with
respect to using "if (fchost->parent && !checkParent(...))" and
returns immediately. The changes made by commit id '08c0ea16f'
are only necessary to run the virStoragePoolFCRefreshThread when
a vHBA was really created because there's a timing lag such that
the refreshPool call made after a startPool from storagePoolCreate*
wouldn't necessarily find LUNs, but the thread would. For an already
existing vHBA, using the thread is unnecessary since the vHBA already
exists and the lag to configure the LUNs wouldn't exist.

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 src/conf/node_device_conf.c        | 55 ------------------------------------
 src/storage/storage_backend_scsi.c | 57 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 55 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 503b129..9c0ffa5 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2259,48 +2259,6 @@ virNodeDeviceGetParentName(virConnectPtr conn,
 }
 
 
-/*
- * 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
-checkParent(virConnectPtr conn,
-            const char *name,
-            const char *parent_name)
-{
-    char *scsi_host_name = NULL;
-    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 (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0)
-        goto cleanup;
-
-    if (!(vhba_parent = virNodeDeviceGetParentName(conn, scsi_host_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);
-    VIR_FREE(scsi_host_name);
-    return retval;
-}
-
-
 /**
  * @conn: Connection pointer
  * @fchost: Pointer to vHBA adapter
@@ -2326,19 +2284,6 @@ virNodeDeviceCreateVport(virConnectPtr conn,
     VIR_DEBUG("conn=%p, parent='%s', wwnn='%s' wwpn='%s'",
               conn, NULLSTR(fchost->parent), fchost->wwnn, fchost->wwpn);
 
-    /* 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 = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) {
-        /* If a parent was provided, let's make sure the 'name' we've
-         * retrieved has the same parent. If not this will cause failure. */
-        if (fchost->parent && checkParent(conn, name, fchost->parent))
-            VIR_FREE(name);
-
-        return name;
-    }
-
     if (fchost->parent) {
         if (VIR_STRDUP(parent_hoststr, fchost->parent) < 0)
             goto cleanup;
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index f7378d3..aa0fb99 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -211,6 +211,48 @@ getAdapterName(virStorageAdapterPtr adapter)
 }
 
 
+/*
+ * 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
+checkParent(virConnectPtr conn,
+            const char *name,
+            const char *parent_name)
+{
+    char *scsi_host_name = NULL;
+    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 (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0)
+        goto cleanup;
+
+    if (!(vhba_parent = virNodeDeviceGetParentName(conn, scsi_host_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);
+    VIR_FREE(scsi_host_name);
+    return retval;
+}
+
+
 static int
 createVport(virConnectPtr conn,
             virStoragePoolDefPtr def,
@@ -226,6 +268,21 @@ createVport(virConnectPtr conn,
               conn, NULLSTR(configFile), NULLSTR(fchost->parent),
               fchost->wwnn, fchost->wwpn);
 
+    /* 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 = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) {
+        ret = 0;
+
+        /* If a parent was provided, let's make sure the 'name' we've
+         * retrieved has the same parent. If not this will cause failure. */
+        if (fchost->parent && !checkParent(conn, name, fchost->parent))
+            ret = -1;
+
+        VIR_FREE(name);
+        return ret;
+    }
 
     /* Since we're creating the vHBA, then we need to manage removing it
      * as well. Since we need this setting to "live" through a libvirtd
-- 
2.9.4