[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 6/8] util: Alter virNodeDevice{Create|Delete}Vport to use nodedev APIs

On 02/20/2017 08:18 AM, John Ferlan wrote:
If we have a connection pointer there's no sense walking through the
sysfs in order to create/destroy the vHBA. Instead, let's make use of
the node device create/destroy API's.

Since we don't have to rewrite all the various parent options for
the test driver in order to test whether the storage pool creation
works as the node device creation has been tested already, let's just
use the altered API to test the storage pool paths.

Fix a "bug" in the storage pool test driver code which "assumed"
testStoragePoolObjSetDefaults should fill in the configFile for
both the Define/Create (persistent) and CreateXML (transient) pools
by just VIR_FREE() of the pool during CreateXML.  Because the
configFile was filled in, during Destroy, the pool wouldn't be
free causing a test using the same name pool to fail.

Signed-off-by: John Ferlan <jferlan redhat com>
  src/conf/node_device_conf.c | 121 ++++++++++++++++++++++++++++++++++++++++++++
  src/test/test_driver.c      |   6 +++
  tests/fchosttest.c          |  15 ++++++
  3 files changed, 142 insertions(+)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 68ed5ad..1d76096 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2390,6 +2390,82 @@ nodeDeviceCheckParent(virConnectPtr conn,
   * @conn: Connection pointer
+ * @fchost: Pointer to the vHBA adapter
+ *
+ * If we have a valid connection, then use the node device create
+ * XML API rather than traversing through the sysfs to create the vHBA.
+ * Generate the Node Device XML using the Storage vHBA Adapter providing
+ * either the parent name, parent wwnn/wwpn, or parent fabric_name if
+ * available to the Node Device code.  Since the Storage XML processing
+ * requires the wwnn/wwpn to be used for the vHBA to be provided (and
+ * not generated), we can use that as the fc_host wwnn/wwpn. This will
+ * allow for easier search later when we need it.
+ *
+ * Returns vHBA name on success, NULL on failure with an error message set
+ */
+static char *
+nodeDeviceCreateNodeDeviceVport(virConnectPtr conn,
+                                virStorageAdapterFCHostPtr fchost)
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    char *vhbaStr = NULL;
+    virNodeDevicePtr dev = NULL;
+    char *name;
+    bool created = false;
+    /* If the nodedev already knows about this vHBA, then we're not
+     * managing it - we'll just use it. */
+    if ((dev = virNodeDeviceLookupSCSIHostByWWN(conn, fchost->wwnn,
+                                                fchost->wwpn, 0)))

I get nervous anytime I see a call to a toplevel public libvirt API inside some low level function even when it's in one of the driver directories. But this is in the conf directory. Is there a precedent for doing that? It doesn't seem like a good idea - if it's called from within any other nodedev function it could end up in a deadlock of the driver-wide lock.

Is there some other organization that could make this cleaner? (haven't thought about what yet)

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]