Liron Aravot has posted comments on this change.

Change subject: core: handle error when iso domain in maintenance
......................................................................


Patch Set 3: (4 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/IsoDomainListSyncronizer.java
Line 87:     }
Line 88: 
Line 89:     /**
Line 90:      * Returns the singleton instance.
Line 91:      * 
General - can you remove the white spaces?
Line 92:      * @return Singleton instance of IsoDomainManager
Line 93:      */
Line 94:     public static IsoDomainListSyncronizer getInstance() {
Line 95:         return isoDomainListSyncronizer;


Line 297:                         fileTypeExt,
Line 298:                         status);
Line 299:             }
Line 300: 
Line 301:             else {
please move the else up
Line 302:                 // Try to refresh the domain of the storage pool id 
because its status is active.
Line 303:                 refreshSucceeded =
Line 304:                         
refreshIsoDomainFileForStoragePool(storageDomainId,
Line 305:                                 storagePoolId,


Line 298:                         status);
Line 299:             }
Line 300: 
Line 301:             else {
Line 302:                 // Try to refresh the domain of the storage pool id 
because its status is active.
I'd just remove it..it's pretty clear from the if :-) your choice..
Line 303:                 refreshSucceeded =
Line 304:                         
refreshIsoDomainFileForStoragePool(storageDomainId,
Line 305:                                 storagePoolId,
Line 306:                                 fileTypeExt);


Line 309:                             storageDomainId,
Line 310:                             fileTypeExt,
Line 311:                             storagePoolId);
Line 312:                     // set a mock repository file meta data with 
storage domain id and storage pool id.
Line 313:                     RepoFileMetaData repoFileMetaData = new 
RepoFileMetaData();
this part was done before to all statuses, now it's done only if we failed and 
it's active - are you sure that this is the correct behaviour and that we don't 
need to avoid it only in case of maintenance and do it for other statuses?
Line 314:                     repoFileMetaData.setStoragePoolId(storagePoolId);
Line 315:                     repoFileMetaData.setRepoDomainId(storageDomainId);
Line 316:                     repoFileMetaData.setFileType(fileTypeExt);
Line 317: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ide53f42c087ffbc9240cccc033e809ff8117ec8c
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to