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

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(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list