ping^2? Any brave soul want to take a look? Tks - John On 1/5/19 9:56 AM, John Ferlan wrote: > > ping? > > Tks, > > John > > On 12/18/18 5:46 PM, 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@target_port", where >> @target_port is just the value of the ID_TARGET_PORT for the LUN. >> >> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> >> --- >> 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; >> } >> >> > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
