conf: Return the vHBA name from virNodeDeviceCreateVport

Rather than returning true/false and having the caller check if the
vHBA was actually created, let's do that check within the CreateVport
function. That way the caller can faithfully assume success based
on a name start the thread looking for the LUNs. Prior to this change
it's possible that the vHBA wasn't really created (e.g if the call to
virVHBAGetHostByWWN returned NULL), we'd claim success, but in reality
there'd be no vHBA for the pool. This also fixes a second yet seen
issue that if the nodedev was present, but the parent by name wasn't
provided (perhaps parent by wwnn/wwpn or by fabric_name), then a failure
would be returned. For this path it shouldn't be an error - we should
just be happy that something else is managing the device and we don't
have to create/delete it.

The end result is that the createVport code can now just start the
refresh thread once it gets a non NULL name back.

Signed-off-by: John Ferlan <jferlan@redhat.com>

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 0cf4214d3b869551176a8c719b806131be02d7c2..ba727468e8b5430def2c14f89c9922e78a39b978 100644 (file)
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1925,13 +1925,12 @@ checkParent(virConnectPtr conn,
  * wwnn/wwpn, or fabric_wwn (if provided). If no parent is provided, then
  * a vport capable scsi_host will be selected.
  *
- * Returns 0 on success, -1 on failure
+ * Returns vHBA name on success, NULL on failure with an error message set
  */
-int
+char *
 virNodeDeviceCreateVport(virConnectPtr conn,
                          virStorageAdapterFCHostPtr fchost)
 {
-    int ret = -1;
     unsigned int parent_host;
     char *name = NULL;
     char *parent_hoststr = NULL;
@@ -1948,9 +1947,9 @@ virNodeDeviceCreateVport(virConnectPtr conn,
         /* 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 = 0;
+            VIR_FREE(name);
 
-        goto cleanup;
+        return name;
     }
 
     if (fchost->parent) {
@@ -2002,12 +2001,17 @@ virNodeDeviceCreateVport(virConnectPtr conn,
                            VPORT_CREATE) < 0)
         goto cleanup;
 
-    ret = 0;
+    /* Let's ensure the device was created */
+    virWaitForDevices();
+    if (!(name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) {
+        ignore_value(virVHBAManageVport(parent_host, fchost->wwpn, fchost->wwnn,
+                                        VPORT_DELETE));
+        goto cleanup;
+    }
 
  cleanup:
-    VIR_FREE(name);
     VIR_FREE(parent_hoststr);
-    return ret;
+    return name;
 }
 
 

diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 82e988afa0b838b9a27ec90471338f33f74a3c0e..a5d5cdd2a541c92fc7e709816a53c1d36dcf3235 100644 (file)
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -357,7 +357,7 @@ char *
 virNodeDeviceGetParentName(virConnectPtr conn,
                            const char *nodedev_name);
 
-int
+char *
 virNodeDeviceCreateVport(virConnectPtr conn,
                          virStorageAdapterFCHostPtr fchost);
 

diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index e406923ebad4b86dd736eabe46119f14a17c8c3a..f7378d34b00c3fb1517792250156e5e2374b8820 100644 (file)
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -240,27 +240,23 @@ createVport(virConnectPtr conn,
         }
     }
 
-    if (virNodeDeviceCreateVport(conn, fchost) < 0)
+    if (!(name = virNodeDeviceCreateVport(conn, fchost)))
         goto cleanup;
 
-    virWaitForDevices();
-
     /* Creating our own VPORT didn't leave enough time to find any LUN's,
      * so, let's create a thread whose job it is to call the FindLU's with
      * retry logic set to true. If the thread isn't created, then no big
      * deal since it's still possible to refresh the pool later.
      */
-    if ((name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) {
-        if (VIR_ALLOC(cbdata) == 0) {
-            memcpy(cbdata->pool_uuid, def->uuid, VIR_UUID_BUFLEN);
-            VIR_STEAL_PTR(cbdata->fchost_name, name);
-
-            if (virThreadCreate(&thread, false, virStoragePoolFCRefreshThread,
-                                cbdata) < 0) {
-                /* Oh well - at least someone can still refresh afterwards */
-                VIR_DEBUG("Failed to create FC Pool Refresh Thread");
-                virStoragePoolFCRefreshDataFree(cbdata);
-            }
+    if (VIR_ALLOC(cbdata) == 0) {
+        memcpy(cbdata->pool_uuid, def->uuid, VIR_UUID_BUFLEN);
+        VIR_STEAL_PTR(cbdata->fchost_name, name);
+
+        if (virThreadCreate(&thread, false, virStoragePoolFCRefreshThread,
+                            cbdata) < 0) {
+            /* Oh well - at least someone can still refresh afterwards */
+            VIR_DEBUG("Failed to create FC Pool Refresh Thread");
+            virStoragePoolFCRefreshDataFree(cbdata);
         }
     }