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

Re: [libvirt] [PATCH 1/1] Add SCSI pool support.



Daniel P. Berrange wrote:

[snipped code & comments that I'm not addressing here]

+static int
+virStorageBackendSCSIFindLUs(virConnectPtr conn,
+                             virStoragePoolObjPtr pool,
+                             uint32_t scanhost,
+                             uint32_t scanbus,
+                             uint32_t scantarget)
+{
+    int retval = 0;
+    uint32_t host, bus, target, lun;
+    char *target_path = NULL;
+    DIR *targetdir = NULL;
+    struct dirent *lun_dirent = NULL;
+
+    VIR_DEBUG(_("Discovering LUs on host %u bus %u target %u"),
+              scanhost, scanbus, scantarget);
+
+    if (virAsprintf(&target_path, "/sys/bus/scsi/devices/target%u:%u:%u",
+             scanhost, scanbus, scantarget) < 0) {
+        virReportOOMError(conn);
+        goto out;
+    }

This unfortauntely does not work on RHEL5, because there are no targetX.X.X
links here. Only the LUNs appear in this directory.

The location which appears to be present on both old and new kernels is
under:

   /sys/class/scsi_host/host0/device/targetX.X.X

This appears to be present for both SCSI and iSCSI hosts.

Unfortunately, that directory isn't present for FC hosts on F10. :( I've put together a scan strategy that I think will for for both RHEL5 and F10 for all transport types. Let me know if it works for you. The attached patch should be applied on top of the udev fix I sent yesterday. This patch is another one that doesn't read well as a patch, but should be pretty straightforward once it's applied.

I have not addressed the question of what device types to allow, because I want to think about that one a bit more.

I also took out the unused struct you noticed.

Thanks for all the feedback and testing.

Dave
>From c749c3cc3d90cf9ae31db0c68fd41b7ce24c2bf3 Mon Sep 17 00:00:00 2001
From: David Allan <dallan redhat com>
Date: Fri, 27 Mar 2009 17:48:18 -0400
Subject: [PATCH 1/1] Changes based on feedback from DanPB

* Fixed LU scan logic so that it now hopefully works for all transport types on both RHEL5 and F10.  The preceeding version didn't work on RHEL5.
* Removed the unused struct from storage_backend_scsi.h
---
 src/storage_backend_iscsi.c |   20 ++++++++++----
 src/storage_backend_scsi.c  |   61 +++++++++++++++++++-----------------------
 src/storage_backend_scsi.h  |   24 +++++------------
 3 files changed, 49 insertions(+), 56 deletions(-)

diff --git a/src/storage_backend_iscsi.c b/src/storage_backend_iscsi.c
index 9da7cdc..b516add 100644
--- a/src/storage_backend_iscsi.c
+++ b/src/storage_backend_iscsi.c
@@ -172,23 +172,31 @@ virStorageBackendISCSIConnection(virConnectPtr conn,
 
 
 static int
-virStorageBackendISCSIFindLUNs(virConnectPtr conn,
-                               virStoragePoolObjPtr pool,
-                               const char *session)
+virStorageBackendISCSIFindLUs(virConnectPtr conn,
+                              virStoragePoolObjPtr pool,
+                              const char *session)
 {
     char sysfs_path[PATH_MAX];
     int retval = 0;
+    uint32_t host;
 
     snprintf(sysfs_path, PATH_MAX,
              "/sys/class/iscsi_session/session%s/device", session);
 
-    if (virStorageBackendSCSIFindTargets(conn, pool, sysfs_path, "target") < 0) {
+    if (virStorageBackendSCSIGetHostNumber(conn, sysfs_path, &host) < 0) {
         virReportSystemError(conn, errno,
-                             _("Failed to get target list for path '%s'"),
+                             _("Failed to get host number for iSCSI session "
+                               "with path '%s'"),
                              sysfs_path);
         retval = -1;
     }
 
+    if (virStorageBackendSCSIFindLUs(conn, pool, host) < 0) {
+        virReportSystemError(conn, errno,
+                             _("Failed to find LUs on host %u"), host);
+        retval = -1;
+    }
+
     return retval;
 }
 
@@ -302,7 +310,7 @@ virStorageBackendISCSIRefreshPool(virConnectPtr conn,
         goto cleanup;
     if (virStorageBackendISCSIRescanLUNs(conn, pool, session) < 0)
         goto cleanup;
-    if (virStorageBackendISCSIFindLUNs(conn, pool, session) < 0)
+    if (virStorageBackendISCSIFindLUs(conn, pool, session) < 0)
         goto cleanup;
     VIR_FREE(session);
 
diff --git a/src/storage_backend_scsi.c b/src/storage_backend_scsi.c
index e0ced8f..2c5168a 100644
--- a/src/storage_backend_scsi.c
+++ b/src/storage_backend_scsi.c
@@ -382,68 +382,67 @@ out:
 }
 
 
-static int
+int
 virStorageBackendSCSIFindLUs(virConnectPtr conn,
                              virStoragePoolObjPtr pool,
-                             uint32_t scanhost,
-                             uint32_t scanbus,
-                             uint32_t scantarget)
+                             uint32_t scanhost)
 {
     int retval = 0;
-    uint32_t host, bus, target, lun;
-    char *target_path = NULL;
-    DIR *targetdir = NULL;
+    uint32_t bus, target, lun;
+    char *device_path = NULL;
+    DIR *devicedir = NULL;
     struct dirent *lun_dirent = NULL;
+    char devicepattern[64];
+
+    VIR_DEBUG(_("Discovering LUs on host %u"), scanhost);
 
-    VIR_DEBUG(_("Discovering LUs on host %u bus %u target %u"),
-              scanhost, scanbus, scantarget);
+    virStorageBackendWaitForDevices(conn);
 
-    if (virAsprintf(&target_path, "/sys/bus/scsi/devices/target%u:%u:%u",
-             scanhost, scanbus, scantarget) < 0) {
+    if (virAsprintf(&device_path, "/sys/bus/scsi/devices") < 0) {
         virReportOOMError(conn);
         goto out;
     }
 
-    targetdir = opendir(target_path);
+    devicedir = opendir(device_path);
 
-    if (targetdir == NULL) {
+    if (devicedir == NULL) {
         virReportSystemError(conn, errno,
-                             _("Failed to opendir path '%s'"), target_path);
+                             _("Failed to opendir path '%s'"), device_path);
         retval = -1;
         goto out;
     }
 
-    while ((lun_dirent = readdir(targetdir))) {
-        if (sscanf(lun_dirent->d_name, "%u:%u:%u:%u\n",
-                   &host, &bus, &target, &lun) != 4) {
+    snprintf(devicepattern, sizeof(devicepattern), "%u:%%u:%%u:%%u\n", scanhost);
+
+    while ((lun_dirent = readdir(devicedir))) {
+        if (sscanf(lun_dirent->d_name, devicepattern,
+                   &bus, &target, &lun) != 3) {
             continue;
         }
 
         VIR_DEBUG(_("Found LU '%s'"), lun_dirent->d_name);
 
-        processLU(conn, pool, host, bus, target, lun);
+        processLU(conn, pool, scanhost, bus, target, lun);
     }
 
-    closedir(targetdir);
+    closedir(devicedir);
 
 out:
-    VIR_FREE(target_path);
+    VIR_FREE(device_path);
     return retval;
 }
 
 
 int
-virStorageBackendSCSIFindTargets(virConnectPtr conn,
-                                 virStoragePoolObjPtr pool,
-                                 const char *sysfs_path,
-                                 const char *pattern)
+virStorageBackendSCSIGetHostNumber(virConnectPtr conn,
+                                   const char *sysfs_path,
+                                   uint32_t *host)
 {
     int retval = 0;
-    uint32_t host, bus, target;
     DIR *sysdir = NULL;
     struct dirent *dirent = NULL;
 
-    VIR_DEBUG(_("Discovering targets in '%s'"), sysfs_path);
+    VIR_DEBUG(_("Finding host number from '%s'"), sysfs_path);
 
     virStorageBackendWaitForDevices(conn);
 
@@ -457,14 +456,13 @@ virStorageBackendSCSIFindTargets(virConnectPtr conn,
     }
 
     while ((dirent = readdir(sysdir))) {
-        if (STREQLEN(dirent->d_name, pattern, strlen(pattern))) {
+        if (STREQLEN(dirent->d_name, "target", strlen("target"))) {
             if (sscanf(dirent->d_name,
-                       "target%u:%u:%u", &host, &bus, &target) != 3) {
+                       "target%u:", host) != 1) {
                 VIR_DEBUG(_("Failed to parse target '%s'"), dirent->d_name);
                 retval = -1;
                 break;
             }
-            virStorageBackendSCSIFindLUs(conn, pool, host, bus, target);
         }
     }
 
@@ -478,7 +476,6 @@ static int
 virStorageBackendSCSIRefreshPool(virConnectPtr conn,
                                  virStoragePoolObjPtr pool)
 {
-    char targetN[64];
     int retval = 0;
     uint32_t host;
 
@@ -492,9 +489,7 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn,
 
     VIR_DEBUG(_("Scanning host%u"), host);
 
-    snprintf(targetN, sizeof(targetN), "target%u", host);
-
-    virStorageBackendSCSIFindTargets(conn, pool, "/sys/bus/scsi/devices", targetN);
+    virStorageBackendSCSIFindLUs(conn, pool, host);
 
 out:
     return retval;
diff --git a/src/storage_backend_scsi.h b/src/storage_backend_scsi.h
index 678ccd6..d63951d 100644
--- a/src/storage_backend_scsi.h
+++ b/src/storage_backend_scsi.h
@@ -30,25 +30,15 @@
 #define LINUX_SYSFS_SCSI_HOST_PREFIX "/sys/class/scsi_host"
 #define LINUX_SYSFS_SCSI_HOST_POSTFIX "device"
 
-struct _virDirectoryWalkData {
-    virConnectPtr conn;
-    virStoragePoolObjPtr pool;
-    const char *dir_path;
-    const char *pattern_to_match;
-    size_t expected_matches;
-    virHashTablePtr matches; // will be created by walk function
-    virHashIterator iterator;
-    virHashDeallocator deallocator;
-};
-typedef struct _virDirectoryWalkData virDirectoryWalkData;
-typedef virDirectoryWalkData *virDirectoryWalkDataPtr;
-
 extern virStorageBackend virStorageBackendSCSI;
 
 int
-virStorageBackendSCSIFindTargets(virConnectPtr conn,
-                                 virStoragePoolObjPtr pool,
-                                 const char *sysfs_path,
-                                 const char *pattern);
+virStorageBackendSCSIGetHostNumber(virConnectPtr conn,
+                                   const char *sysfs_path,
+                                   uint32_t *host);
+int
+virStorageBackendSCSIFindLUs(virConnectPtr conn,
+                             virStoragePoolObjPtr pool,
+                             uint32_t scanhost);
 
 #endif /* __VIR_STORAGE_BACKEND_SCSI_H__ */
-- 
1.6.0.6


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