Omer Frenkel has posted comments on this change.

Change subject: engine-core: Multiple storage domains when importing VM
......................................................................


Patch Set 2: (3 inline comments)

some minor comments

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
Line 164:         if(retVal) {
are you using the formatter? there should be a space after the if sentences 
(this applies also later in this file)

Line 270:             if(domainMap.isEmpty()) {
doesn't it make more sense to put this piece of code (setting the map if its 
empty) before all dest domain checks, and do all the checks in one loop over 
it? (currently i see 3 different loops over this map)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java
Line 300: //    protected Map<storage_domains, Integer> 
getSpaceRequirementsForStorageDomains(List<DiskImage> images) {
any reason to keep this commented code?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I67ebc1b402194897cf2ed12f45228153e2efe153
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Jonathan Choate <jcho...@redhat.com>
Gerrit-Reviewer: Jonathan Choate <jcho...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to