[libvirt] [PATCH] storage: More uniquely identify NPIV LUNs
Ján Tomko
jtomko at redhat.com
Tue Jan 15 16:32:39 UTC 2019
On Tue, Dec 18, 2018 at 05:46:53PM -0500, John Ferlan wrote:
>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 at target_port", where
>@target_port is just the value of the ID_TARGET_PORT for the LUN.
>
>Signed-off-by: John Ferlan <jferlan at 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
This sort of conditional code within a function can lead to confusing
code. Also, why is this even depending on WITH_UDEV if it does not
depend on libudev at all?
>+ virCommandPtr cmd = virCommandNewArgList(
>+ "/lib/udev/scsi_id",
>+ "--replace-whitespace",
>+ "--whitelisted",
>+ "--export",
>+ "--device", dev,
>+ NULL
>+ );
>+
>+ /* Run the program and capture its output */
// THIS IS BRIDGE [0]
>+ 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);
The other usage of /lib/udev/scsi_id oddly guarded by WITH_UDEV actually
provides a fallback in case we're compiling on a system without udev.
Erroring out anyway makes the whole #ifdef even more pointless.
Jano
>+ }
>+
>+#ifdef WITH_UDEV
>+ cleanup:
>+ virCommandFree(cmd);
>+#endif
>+
>+ return target_port;
>+}
>+
>+
[0] https://www.abstrusegoose.com/432
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190115/be0da9ce/attachment-0001.sig>
More information about the libvir-list
mailing list