Yair Zaslavsky has posted comments on this change. Change subject: core: RunVmCommand.canDoAction cleanup (3) ......................................................................
Patch Set 5: (3 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java Line 276: vdsSelector, Line 277: new SnapshotsValidator()); Line 278: } Line 279: Line 280: private static DiskDao getDiskDao() { Why static and why private ? Think about possible mocking here... See DAO getters at AuditLoggableBase. Line 281: return DbFacade.getInstance().getDiskDao(); Line 282: } Line 283: Line 284: private static RunVmValidator getRunVmValidator() { .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java Line 41: import org.ovirt.engine.core.dao.StoragePoolDAO; Line 42: Line 43: /** A utility class for verifying running a vm*/ Line 44: public class VmRunHandler { Line 45: private static final VmRunHandler instance = new VmRunHandler(); No double-check locking here? What a shame ;) Line 46: Line 47: public static VmRunHandler getInstance() { Line 48: return instance; Line 49: } Line 161: * Line 162: * @param vm Line 163: * @return true if all storage domains have enough space to create snapshots for this VM plugged disks Line 164: */ Line 165: public boolean hasSpaceForSnapshots(VM vm, ArrayList<String> message) { You changed ArrayList to List at some previous patch at the Patchset. Would be awesome if you can do this here as well :) Line 166: Integer minSnapshotSize = Config.<Integer> GetValue(ConfigValues.InitStorageSparseSizeInGB); Line 167: for (Entry<StorageDomain, Integer> e : mapStorageDomainsToNumOfDisks(vm).entrySet()) { Line 168: if (!destinationHasSpace(e.getKey(), minSnapshotSize * e.getValue(), message)) { Line 169: return false; -- To view, visit http://gerrit.ovirt.org/13399 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ida913a72e04c4edc57f9b6e2baf93e31e3df82f2 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Laszlo Hornyak <lhorn...@redhat.com> Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: ofri masad <oma...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches