Allon Mureinik has posted comments on this change. Change subject: core: Add storage space thresholds support ......................................................................
Patch Set 28: Code-Review-1 (17 comments) https://gerrit.ovirt.org/#/c/35277/28/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskCommandTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddDiskCommandTest.java: Line 72: Line 73: @ClassRule Line 74: public static MockConfigRule mcr = new MockConfigRule( Line 75: mockConfig(ConfigValues.MaxBlockDiskSize, MAX_BLOCK_SIZE), Line 76: mockConfig(ConfigValues.CriticalSpaceThreshold, CRITICAL_SPACE_THRESHOLD), The command should now take this value from the domain - hence this mocking should be removed. Line 77: mockConfig(ConfigValues.ShareableDiskEnabled, Version.v3_1.toString(), true), Line 78: mockConfig(ConfigValues.VirtIoScsiEnabled, Version.v3_3.toString(), true) Line 79: ); Line 80: https://gerrit.ovirt.org/#/c/35277/28/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmCommandTest.java: Line 607: mcr.mockConfigValue(ConfigValues.IsMigrationSupported, Version.v3_3, migrationMap); Line 608: } Line 609: Line 610: private void mockConfigSizeRequirements(int requiredSpaceBufferInGB) { Line 611: mcr.mockConfigValue(ConfigValues.CriticalSpaceThreshold, requiredSpaceBufferInGB); The command should now take this value from the domain - hence this mocking should be removed. Line 612: } Line 613: Line 614: private void mockConfigSizeDefaults() { Line 615: int requiredSpaceBufferInGB = 5; https://gerrit.ovirt.org/#/c/35277/28/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommandTestAbstract.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommandTestAbstract.java: Line 68: mockConfig(ConfigValues.MaxVmNameLengthNonWindows, 64), Line 69: mockConfig(ConfigValues.MaxVmsInPool, 87), Line 70: mockConfig(ConfigValues.VM32BitMaxMemorySizeInMB, 2048), Line 71: mockConfig(ConfigValues.VM64BitMaxMemorySizeInMB, 262144), Line 72: mockConfig(ConfigValues.CriticalSpaceThreshold, 1), The command should now take this value from the domain - hence this mocking should be removed. Line 73: mockConfig(ConfigValues.InitStorageSparseSizeInGB, 1), Line 74: mockConfig(ConfigValues.ValidNumOfMonitors, Arrays.asList("1,2,4".split(","))) Line 75: ); Line 76: https://gerrit.ovirt.org/#/c/35277/28/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommandTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommandTest.java: Line 50: private final static int CRITICAL_SPACE_THRESHOLD = 0; Line 51: Line 52: @ClassRule Line 53: public static MockConfigRule mcr = new MockConfigRule( Line 54: mockConfig(ConfigValues.CriticalSpaceThreshold, CRITICAL_SPACE_THRESHOLD)); The command should now take this value from the domain - hence this mocking should be removed. Line 55: Line 56: @Mock Line 57: private DiskImageDAO diskImageDao; Line 58: @Mock https://gerrit.ovirt.org/#/c/35277/28/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java: Line 122: doReturn(snapshot).when(snapshotDao).get(snapshot.getId()); Line 123: } Line 124: Line 125: private void mockConfigSizeRequirements(int requiredSpaceBufferInGB) { Line 126: mcr.mockConfigValue(ConfigValues.CriticalSpaceThreshold, requiredSpaceBufferInGB); The command should now take this value from the domain - hence this mocking should be removed. Line 127: } Line 128: Line 129: private void mockConfigSizeDefaults() { Line 130: int requiredSpaceBufferInGB = 5; https://gerrit.ovirt.org/#/c/35277/28/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RestoreAllSnapshotCommandTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RestoreAllSnapshotCommandTest.java: Line 52: public class RestoreAllSnapshotCommandTest { Line 53: @ClassRule Line 54: public static MockConfigRule mcr = Line 55: new MockConfigRule( Line 56: mockConfig(ConfigValues.CriticalSpaceThreshold, 5) The command should now take this value from the domain - hence this mocking should be removed. Line 57: ); Line 58: Line 59: @Mock Line 60: private VDSBrokerFrontend vdsBrokerFrontend; https://gerrit.ovirt.org/#/c/35277/28/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java: Line 51: public static final int VM_SPACE_IN_MB = 2000; Line 52: Line 53: @Rule Line 54: public MockConfigRule mcr = new MockConfigRule( Line 55: mockConfig(ConfigValues.CriticalSpaceThreshold, THRESHOLD_IN_GB)); The command should now take this value from the domain - hence this mocking should be removed. Line 56: Line 57: @Before Line 58: public void setUp() { Line 59: VmHandler.init(); https://gerrit.ovirt.org/#/c/35277/28/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/storage/MultipleStorageDomainsValidatorTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/storage/MultipleStorageDomainsValidatorTest.java: Line 40: Line 41: private final static int CRITICAL_SPACE_THRESHOLD = 5; Line 42: Line 43: @ClassRule Line 44: public static MockConfigRule mcr = new MockConfigRule(mockConfig(ConfigValues.CriticalSpaceThreshold, 10)); The command should now take this value from the domain - hence this mocking should be removed. Line 45: Line 46: @Mock Line 47: private StorageDomainDAO dao; Line 48: https://gerrit.ovirt.org/#/c/35277/28/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/storage/StorageDomainValidatorTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/storage/StorageDomainValidatorTest.java: Line 28: private final static int CRITICAL_SPACE_THRESHOLD = 5; Line 29: Line 30: @ClassRule Line 31: public static MockConfigRule mcr = new MockConfigRule( Line 32: mockConfig(ConfigValues.CriticalSpaceThreshold, CRITICAL_SPACE_THRESHOLD) The class should now take this value from the domain - hence this mocking should be removed. Line 33: ); Line 34: Line 35: @Before Line 36: public void setUp() { https://gerrit.ovirt.org/#/c/35277/28/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StorageDomain.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StorageDomain.java: Line 137: Integer availableSize = getAvailableDiskSize(); Line 138: return availableSize != null ? availableSize * SizeConverter.BYTES_IN_GB : null; Line 139: } Line 140: Line 141: public Integer getLowSpaceThreshold() { This should be a primitive int - symmetrically to setLowSpaceThreshold Line 142: return staticData.getLowSpaceThreshold(); Line 143: } Line 144: Line 145: public void setLowSpaceThreshold(int lowSpaceThreshold) { Line 145: public void setLowSpaceThreshold(int lowSpaceThreshold) { Line 146: staticData.setLowSpaceThreshold(lowSpaceThreshold); Line 147: } Line 148: Line 149: public Integer getCriticalSpaceThreshold() { same here Line 150: return staticData.getCriticalSpaceThreshold(); Line 151: } Line 152: Line 153: public void setCriticalSpaceThreshold(int criticalSpaceThreshold) { https://gerrit.ovirt.org/#/c/35277/28/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/StorageDomainMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/StorageDomainMapper.java: Line 58: } Line 59: if (model.isSetLowSpaceThreshold()) { Line 60: entity.setLowSpaceThreshold((model.getLowSpaceThreshold())); Line 61: } else if (entity.getLowSpaceThreshold() == -1) { Line 62: entity.setLowSpaceThreshold(Config.<Integer>getValue(ConfigValues.LowSpaceThreshold)); Formatter - you're missing a space after <Integer> Line 63: } Line 64: if (model.isSetCriticalSpaceThreshold()) { Line 65: entity.setCriticalSpaceThreshold((model.getCriticalSpaceThreshold())); Line 66: } else if (entity.getCriticalSpaceThreshold() == -1) { Line 63: } Line 64: if (model.isSetCriticalSpaceThreshold()) { Line 65: entity.setCriticalSpaceThreshold((model.getCriticalSpaceThreshold())); Line 66: } else if (entity.getCriticalSpaceThreshold() == -1) { Line 67: entity.setCriticalSpaceThreshold(Config.<Integer>getValue(ConfigValues.CriticalSpaceThreshold)); Same formatting comment Line 68: } Line 69: return entity; Line 70: } Line 71: https://gerrit.ovirt.org/#/c/35277/28/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java: Line 434: if (freeDiskInGB != null) { Line 435: if (freePercent < domainFromDb.getLowSpaceThreshold()) { Line 436: type = AuditLogType.IRS_DISK_SPACE_LOW; Line 437: } Line 438: if (freeDiskInGB < domainFromDb.getCriticalSpaceThreshold()) { //This one takes in case both thresholds are met This comment does not seem right - you can skip the first if and only hit this one, no? Line 439: type = AuditLogType.IRS_DISK_SPACE_LOW_ERROR; Line 440: } Line 441: } Line 442: https://gerrit.ovirt.org/#/c/35277/28/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/storage/NfsStorageView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/storage/NfsStorageView.java: Line 205: setElementVisibility(retransmissionsLabel, object.getRetransmissions().getIsAvailable()); Line 206: setElementVisibility(timeoutLabel, object.getTimeout().getIsAvailable()); Line 207: setElementVisibility(mountOptionsLabel, object.getMountOptions().getIsAvailable()); Line 208: Line 209: // When all advanced fields are unavailable - hide the expander. huh? Line 210: boolean anyField = object.getVersion().getIsAvailable() Line 211: || object.getRetransmissions().getIsAvailable() Line 212: || object.getTimeout().getIsAvailable() Line 213: || object.getMountOptions().getIsAvailable(); https://gerrit.ovirt.org/#/c/35277/28/packaging/dbscripts/upgrade/03_06_1181_add_thresholds_to_storage_domain_static.sql File packaging/dbscripts/upgrade/03_06_1181_add_thresholds_to_storage_domain_static.sql: Line 1: select fn_db_add_column('storage_domain_static', 'low_space_threshold', 'INTEGER NOT NULL DEFAULT 0'); Line 2: select fn_db_add_column('storage_domain_static', 'critical_space_threshold', 'INTEGER NOT NULL DEFAULT 0'); Line 3: Line 4: UPDATE storage_domain_Static please have "Static" with a lower-case s Line 5: SET low_space_threshold=cast(( Line 6: SELECT option_value Line 7: FROM vdc_options Line 8: WHERE option_name='LowSpaceThreshold' AND version='general') AS INTEGER), Line 5: SET low_space_threshold=cast(( Line 6: SELECT option_value Line 7: FROM vdc_options Line 8: WHERE option_name='LowSpaceThreshold' AND version='general') AS INTEGER), Line 9: critical_space_threshold=cast(( There's a redundant space here before "critical" Line 10: SELECT option_value Line 11: FROM vdc_options -- To view, visit https://gerrit.ovirt.org/35277 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I19621dfc770c69003d731a7593d037d7d4040a82 Gerrit-PatchSet: 28 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vered Volansky <vvola...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
