Vered Volansky has uploaded a new change for review. Change subject: core: use config value when possible ......................................................................
core: use config value when possible ConfigValues.InitStorageSparseSizeInGB should be used/mocked when possible and not duplicated with data-members that may or may not have the same value. Change-Id: Ic87e7406be257d7706084c3264fc85b08795d83c Signed-off-by: Vered Volansky <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/StorageDomainValidator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/StorageDomainValidatorFreeSpaceTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/StorageDomainValidatorTest.java 4 files changed, 34 insertions(+), 23 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/18/33918/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/StorageDomainValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/StorageDomainValidator.java index 26ed10a..0437dac 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/StorageDomainValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/StorageDomainValidator.java @@ -16,9 +16,8 @@ public class StorageDomainValidator { - private static final double QCOW_OVERHEAD_FACTOR = 1.1; - private static final long INITIAL_BLOCK_ALLOCATION_SIZE = 1024L * 1024L * 1024L; private static final long EMPTY_QCOW_HEADER_SIZE = 1024L * 1024L; + private static final double QCOW_OVERHEAD_FACTOR = 1.1; private final StorageDomain storageDomain; @@ -69,6 +68,11 @@ return Config.<Integer> getValue(ConfigValues.FreeSpaceCriticalLowInGB); } + private static long getInitialBlockAllocationSizeInBytes() { + return Config.<Integer>getValue(ConfigValues.InitStorageSparseSizeInGB) + * 1024L * 1024L * 1024L; + + } /** * Verify there's enough space in the storage domain for creating new DiskImages. * Some space should be allocated on the storage domain according to the volumes type and format, and allocation policy, @@ -92,7 +96,7 @@ if (storageDomain.getStorageType().isFileDomain()) { sizeForDisk = EMPTY_QCOW_HEADER_SIZE; } else { - sizeForDisk = INITIAL_BLOCK_ALLOCATION_SIZE; + sizeForDisk = getInitialBlockAllocationSizeInBytes(); } } else if (diskImage.getVolumeType() == VolumeType.Sparse) { sizeForDisk = EMPTY_QCOW_HEADER_SIZE; diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskCommandTest.java index 095338b..d72a7d5 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskCommandTest.java @@ -67,12 +67,14 @@ public class AddDiskCommandTest { private static int MAX_BLOCK_SIZE = 8192; private static int FREE_SPACE_CRITICAL_LOW_IN_GB = 5; + private final static int INITIAL_BLOCK_ALLOCATION_SIZE = 1; private static int MAX_PCI_SLOTS = 26; @ClassRule public static MockConfigRule mcr = new MockConfigRule( mockConfig(ConfigValues.MaxBlockDiskSize, MAX_BLOCK_SIZE), mockConfig(ConfigValues.FreeSpaceCriticalLowInGB, FREE_SPACE_CRITICAL_LOW_IN_GB), + mockConfig(ConfigValues.InitStorageSparseSizeInGB, INITIAL_BLOCK_ALLOCATION_SIZE), mockConfig(ConfigValues.ShareableDiskEnabled, Version.v3_1.toString(), true), mockConfig(ConfigValues.VirtIoScsiEnabled, Version.v3_3.toString(), true) ); @@ -103,6 +105,9 @@ @Mock private DiskValidator diskValidator; + + @Mock + private StorageDomainValidator storageDomainValidator; /** * The command under test. @@ -200,8 +205,8 @@ mockVm(); mockEntities(storageId); - doReturn(mockStorageDomainValidatorWithSpace()).when(command).createStorageDomainValidator(); - + mockStorageDomainValidator(); + doReturn(storageDomainValidator).when(command).createStorageDomainValidator(); assertTrue(command.canDoAction()); } @@ -214,7 +219,10 @@ mockStorageDomain(storageId); mockStoragePoolIsoMap(); mockMaxPciSlots(); - doReturn(mockStorageDomainValidatorWithoutSpace()).when(command).createStorageDomainValidator(); + mockStorageDomainValidator(); + when(storageDomainValidator.hasSpaceForNewDisk(any(DiskImage.class))).thenReturn( + new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_STORAGE_DOMAIN)); + doReturn(storageDomainValidator).when(command).createStorageDomainValidator(); assertFalse(command.canDoAction()); assertTrue(command.getReturnValue() @@ -423,24 +431,10 @@ return snapshotsValidator; } - private static StorageDomainValidator mockStorageDomainValidatorWithoutSpace() { - StorageDomainValidator storageDomainValidator = mockStorageDomainValidator(); - when(storageDomainValidator.hasSpaceForNewDisk(any(DiskImage.class))).thenReturn( - new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_STORAGE_DOMAIN)); - return storageDomainValidator; - } - - private static StorageDomainValidator mockStorageDomainValidatorWithSpace() { - StorageDomainValidator storageDomainValidator = mockStorageDomainValidator(); - when(storageDomainValidator.hasSpaceForNewDisk(any(DiskImage.class))).thenReturn(ValidationResult.VALID); - return storageDomainValidator; - } - - private static StorageDomainValidator mockStorageDomainValidator() { - StorageDomainValidator storageDomainValidator = mock(StorageDomainValidator.class); + private void mockStorageDomainValidator() { when(storageDomainValidator.isDomainExistAndActive()).thenReturn(ValidationResult.VALID); when(storageDomainValidator.isDomainWithinThresholds()).thenReturn(ValidationResult.VALID); - return storageDomainValidator; + when(storageDomainValidator.hasSpaceForNewDisk(any(DiskImage.class))).thenReturn(ValidationResult.VALID); } private DiskValidator spyDiskValidator(Disk disk) { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/StorageDomainValidatorFreeSpaceTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/StorageDomainValidatorFreeSpaceTest.java index c53f3cb..8217f1f 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/StorageDomainValidatorFreeSpaceTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/StorageDomainValidatorFreeSpaceTest.java @@ -1,6 +1,7 @@ package org.ovirt.engine.core.bll.validator; import static org.junit.Assert.assertEquals; +import static org.ovirt.engine.core.utils.MockConfigRule.mockConfig; import java.util.ArrayList; import java.util.Arrays; @@ -8,6 +9,7 @@ import java.util.Collections; import java.util.List; +import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -17,6 +19,8 @@ import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.VolumeFormat; import org.ovirt.engine.core.common.businessentities.VolumeType; +import org.ovirt.engine.core.common.config.ConfigValues; +import org.ovirt.engine.core.utils.MockConfigRule; @RunWith(Parameterized.class) public class StorageDomainValidatorFreeSpaceTest { @@ -27,6 +31,13 @@ private boolean isValidForNew; private boolean isValidForSnapshots; + private final static int INITIAL_BLOCK_ALLOCATION_SIZE = 1; + + @ClassRule + public static MockConfigRule mcr = new MockConfigRule( + mockConfig(ConfigValues.InitStorageSparseSizeInGB, INITIAL_BLOCK_ALLOCATION_SIZE) + ); + public StorageDomainValidatorFreeSpaceTest(DiskImage disk, StorageDomain sd, boolean isValidForCloned, diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/StorageDomainValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/StorageDomainValidatorTest.java index 85c1b66..215faa3 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/StorageDomainValidatorTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/StorageDomainValidatorTest.java @@ -23,10 +23,12 @@ private StorageDomain domain; private StorageDomainValidator validator; private final static int FREE_SPACE_CRITICAL_LOW_IN_GB = 5; + private final static int INITIAL_BLOCK_ALLOCATION_SIZE = 1; @ClassRule public static MockConfigRule mcr = new MockConfigRule( - mockConfig(ConfigValues.FreeSpaceCriticalLowInGB, FREE_SPACE_CRITICAL_LOW_IN_GB) + mockConfig(ConfigValues.FreeSpaceCriticalLowInGB, FREE_SPACE_CRITICAL_LOW_IN_GB), + mockConfig(ConfigValues.InitStorageSparseSizeInGB, INITIAL_BLOCK_ALLOCATION_SIZE) ); @Before -- To view, visit http://gerrit.ovirt.org/33918 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic87e7406be257d7706084c3264fc85b08795d83c Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vered Volansky <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
