Tal Nisan has posted comments on this change. Change subject: core,webadmin: Move storage type checks to domain instead of pool ......................................................................
Patch Set 5: (6 comments) http://gerrit.ovirt.org/#/c/23294/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDiskAlignmentCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDiskAlignmentCommand.java: Line 115: Line 116: StorageDomainStatic sds = getStorageDomainStaticDAO().get(((DiskImage) getDisk()).getStorageIds().get(0)); Line 117: if (!sds.getStorageType().isBlockDomain()) { Line 118: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_ALIGNMENT_SCAN_STORAGE_TYPE); Line 119: } > This is only relevant for images it should be in the DiskImagesValidator (s This check is only relevant here and I haven't changed the actual check anyway, I have to disagree with you on that one Line 120: } Line 121: Line 122: if (isImageExclusiveLockNeeded() && getVm().isRunningOrPaused()) { Line 123: return failCanDoAction(VdcBllMessages.ERROR_CANNOT_RUN_ALIGNMENT_SCAN_VM_IS_RUNNING); http://gerrit.ovirt.org/#/c/23294/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java: Line 371: } Line 372: } Line 373: Line 374: private VolumeType getMemoryVolumeType() { Line 375: return getMemoryVolumeTypeForPool(getStorageDomain().getStorageType()); > again, see my comment at http://gerrit.ovirt.org/#/c/23072/4/backend/manage Done Line 376: } Line 377: Line 378: /** Line 379: * Returns whether to use Sparse or Preallocation. If the storage type is file system devices ,it would be more http://gerrit.ovirt.org/#/c/23294/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java: Line 722: } Line 723: Line 724: private MoveOrCopyImageGroupParameters buildMoveOrCopyImageGroupParametersForMemoryDumpImage(Guid containerID, Line 725: Guid storageId, Guid imageId, Guid volumeId) { Line 726: > empty line is redundant Done Line 727: MoveOrCopyImageGroupParameters params = new MoveOrCopyImageGroupParameters(containerID, Line 728: imageId, volumeId, imageId, volumeId, storageId, getMoveOrCopyImageOperation()); Line 729: params.setParentCommand(getActionType()); Line 730: params.setCopyVolumeType(CopyVolumeType.LeafVol); Line 735: params.setEntityInfo(new EntityInfo(VdcObjectType.VM, getVm().getId())); Line 736: params.setParentParameters(getParameters()); Line 737: Line 738: StorageDomainStatic storageDomain = getStorageDomainStaticDAO().get(storageId); Line 739: if (storageDomain.getStorageType().isBlockDomain()) { > If storage domain is null we have a bug and we should add an error log abou It is checked for existence before the memory snapshots are populated, see my comment to derez in an earlier patchset Line 740: params.setUseCopyCollapse(true); Line 741: params.setVolumeType(VolumeType.Preallocated); Line 742: params.setVolumeFormat(VolumeFormat.RAW); Line 743: } http://gerrit.ovirt.org/#/c/23294/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/LiveSnapshotMemoryImageBuilder.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/LiveSnapshotMemoryImageBuilder.java: Line 92: storagePool.getId(), Line 93: storageDomainId, Line 94: memoryDumpImageGroupId, Line 95: vm.getTotalMemorySizeInBytes(), Line 96: HibernateVmCommand.getMemoryVolumeTypeForPool(storageDomainStatic.getStorageType()), > The method name getMemoryVolumeTypeForPool is wrong now, you should call it Done Line 97: VolumeFormat.RAW, Line 98: memoryDumpVolumeId, Line 99: "")); Line 100: http://gerrit.ovirt.org/#/c/23294/5/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetDiskAlignmentCommandTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetDiskAlignmentCommandTest.java: Line 184: VdcBllMessages.ACTION_TYPE_FAILED_IMAGE_REPOSITORY_NOT_FOUND); Line 185: } Line 186: Line 187: @Test Line 188: public void testCanDoActionStoragePoolFile() { > The name of the method is not relevant to the test, you are now testing it Done Line 189: storageDomain.setStorageType(StorageType.NFS); Line 190: CanDoActionTestUtils.runAndAssertCanDoActionFailure(cmd, Line 191: VdcBllMessages.ACTION_TYPE_FAILED_ALIGNMENT_SCAN_STORAGE_TYPE); Line 192: } -- To view, visit http://gerrit.ovirt.org/23294 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3db80f1749d93444e9cdac8038859b7d0865f6e5 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tal Nisan <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: Vered Volansky <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
