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>

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;
 }
 
 

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);
 

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 (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 (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);
         }
     }