[PATCH 10/11] Improve SCSI volume key and name generation

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

The SCSI volumes currently get a name like '17:0:0:1' based
on $host:$bus:$target:$lun. The names are intended to be unique
per pool and stable across pool restarts. The inclusion of the
$host component breaks this, because the $host number for iSCSI
pools is dynamically allocated by the kernel at time of login.
This changes the name to be 'unit:0:0:1', ie removes the leading
host component. THe 'unit:' prefix is just to ensure the volume
name doesn't start with a number and make it clearer when seen
out of context.

The SCSI volumes also get a 'key' field based on the fully
qualified volume path. All SCSI volumes have a unique serial
available in hardware which can be obtained by sending a
suitable SCSI command. Call out to udev's 'scsi_id' command
to fetch this value

* src/storage/storage_backend_scsi.c: Improve key and name
  field value stability and uniqness
---
 src/storage/storage_backend_scsi.c |   71 +++++++++++++++++++++++++++++++++---
 1 files changed, 65 insertions(+), 6 deletions(-)

diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index 0d6b1ac..aeabd36 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -163,9 +163,66 @@ cleanup:
     return ret;
 }
 
+static char *
+virStorageBackendSCSISerial(const char *dev)
+{
+    const char *cmdargv[] = {
+        "/lib/udev/scsi_id",
+        "--replace-whitespace",
+        "--whitelisted",
+        "--device", dev,
+        NULL
+    };
+    int fd = -1;
+    pid_t child;
+    FILE *list = NULL;
+    char line[1024];
+    char *serial = NULL;
+    int err;
+    int exitstatus;
+
+    /* Run the program and capture its output */
+    if (virExec(cmdargv, NULL, NULL,
+                &child, -1, &fd, NULL, VIR_EXEC_NONE) < 0)
+        goto cleanup;
+
+    if ((list = fdopen(fd, "r")) == NULL) {
+        virStorageReportError(VIR_ERR_INTERNAL_ERROR,
+                              "%s", _("cannot read fd"));
+        goto cleanup;
+    }
+
+    if (fgets(line, sizeof(line), list)) {
+        char *nl = strchr(line, '\n');
+        if (nl)
+            *nl = '\0';
+        VIR_ERROR("GOT ID %s\n", line);
+        serial = strdup(line);
+    } else {
+        VIR_ERROR("NO ID %s\n", dev);
+        serial = strdup(dev);
+    }
+
+    if (!serial) {
+        virReportOOMError();
+        goto cleanup;
+    }
+
+cleanup:
+    if (list)
+        fclose(list);
+    else
+        VIR_FORCE_CLOSE(fd);
+
+    while ((err = waitpid(child, &exitstatus, 0) == -1) && errno == EINTR);
+
+    return serial;
+}
+
+
 static int
 virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
-                            uint32_t host,
+                            uint32_t host ATTRIBUTE_UNUSED,
                             uint32_t bus,
                             uint32_t target,
                             uint32_t lun,
@@ -183,7 +240,12 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
 
     vol->type = VIR_STORAGE_VOL_BLOCK;
 
-    if (virAsprintf(&(vol->name), "%u.%u.%u.%u", host, bus, target, lun) < 0) {
+    /* 'host' is dynamically allocated by the kernel, first come,
+     * first served, per HBA. As such it isn't suitable for use
+     * in the volume name. We only need uniquness per-pool, so
+     * just leave 'host' out
+     */
+    if (virAsprintf(&(vol->name), "unit:%u:%u:%u", bus, target, lun) < 0) {
         virReportOOMError();
         retval = -1;
         goto free_vol;
@@ -231,10 +293,7 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
         goto free_vol;
     }
 
-    /* XXX should use logical unit's UUID instead */
-    vol->key = strdup(vol->target.path);
-    if (vol->key == NULL) {
-        virReportOOMError();
+    if (!(vol->key = virStorageBackendSCSISerial(vol->target.path))) {
         retval = -1;
         goto free_vol;
     }
-- 
1.7.2.3

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