Allon Mureinik has posted comments on this change. Change subject: core: Refactor SP validation out of ImagesHandler ......................................................................
Patch Set 1: (4 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java Line 198: diskInfoDestinationMap.put(image.getId(), image); Line 199: } Line 200: } Line 201: Line 202: // Check storage pool (spId was set in the constructor The comment doesn't really add any information to the code itself, frankly - I'll just remove it, I added it for myself when writing the patch, and left it there by mistake. Line 203: if (!validate(new StoragePoolValidator(getStoragePool()).isUp())) { Line 204: return false; Line 205: } Line 206: .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java Line 208: Line 209: ValidationResult spUpResult = new StoragePoolValidator(sp).isUp(); Line 210: if (!spUpResult.isValid()) { Line 211: messages.add(spUpResult.getMessage().name()); Line 212: returnValue = false; I think this entire method should be streamlined, but I don't want to do it in this patch - let's leave it to a later one? Line 213: } Line 214: Line 215: if (returnValue) { Line 216: returnValue = .................................................... File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java Line 332: when(spDao.get(any(Guid.class))).thenReturn(sp); Line 333: doReturn(spDao).when(vmRunHandler).getStoragePoolDAO(); Line 334: Line 335: doReturn(vmRunHandler).when(command).getVmRunHandler(); Line 336: will fix Line 337: Line 338: doReturn(true).when(vmRunHandler).performImageChecksForRunningVm(any(VM.class), Line 339: anyListOf(String.class), Line 340: any(RunVmParams.class), .................................................... Commit Message Line 10: ImagesHandler.PerformImagesChecks to StoragePoolValidator, where it Line 11: belongs. Line 12: Line 13: In all the cases where this validation was performed by an ImagesHandler Line 14: call a call to StoragePoolValidator was added in order to keep backwards will fix Line 15: consistency, even though it is probably useless in many (most?) cases. Line 16: E.g., there is little point in checking if the SP is UP if we already Line 17: know we have an UP domain on it. Line 18: However, these changes are left for future patches in order not to "rock -- To view, visit http://gerrit.ovirt.org/11323 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5caa3724fb73c8fa8932c8071f6d27a78d681bb3 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Alissa Bonas <abo...@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: Vered Volansky <vvola...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches