Maor Lipchuk has posted comments on this change. Change subject: core,webadmin: Move storage type checks to domain instead of pool ......................................................................
Patch Set 5: Code-Review-1 (7 comments) none of my comments were addressed... again... http://gerrit.ovirt.org/#/c/23294/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmCommand.java: Line 341: params.setParentParameters(getParameters()); Line 342: Line 343: StorageDomainStatic sourceDomain = getStorageDomainStaticDAO().get(storageDomainId); Line 344: Line 345: // if the data domains are block based storage, the memory volume type is preallocated There are no storage domains only one storage domain Line 346: // so we need to use copy collapse in order to convert it to be sparsed in the export domain Line 347: if (sourceDomain.getStorageType().isBlockDomain()) { Line 348: params.setUseCopyCollapse(true); Line 349: params.setVolumeType(VolumeType.Sparse); 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 (see my comment at http://gerrit.ovirt.org/#/c/23072/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDiskAlignmentCommand.java) 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/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java - "The name of the method is wrong, you getMemoryVolumeFor StorageDomain not for pool anymore" 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 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 about it 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 getMemoryVolumeTypeForDomain 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 on storageDomain 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
