piyush5netapp commented on code in PR #14192:
URL: https://github.com/apache/cloudstack/pull/14192#discussion_r4521625003
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java:
##########
@@ -148,137 +153,24 @@ public boolean connectPhysicalDisk(String volumeUuid,
KVMStoragePool pool, Map<S
// After a certain number of tries and a certain waiting period in
between tries,
// this method could still return (it should not block indefinitely)
(the race condition
// isn't solved here, but made highly unlikely to be a problem).
- // If the by-path is missing or is a regular file (not the iSCSI block
symlink), size
- // stays 0. Return false so connect does not succeed and a raw file is
not created at
- // that by-path in place of the real LUN device.
- if (!waitForDiskToBecomeAvailable(volumeUuid, pool)) {
- logger.warn("iSCSI device not ready for target {} at {}:{} after
wait", volumeUuid, host, port);
- return false;
- }
+ waitForDiskToBecomeAvailable(volumeUuid, pool);
return true;
}
- /**
- * Checks the result of an iscsiadm node-create command.
- * Returns true if the node was created or already exists, false on
failure.
- */
- boolean handleNodeCreateResult(String result, String volumeUuid) {
Review Comment:
@winterhazel Yes, this was made intentional as the changes done earlier on
IscsiAdmStorageAdaptor.java were to support Netapp Ontap iscsi pools. Since we
have introduced new Netapp Ontap pool type as OntapIscsi, all those changes are
replicated to new adapter OntapIscsiStorageAdaptor. I did this keeping
separation of concern as one of the thought. Also, I see IscsiAdmStorageAdaptor
is written mostly in a way to support a design where iscsi session cannot be
shared across multiple cloudstack volume which was not aligning with Netapp
Ontap where same iscsi session can be shared across multiple cloudstack
volumes. But, I feel refactoring of the changes like idempotency in node
creation, session login or pre-existence can also sit in IscsiAdmStorageAdaptor
as an enhancement. Let me know your thoughts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
