Re: [libvirt] [PATCH] storage: More uniquely identify NPIV LUNs

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&gt; --- 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; +} + +