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

Re: [libvirt] [PATCH 07/11] storage_scsi: Translate the stable address into scsi host number



On 20/06/13 00:35, John Ferlan wrote:
On 06/07/2013 01:03 PM, Osier Yang wrote:
This takes use of the two utils introduced in previous patches.

Node device HAL backend represents PCI device like PCI_8086_2922,
(I.E PCI_$vendor_$product), to get the PCI address, we have to
traverse /sys/devices/ to find it out.

And to get the current scsi host number assigned by the system
for the scsi host device. We need to traverse /sys/bus/pci/devices/
with the found PCI address by getScsiHostParentAddress, and the
specified unique_id.

Later patch will allow to omit the "unique_id", by using the
scsi host which has the smallest unique_id.
---
  src/storage/storage_backend_scsi.c | 81 +++++++++++++++++++++++++++++++++++++-
  1 file changed, 79 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index 13c498d..a77b9ae 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -591,6 +591,66 @@ out:
      return retval;
  }
+/*
+ * Node device udev backend and HAL backend represent PCI
+ * device in different style. Udev backend represents it like
+ * pci_0000_00_1f_2, and HAL backend represents it like
+ * pci_8086_2922.
+ *
+ * Covert the value of "parent" into PCI device address string
+ * (e.g. 0000:00:1f:2). Return the PCI address as string on
+ * success, or -1 on failure.
+ */
+static char *
+getScsiHostParentAddress(const char *parent)
+{
+    char **tokens = NULL;
+    char *vendor = NULL;
+    char *product = NULL;
+    char *ret = NULL;
+    int len;
+
+    if (!strchr(parent, '_')) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("'parent' of scsi_host adapter must "
+                         "be consistent with name of node device"));
+        return NULL;
+    }
+
+    if (!(tokens = virStringSplit(parent, "_", 0)))
+        return NULL;
Given the following code, the call to Split could use 4 (or 5) for the
max...  Not that it matters.

hm, it should be 5.


+
+    len = virStringListLength(tokens);
+
+    switch (len) {
+    case 4:
+        if (!(ret = virStringJoin((const char **)(&tokens[1]), ":")))
+            goto cleanup;
+        break;
+
+    case 2:
+        vendor = tokens[1];
+        product = tokens[2];
+        if (!(ret = virFindPCIDeviceByVPD(NULL, vendor, product))) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Unable to find PCI device with vendor '%s' "
+                             "product '%s'"), vendor, product);
+            goto cleanup;
+        }
+
+        break;
+    default:
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("'parent' of scsi_host adapter must "
+                         "be consistent with name of node device"));
+        goto cleanup;
+    }
+
+cleanup:
+    virStringFreeList(tokens);
+    return ret;
+}
+
  static int
  getHostNumber(const char *adapter_name,
                unsigned int *result)
@@ -627,10 +687,27 @@ static char *
  getAdapterName(virStoragePoolSourceAdapter adapter)
  {
      char *name = NULL;
+    char *addr = NULL;
if (adapter.type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
-        if (VIR_STRDUP(name, adapter.data.scsi_host.name) < 0)
-            return NULL;
+        if (adapter.data.scsi_host.parent) {
+            if (!adapter.data.scsi_host.has_unique_id) {
+                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                               _("'unique_id' is not specified for "
+                                 "scsi_host adapter"));
so the optional unique_id isn't so optional after all for a couple of
code paths.  Issuing a message here could be confusing.  Has this been
tested within an environment where unique_id isn't specified?

It's made optional by later patches. Commit log clarifies it.



+                return NULL;
+            } else {
+                if (!(addr = getScsiHostParentAddress(adapter.data.scsi_host.parent)))
+                    return NULL;
+
+                name = virParseStableScsiHostAddress(NULL, addr,
+                                                     adapter.data.scsi_host.unique_id);
You need a VIR_FREE(addr); here

John
+            }
+        } else if (adapter.data.scsi_host.name) {
+            if (VIR_STRDUP(name, adapter.data.scsi_host.name) < 0)
+                return NULL;
+        }
+
          return name;
      }


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