Allon Mureinik has posted comments on this change.

Change subject: core: Add vdc query for attached Storage Domain
......................................................................


Patch Set 4: Code-Review-1

(4 comments)

http://gerrit.ovirt.org/#/c/36447/4//COMMIT_MSG
Commit Message:

Line 10: another Data Center.
Line 11: The validation is being done on the Storage Domain metadata which
Line 12: contains the storage pool id.
Line 13: 
Line 14: The Query uses connect and disconnect operations for the Storage Domain
s/Query/query/
Line 15: to fetch the storage domain info.
Line 16: 
Line 17: Change-Id: I749cad223d90dcc8d193765ed842d7e062ad566c
Line 18: Bug-Url: https://bugzilla.redhat.com/1138115


http://gerrit.ovirt.org/#/c/36447/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetStorageDomainsWithAttachedStoragePoolGuidQuery.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetStorageDomainsWithAttachedStoragePoolGuidQuery.java:

Line 28:     public GetStorageDomainsWithAttachedStoragePoolGuidQuery(P 
parameters) {
Line 29:         super(parameters);
Line 30:     }
Line 31: 
Line 32:     public Guid getVdsForConnectStorage() {
should be private. Also, move it below the executeQueryCommand() - it's not the 
"public contract"
Line 33:         vdsId = getParameters().getVdsId();
Line 34:         if (vdsId == null) {
Line 35:             // Get a Host which is at UP state to connect to the 
Storage Domain.
Line 36:             List<VDS> hosts =


Line 57:     private boolean isDataCenterValidForAttachedStorageDomains() {
Line 58:         StoragePool storagePool = 
getDbFacade().getStoragePoolDao().get(getParameters().getId());
Line 59:         if ((storagePool != null) && storagePool.getStatus() != 
StoragePoolStatus.Up) {
Line 60:             log.info("The attach of the Storage Domain is being done 
on a Data Center which is not in UP status." +
Line 61:                     " The query for attached Storage Domain will not 
be excuted");
"The attach of ..." etc - not relevant here. This is just a query. You can just 
have "Data Center XYZ is not up - returning null".
Line 62:             return false;
Line 63:         }
Line 64:         return true;
Line 65:     }


http://gerrit.ovirt.org/#/c/36447/4/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/GetStorageDomainsWithAttachedStoragePoolGuidQueryTest.java
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/GetStorageDomainsWithAttachedStoragePoolGuidQueryTest.java:

Line 39: 
Line 40:     private StorageDomain storageDomain;
Line 41: 
Line 42:     @Before
Line 43:     public void setup() {
s/setup/setUp/
Line 44:         storageDomain = new StorageDomain();
Line 45:         storageDomain.setStorageName("Name of Storage");
Line 46:         storageDomain.setStorageType(StorageType.ISCSI);
Line 47: 


-- 
To view, visit http://gerrit.ovirt.org/36447
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I749cad223d90dcc8d193765ed842d7e062ad566c
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to