[libvirt] [RFC 5/5]: Rewrite findLuns function

[Chris Lalancette <clalance redhat com>]

This rather large patch rewrites the virStorageBackendISCSIFindLUNs() function
to only rely on sysfs for finding LUNs, given a session number.  Along the way,
it also fixes the bug where we wouldn't find LUNs for older kernels (with the
block:sda format), and also (possibly) fixes a race condition where we could try
to find the LUN before udev has finished connecting it.  I say it "possibly"
fixes it because I haven't been able to hit it so far, but I definitely need
more testing to try and confirm.

Signed-off-by: Chris Lalancette <clalance redhat com>

--- libvirt.sendtarget/src/storage_backend_iscsi.c	2008-06-12 15:31:00.000000000 +0200
+++ libvirt/src/storage_backend_iscsi.c	2008-06-12 15:43:39.000000000 +0200
@@ -164,110 +164,30 @@ virStorageBackendISCSIConnection(virConn
     return 0;
 }
 
-
 static int
-virStorageBackendISCSIMakeLUN(virConnectPtr conn,
-                              virStoragePoolObjPtr pool,
-                              char **const groups,
-                              void *data)
+virStorageBackendISCSINewLun(virConnectPtr conn, virStoragePoolObjPtr pool,
+							 unsigned int lun, char *dev)
 {
     virStorageVolDefPtr vol;
     int fd = -1;
-    unsigned int target, channel, id, lun;
-    char lunid[100];
-    int opentries = 0;
     char *devpath = NULL;
-    char *session = data;
-    char sysfs_path[PATH_MAX];
-    char *dev = NULL;
-    DIR *sysdir;
-    struct dirent *block_dirent;
-    struct stat sbuf;
-    int len;
-
-    if ((virStrToLong_ui(groups[0], NULL, 10, &target) < 0) ||
-        (virStrToLong_ui(groups[1], NULL, 10, &channel) < 0) ||
-        (virStrToLong_ui(groups[2], NULL, 10, &id) < 0) ||
-        (virStrToLong_ui(groups[3], NULL, 10, &lun) < 0)) {
-        virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
-                              _("Failed parsing iscsiadm commands"));
-        return -1;
-    }
-
-    if (lun == 0) {
-        /* the 0'th LUN isn't a real LUN, it's just a control LUN; skip it */
-        return 0;
-    }
-
-    snprintf(sysfs_path, PATH_MAX,
-             "/sys/class/iscsi_session/session%s/device/"
-             "target%d:%d:%d/%d:%d:%d:%d/block",
-             session, target, channel, id, target, channel, id, lun);
-
-    if (stat(sysfs_path, &sbuf) < 0) {
-        /* block path in subdir didn't exist; this is unexpected, so fail */
-        virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                              _("Failed to find the sysfs path for %d:%d:%d:%d: %s"),
-                              target, channel, id, lun, strerror(errno));
-        return -1;
-    }
-
-    sysdir = opendir(sysfs_path);
-    if (sysdir == NULL) {
-        /* we failed for some reason; return an error */
-        virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                              _("Failed to opendir sysfs path %s: %s"),
-                              sysfs_path, strerror(errno));
-        return -1;
-    }
-
-    while ((block_dirent = readdir(sysdir)) != NULL) {
-        len = strlen(block_dirent->d_name);
-        if ((len == 1 && block_dirent->d_name[0] == '.') ||
-            (len == 2 && block_dirent->d_name[0] == '.' && block_dirent->d_name[1] == '.')) {
-            /* the . and .. directories; just skip them */
-            continue;
-        }
-
-        /* OK, not . or ..; let's see if it is a SCSI device */
-        if (len > 2 &&
-            block_dirent->d_name[0] == 's' &&
-            block_dirent->d_name[1] == 'd') {
-            /* looks like a scsi device, smells like scsi device; it must be
-               a scsi device */
-            dev = strdup(block_dirent->d_name);
-            break;
-        }
-    }
-    closedir(sysdir);
-
-    if (dev == NULL) {
-        /* we didn't find the sd? device we were looking for; fail */
-        virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                              _("Failed to find SCSI device for %d:%d:%d:%d: %s"),
-                              target, channel, id, lun, strerror(errno));
-        return -1;
-    }
-
-    snprintf(lunid, sizeof(lunid)-1, "lun-%s", groups[3]);
+	int opentries = 0;
 
     if (VIR_ALLOC(vol) < 0) {
         virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("volume"));
         goto cleanup;
     }
 
-    if ((vol->name = strdup(lunid)) == NULL) {
+	if (asprintf(&(vol->name), "lun-%d", lun) < 0) {
         virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("name"));
         goto cleanup;
     }
 
-    if (VIR_ALLOC_N(devpath, 5 + strlen(dev) + 1) < 0) {
+	if (asprintf(&devpath, "/dev/%s", dev) < 0) {
         virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("devpath"));
         goto cleanup;
-    }
-    strcpy(devpath, "/dev/");
-    strcat(devpath, dev);
-    VIR_FREE(dev);
+	}
+
     /* It can take a little while between logging into the ISCSI
      * server and udev creating the /dev nodes, so if we get ENOENT
      * we must retry a few times - they should eventually appear.
@@ -312,7 +232,6 @@ virStorageBackendISCSIMakeLUN(virConnect
         goto cleanup;
     }
 
-
     pool->def->capacity += vol->capacity;
     pool->def->allocation += vol->allocation;
 
@@ -328,51 +247,183 @@ virStorageBackendISCSIMakeLUN(virConnect
     if (fd != -1) close(fd);
     VIR_FREE(devpath);
     virStorageVolDefFree(vol);
-    VIR_FREE(dev);
     return -1;
 }
 
+static int notdotdir(const struct dirent *dir)
+{
+	return !(STREQLEN(dir->d_name, ".", 1) || STREQLEN(dir->d_name, "..", 2));
+}
+
 static int
 virStorageBackendISCSIFindLUNs(virConnectPtr conn,
                                virStoragePoolObjPtr pool,
                                const char *session)
 {
-    /*
-     * # iscsiadm --mode session -r $session -P 3
-     *
-     *           scsi1 Channel 00 Id 0 Lun: 0
-     *           scsi1 Channel 00 Id 0 Lun: 1
-     *                   Attached scsi disk sdc          State: running
-     *           scsi1 Channel 00 Id 0 Lun: 2
-     *                   Attached scsi disk sdd          State: running
-     *           scsi1 Channel 00 Id 0 Lun: 3
-     *                   Attached scsi disk sde          State: running
-     *           scsi1 Channel 00 Id 0 Lun: 4
-     *                   Attached scsi disk sdf          State: running
-     *           scsi1 Channel 00 Id 0 Lun: 5
-     *                   Attached scsi disk sdg          State: running
-     *
-     * Need a regex to match the Channel:Id:Lun lines
-     */
-    const char *regexes[] = {
-        "^\\s*scsi(\\S+)\\s+Channel\\s+(\\S+)\\s+Id\\s+(\\S+)\\s+Lun:\\s+(\\S+)\\s*$"
-    };
-    int vars[] = {
-        4
-    };
-    const char *prog[] = {
-        ISCSIADM, "--mode", "session", "-r", session, "-P", "3", NULL,
-    };
+	char sysfs_path[PATH_MAX];
+	uint32_t host, bus, target, lun;
+	DIR *sysdir;
+	struct dirent *sys_dirent;
+	struct dirent **namelist;
+	int i, n, tries, retval;
+	char *block, *scsidev, *block2;
+
+	retval = 0;
+	block = NULL;
+	scsidev = NULL;
 
-    return virStorageBackendRunProgRegex(conn, pool,
-                                         prog,
-                                         1,
-                                         regexes,
-                                         vars,
-                                         virStorageBackendISCSIMakeLUN,
-                                         (void *)session, NULL);
-}
+	snprintf(sysfs_path, PATH_MAX,
+			 "/sys/class/iscsi_session/session%s/device", session);
 
+	sysdir = opendir(sysfs_path);
+	if (sysdir == NULL) {
+        virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                              _("Failed to opendir sysfs path %s: %s"),
+                              sysfs_path, strerror(errno));
+		return -1;
+	}
+	while ((sys_dirent = readdir(sysdir))) {
+		/* double-negative, so we can use the same function for scandir below */
+		if (!notdotdir(sys_dirent))
+			continue;
+
+		if (STREQLEN(sys_dirent->d_name, "target", 6)) {
+			if (sscanf(sys_dirent->d_name, "target%u:%u:%u",
+					   &host, &bus, &target) != 3) {
+				virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+									  _("Failed to parse target in sysfs path %s: %s"),
+									  sysfs_path, strerror(errno));
+				closedir(sysdir);
+				return -1;
+				
+			}
+			break;
+		}
+	}
+	closedir(sysdir);
+
+	/* we now have the host, bus, and target; let's scan for LUNs */
+	snprintf(sysfs_path, PATH_MAX,
+			 "/sys/class/iscsi_session/session%s/device/target%u:%u:%u",
+			 session, host, bus, target);
+
+	n = scandir(sysfs_path, &namelist, notdotdir, versionsort);
+	if (n <= 0) {
+		/* we didn't find any reasonable entries; return failure */
+		virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+							  _("Failed to find any LUNs for session %s: %s"),
+							  session, strerror(errno));
+
+		return -1;
+	}
+
+	for (i=0; i<n; i++) {
+		if (sscanf(namelist[i]->d_name, "%u:%u:%u:%u\n",
+				   &host, &bus, &target, &lun) != 4)
+			continue;
+
+		if (lun == 0)
+			/* the 0'th LUN isn't a real LUN, it's just a control LUN; skip */
+			continue;
+
+		/* we found a LUN; let's find the device */
+		block = NULL;
+		scsidev = NULL;
+		tries = 0;
+
+		while (block == NULL && tries < 5) {
+			snprintf(sysfs_path, PATH_MAX, "/sys/bus/scsi/devices/%u:%u:%u:%u",
+					 host, bus, target, lun);
+
+			sysdir = opendir(sysfs_path);
+			if (sysdir == NULL) {
+				virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+									  _("Failed to opendir sysfs path %s: %s"),
+									  sysfs_path, strerror(errno));
+				retval = -1;
+				goto namelist_cleanup;
+			}
+			while ((sys_dirent = readdir(sysdir))) {
+				if (!notdotdir(sys_dirent))
+					continue;
+				if (STREQLEN(sys_dirent->d_name, "block", 5)) {
+					block = strdup(sys_dirent->d_name);
+					break;
+				}
+			}
+			closedir(sysdir);
+			tries++;
+			if (block == NULL)
+				usleep(100 * 1000);
+		}
+
+		if (block == NULL) {
+			/* we couldn't find the device link for this device; fail */
+			virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+								  _("Failed to find device link for lun %d"),
+								  lun);
+			retval = -1;
+			goto namelist_cleanup;
+		}
+
+		if (strlen(block) == 5) {
+			/* OK, this is exactly "block"; must be new-style */
+			snprintf(sysfs_path, PATH_MAX,
+					 "/sys/bus/scsi/devices/%u:%u:%u:%u/block",
+					 host, bus, target, lun);
+			sysdir = opendir(sysfs_path);
+			if (sysdir == NULL) {
+				virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+									  _("Failed to opendir sysfs path %s: %s"),
+									  sysfs_path, strerror(errno));
+				retval = -1;
+				goto namelist_cleanup;
+			}
+			while ((sys_dirent = readdir(sysdir))) {
+				if (!notdotdir(sys_dirent))
+					continue;
+
+				scsidev = strdup(sys_dirent->d_name);
+				break;
+			}
+			closedir(sysdir);
+		}
+		else {
+			/* old-style; just parse out the sd */
+			block2 = strrchr(block, ':');
+			if (block2 == NULL) {
+				/* Hm, wasn't what we were expecting; have to give up */
+				virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+									  _("Failed to parse block path %s"),
+									  block);
+				retval = -1;
+				goto namelist_cleanup;
+			}
+			block2++;
+			scsidev = strdup(block2);
+		}
+
+		retval = virStorageBackendISCSINewLun(conn, pool, lun, scsidev);
+		if (retval < 0)
+			break;
+		VIR_FREE(scsidev);
+		VIR_FREE(block);
+	}
+
+namelist_cleanup:
+	/* we call these VIR_FREE here to make sure we don't leak memory on
+	   error cases; in the success case, these are already freed but NULL,
+	   which should be fine */
+	VIR_FREE(scsidev);
+	VIR_FREE(block);
+
+	for (i=0; i<n; i++)
+		VIR_FREE(namelist[i]);
+	
+	VIR_FREE(namelist);
+
+	return retval;
+}
 
 static int
 virStorageBackendISCSIRescanLUNs(virConnectPtr conn,