4:46 p.m.
https://bugzilla.redhat.com/show_bug.cgi?id=1657468
Commit be1bb6c95 changed the way volumes were stored from a forward
linked list to a hash table. In doing so, it required that each vol
object would have 3 unique values as keys into tables - key, name,
and path. Due to how vHBA/NPIV LUNs are created/used this resulted
in a failure to utilize all the LUN's found during processing.
The virStorageBackendSCSINewLun uses virStorageBackendSCSISerial
in order to read/return the unique serial number of the LUN to be
used as a key for the volume.
However, for NPIV based LUNs the logic results in usage of the
same serial value for each path to the LUN. For example,
$ lsscsi -tg
...
[4:0:3:13] disk fc:0x207800c0ffd79b2a0xeb02ef /dev/sde /dev/sg16
[4:0:4:0] disk fc:0x50060169446021980xeb1f00 /dev/sdf /dev/sg17
[4:0:5:0] disk fc:0x50060161446021980xeb2000 /dev/sdg /dev/sg18
...
/lib/udev/scsi_id --replace-whitespace --whitelisted --device /dev/sde
3600c0ff000d7a2965c603e5401000000
/lib/udev/scsi_id --replace-whitespace --whitelisted --device /dev/sdf
350060160c460219850060160c4602198
/lib/udev/scsi_id --replace-whitespace --whitelisted --device /dev/sdg
350060160c460219850060160c4602198
The /dev/sdf and /dev/sdg although separate LUNs would end up with the
same serial number used for the vol->key value. When attempting to add
the LUN via virStoragePoolObjAddVol, the hash table code will indicate
that we have a duplicate:
virHashAddOrUpdateEntry:341 : internal error: Duplicate key
and thus the LUN is not added to the pool.
Digging deeper into the scsi_id output using the --export option one
will find that the only difference between the two luns is:
ID_TARGET_PORT=1 for /dev/sdf
ID_TARGET_PORT=2 for /dev/sdg
So, let's use the ID_TARGET_PORT string value along with the serial
to generate a more unique key using "@serial_PORT@target_port", where
@target_port is just the value of the ID_TARGET_PORT for the LUN.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_util.c | 61 +++++++++++++++++++++++++++++++++++++-
1 file changed, 60 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index a84ee5b600..d6d441c06d 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -3755,6 +3755,49 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool)
}
+static char *
+virStorageBackendSCSITargetPort(const char *dev)
+{
+ char *target_port = NULL;
+ const char *id;
+#ifdef WITH_UDEV
+ virCommandPtr cmd = virCommandNewArgList(
+ "/lib/udev/scsi_id",
+ "--replace-whitespace",
+ "--whitelisted",
+ "--export",
+ "--device", dev,
+ NULL
+ );
+
+ /* Run the program and capture its output */
+ virCommandSetOutputBuffer(cmd, &target_port);
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
+#endif
+
+ if (target_port && STRNEQ(target_port, "") &&
+ (id = strstr(target_port, "ID_TARGET_PORT="))) {
+ char *nl = strchr(id, '\n');
+ if (nl)
+ *nl = '\0';
+ id = strrchr(id, '=');
+ memmove(target_port, id + 1, strlen(id));
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unable to uniquely identify target port for
'%s'"),
+ dev);
+ }
+
+#ifdef WITH_UDEV
+ cleanup:
+ virCommandFree(cmd);
+#endif
+
+ return target_port;
+}
+
+
static char *
virStorageBackendSCSISerial(const char *dev)
{
@@ -3813,6 +3856,8 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
virStorageVolDefPtr vol = NULL;
char *devpath = NULL;
+ char *key = NULL;
+ char *target_port = NULL;
int retval = -1;
/* Check if the pool is using a stable target path. The call to
@@ -3877,9 +3922,21 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
VIR_STORAGE_VOL_READ_NOERROR)) < 0)
goto cleanup;
- if (!(vol->key = virStorageBackendSCSISerial(vol->target.path)))
+ if (!(key = virStorageBackendSCSISerial(vol->target.path)))
goto cleanup;
+ if (def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST &&
+ STRNEQ(key, vol->target.path)) {
+ /* NPIV based LUNs use the same "serial" key. In order to distinguish
+ * we need to append a port value */
+ if (!(target_port = virStorageBackendSCSITargetPort(vol->target.path)))
+ goto cleanup;
+ if (virAsprintf(&vol->key, "%s_PORT%s", key, target_port) <
0)
+ goto cleanup;
+ } else {
+ VIR_STEAL_PTR(vol->key, key);
+ }
+
def->capacity += vol->target.capacity;
def->allocation += vol->target.allocation;
@@ -3892,6 +3949,8 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
cleanup:
virStorageVolDefFree(vol);
VIR_FREE(devpath);
+ VIR_FREE(target_port);
+ VIR_FREE(key);
return retval;
}
--
2.17.2