Allon Mureinik has posted comments on this change.

Change subject: core: Add a query for attached Storage Domains
......................................................................


Patch Set 8: Code-Review+1

(2 comments)

minor comments inline

http://gerrit.ovirt.org/#/c/34233/8/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 33:         // Get a Host which is at UP state to connect to the Storage 
Domain.
Line 34:         List<VDS> hosts =
Line 35:                 
getDbFacade().getVdsDao().getAllForStoragePoolAndStatus(getParameters().getId(),
 VDSStatus.Up);
Line 36:         if (!hosts.isEmpty()) {
Line 37:             vdsId = RandomUtils.instance().pickRandom(hosts).getId();
You should probably log which host you're using
Line 38:         } else {
Line 39:             return;
Line 40:         }
Line 41: 


Line 72:         return 
StorageHelperDirector.getInstance().getItem(storageDomain.getStorageType())
Line 73:                 .connectStorageToDomainByVdsId(storageDomain, vdsId);
Line 74:     }
Line 75: 
Line 76:     private void printErrorMessage(StorageDomain storageDomain) {
logErrorMessage would probably be a better name, as you aren'y really printing 
anything.

Also, this should be static (assuming the log is too)
Line 77:         log.error("Could not get Storage Domain info for Storage 
Domain (name:'{}', id:'{}') with VDS '{}'. ",
Line 78:                 storageDomain.getName(),
Line 79:                 storageDomain.getId(),
Line 80:                 vdsId);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id6fc7a2d722611b1bccecee9868223ff437596ed
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to