Re: [PATCH 04/11] nodedev: Add the ability to create vHBA by parent wwnn/wwpn or fabric_wwn

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

 



John,
I have a concern regarding usage of the parent_fabric_name.
As far I have been told there are a lot of fc_host attributes in Linux that are optional and left to the low-level driver to decide if implemented. fabric_name apparently happens to be one of these attributes and I know an architecture that has a driver (zfcp) that does not provide it.

Up until this patch the fabric_name was collected on the host during detection of the fc_host capability and provided during a nodedev dumpxml. I would like to propose making the existence of fabric_name option in the detection of the fc_host capability. In doing so it would also be required make the fabric_name xml tag optional in the xml of the nodedev dump. I am aware that this is a creating a slightly incompatible output but how else could that be fixed than making the tag optional? I can send a patch for this if you agree to the solution.

The implications regarding the search by parent_fabric_wwn should be obvious.

On 11/18/2016 03:26 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1349696

When creating a vHBA, the process is to feed XML to nodeDeviceCreateXML
that lists the <parent> scsi_hostX to use to create the vHBA. However,
between reboots, it's possible that the <parent> changes its scsi_hostX
to scsi_hostY and saved XML to perform the creation will either fail or
create a vHBA using the wrong parent.

So add the ability to provide <parent_wwnn> and <parent_wwpn> or
<parent_fabric_wwn> in order to use those values as more consistent
search parameters. For some providing the wwnn/wwpn pair will provide
the most specific search option, while for others providing a fabric_wwn
will at least ensure usage of the same SAN.

This patch will add the new fields to the nodedev.rng for input purposes
only since the input XML is essentially thrown away, no need to Format
the values since they'd already be printed as part of the scsi_host
data block.

New API virNodeDeviceGetParentHostByWWNs will take the parent_wwnn and
parent_wwpn in order to search the list of devices for matching capability
data fields wwnn and wwpn.

New API virNodeDeviceGetParentHostByFabricWWN will take the parent_fabric_wwn
in order to search the list of devices for matching capability data field
fabric_wwn.

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
 docs/schemas/nodedev.rng             |  15 +++++
 src/conf/node_device_conf.c          | 120 +++++++++++++++++++++++++++++++++++
 src/conf/node_device_conf.h          |  14 ++++
 src/libvirt_private.syms             |   2 +
 src/node_device/node_device_driver.c |  13 ++++
 5 files changed, 164 insertions(+)

diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index 93a88d8..6fe49a3 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -18,6 +18,21 @@
       <optional>
         <element name="parent"><text/></element>
       </optional>
+      <optional>
+        <element name="parent_wwnn">
+          <ref name='wwn'/>
+        </element>
+      </optional>
+      <optional>
+        <element name="parent_wwpn">
+          <ref name='wwn'/>
+        </element>
+      </optional>
+      <optional>
+        <element name="parent_fabric_wwn">
+          <ref name='wwn'/>
+        </element>
+      </optional>

       <optional>
         <element name="driver">
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 3aa77cf..cecd915 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -92,6 +92,30 @@ int virNodeDeviceHasCap(const virNodeDeviceObj *dev, const char *cap)
 }


+/* virNodeDeviceFindFCCapDef:
+ * @dev: Pointer to current device
+ *
+ * Search the device object 'caps' array for fc_host capability.
+ *
+ * Returns:
+ * Pointer to the caps or NULL if not found
+ */
+static virNodeDevCapsDefPtr
+virNodeDeviceFindFCCapDef(const virNodeDeviceObj *dev)
+{
+    virNodeDevCapsDefPtr caps = dev->def->caps;
+
+    while (caps) {
+        if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST &&
+            (caps->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST))
+            break;
+
+        caps = caps->next;
+    }
+    return caps;
+}
+
+
 /* virNodeDeviceFindVPORTCapDef:
  * @dev: Pointer to current device
  *
@@ -152,6 +176,46 @@ virNodeDeviceObjPtr virNodeDeviceFindByName(virNodeDeviceObjListPtr devs,


 static virNodeDeviceObjPtr
+virNodeDeviceFindByWWNs(virNodeDeviceObjListPtr devs,
+                        const char *parent_wwnn,
+                        const char *parent_wwpn)
+{
+    size_t i;
+
+    for (i = 0; i < devs->count; i++) {
+        virNodeDevCapsDefPtr cap;
+        virNodeDeviceObjLock(devs->objs[i]);
+        if ((cap = virNodeDeviceFindFCCapDef(devs->objs[i])) &&
+            STREQ_NULLABLE(cap->data.scsi_host.wwnn, parent_wwnn) &&
+            STREQ_NULLABLE(cap->data.scsi_host.wwpn, parent_wwpn))
+            return devs->objs[i];
+        virNodeDeviceObjUnlock(devs->objs[i]);
+    }
+
+    return NULL;
+}
+
+
+static virNodeDeviceObjPtr
+virNodeDeviceFindByFabricWWN(virNodeDeviceObjListPtr devs,
+                             const char *parent_fabric_wwn)
+{
+    size_t i;
+
+    for (i = 0; i < devs->count; i++) {
+        virNodeDevCapsDefPtr cap;
+        virNodeDeviceObjLock(devs->objs[i]);
+        if ((cap = virNodeDeviceFindFCCapDef(devs->objs[i])) &&
+            STREQ_NULLABLE(cap->data.scsi_host.fabric_wwn, parent_fabric_wwn))
+            return devs->objs[i];
+        virNodeDeviceObjUnlock(devs->objs[i]);
+    }
+
+    return NULL;
+}
+
+
+static virNodeDeviceObjPtr
 virNodeDeviceFindByCap(virNodeDeviceObjListPtr devs,
                        const char *cap)
 {
@@ -177,6 +241,9 @@ void virNodeDeviceDefFree(virNodeDeviceDefPtr def)

     VIR_FREE(def->name);
     VIR_FREE(def->parent);
+    VIR_FREE(def->parent_wwnn);
+    VIR_FREE(def->parent_wwpn);
+    VIR_FREE(def->parent_fabric_wwn);
     VIR_FREE(def->driver);
     VIR_FREE(def->sysfs_path);
     VIR_FREE(def->parent_sysfs_path);
@@ -1652,6 +1719,10 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,

     /* Extract device parent, if any */
     def->parent = virXPathString("string(./parent[1])", ctxt);
+    def->parent_wwnn = virXPathString("string(./parent_wwnn[1])", ctxt);
+    def->parent_wwpn = virXPathString("string(./parent_wwpn[1])", ctxt);
+    def->parent_fabric_wwn = virXPathString("string(./parent_fabric_wwn[1])",
+                                            ctxt);

     /* Parse device capabilities */
     nodes = NULL;
@@ -1847,6 +1918,55 @@ virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs,


 int
+virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs,
+                                 const char *dev_name,
+                                 const char *parent_wwnn,
+                                 const char *parent_wwpn,
+                                 int *parent_host)
+{
+    virNodeDeviceObjPtr parent = NULL;
+    int ret;
+
+    if (!(parent = virNodeDeviceFindByWWNs(devs, parent_wwnn, parent_wwpn))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Could not find parent device for '%s'"),
+                       dev_name);
+        return -1;
+    }
+
+    ret = virNodeDeviceFindFCParentHost(parent, parent_host);
+
+    virNodeDeviceObjUnlock(parent);
+
+    return ret;
+}
+
+
+int
+virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs,
+                                      const char *dev_name,
+                                      const char *parent_fabric_wwn,
+                                      int *parent_host)
+{
+    virNodeDeviceObjPtr parent = NULL;
+    int ret;
+
+    if (!(parent = virNodeDeviceFindByFabricWWN(devs, parent_fabric_wwn))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Could not find parent device for '%s'"),
+                       dev_name);
+        return -1;
+    }
+
+    ret = virNodeDeviceFindFCParentHost(parent, parent_host);
+
+    virNodeDeviceObjUnlock(parent);
+
+    return ret;
+}
+
+
+int
 virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs,
                                  int *parent_host)
 {
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 2b2aed7..1634483 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -200,6 +200,9 @@ struct _virNodeDeviceDef {
     char *sysfs_path;                   /* udev name/sysfs path */
     char *parent;			/* optional parent device name */
     char *parent_sysfs_path;            /* udev parent name/sysfs path */
+    char *parent_wwnn;			/* optional parent wwnn */
+    char *parent_wwpn;			/* optional parent wwpn */
+    char *parent_fabric_wwn;		/* optional parent fabric_wwn */
     char *driver;                       /* optional driver name */
     virNodeDevCapsDefPtr caps;		/* optional device capabilities */
 };
@@ -273,6 +276,17 @@ int virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs,
                                const char *parent_name,
                                int *parent_host);

+int virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs,
+                                     const char *dev_name,
+                                     const char *parent_wwnn,
+                                     const char *parent_wwpn,
+                                     int *parent_host);
+
+int virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs,
+                                          const char *dev_name,
+                                          const char *parent_fabric_wwn,
+                                          int *parent_host);
+
 int virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs,
                                      int *parent_host);

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index de14a7e..5673bda 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -701,6 +701,8 @@ virNodeDeviceFindByName;
 virNodeDeviceFindBySysfsPath;
 virNodeDeviceFindVportParentHost;
 virNodeDeviceGetParentHost;
+virNodeDeviceGetParentHostByFabricWWN;
+virNodeDeviceGetParentHostByWWNs;
 virNodeDeviceGetWWNs;
 virNodeDeviceHasCap;
 virNodeDeviceObjListExport;
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 0e091fe..b7bec65 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -590,6 +590,19 @@ nodeDeviceCreateXML(virConnectPtr conn,
                                        def->parent,
                                        &parent_host) < 0)
             goto cleanup;
+    } else if (def->parent_wwnn && def->parent_wwpn) {
+        if (virNodeDeviceGetParentHostByWWNs(&driver->devs,
+                                             def->name,
+                                             def->parent_wwnn,
+                                             def->parent_wwpn,
+                                             &parent_host) < 0)
+            goto cleanup;
+    } else if (def->parent_fabric_wwn) {
+        if (virNodeDeviceGetParentHostByFabricWWN(&driver->devs,
+                                                  def->name,
+                                                  def->parent_fabric_wwn,
+                                                  &parent_host) < 0)
+            goto cleanup;
     } else {
         /* Try to find "a" vport capable scsi_host when no parent supplied */
         if (virNodeDeviceFindVportParentHost(&driver->devs, &parent_host) < 0)



--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux