Multi-Host support for NFS3 and iSCSI#64
Conversation
sandeeplocharla
requested review from
piyush5netapp,
rajiv-jain-netapp and
suryag1201
as code owners
sandeeplocharla
requested review from
piyush5netapp,
rajiv-jain-netapp and
suryag1201
and removed request for
piyush5netapp,
rajiv-jain-netapp and
suryag1201
| if (!hostConnectedToPool) { | ||
| try { | ||
| logger.info("grantAccess: Host [{}] is not connected to NFS pool [{}], reconnecting host to pool", host.getId(), storagePool.getId()); | ||
| boolean connected = storageManager.connectHostToSharedPool(host, storagePool.getId()); |
There was a problem hiding this comment.
The shared pool term is picked from the CloudStack perspective, or from the way we are approaching storage pool design. Please add code comment, if possible
There was a problem hiding this comment.
Though I'm aware that this code is a bit of a stretch, I have added with nfs protocol being enabled on a host post its addition to the storage pool. I wasn't able to simulate the scenario and looks like a rare occurence. So, on discussing with the team in the scrum today, we came to consensus that at this point we can document the pre-requisite of enabling the protocol on host and remove this code. I wanted to take your opinion on the same. So, please let me know if you feel the same?
| throw new CloudRuntimeException("Invalid accessGroup object - hostsToConnect is null or empty"); | ||
| } | ||
|
|
||
| Map<String, String> details = storagePoolDetailsDao.listDetailsKeyPairs(accessGroup.getStoragePoolId()); |
There was a problem hiding this comment.
How can an NFS storage pool exist without an export policy associated with it? Based on that, I’m not sure how the null check below would help or how it would lead to creating a policy for the storage pool.
Also, could you add code comments explaining the scenarios in which the workflow would enter the if and else blocks?
There was a problem hiding this comment.
Thankyou for pointing out. The null check has been auto-added while refactoring the code at the end.
Sure will update the necessary comments.
2 participants
Description
This PR...
Provides support to Multi-Host for NFS3 and iSCSI
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?