Vered Volansky has posted comments on this change. Change subject: core: Add storage space thresholds support ......................................................................
Patch Set 23: (17 comments) https://gerrit.ovirt.org/#/c/35277/23//COMMIT_MSG Commit Message: Line 10: LowSpaceThreshold (Percentage) and CriticalSpaceThreshold (absolute value in GB). Line 11: The initial and default values for these thresholds are taken from Line 12: configuration/vdc_options table. Line 13: These replace the previously named FreeSpaceLow and Line 14: FreeSpaceCriticalLowInGB configuration values. > Worth mentioning that these conf values still exist, and act as a "default Done Line 15: Line 16: Change-Id: I19621dfc770c69003d731a7593d037d7d4040a82 Line 17: Bug-Url: https://bugzilla.redhat.com/679070 Line 18: Bug-Url: https://bugzilla.redhat.com/1185839 https://gerrit.ovirt.org/#/c/35277/23/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageDomainCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageDomainCommand.java: Line 54: props.remove("description"); Line 55: props.remove("comment"); Line 56: props.remove("wipeAfterDelete"); Line 57: props.remove("lowSpaceThreshold"); Line 58: props.remove("criticalSpaceThreshold"); > This is becoming way too cumbersome. We need to find a better solution for Agreed Line 59: if (!props.isEmpty()) { Line 60: log.warn("There was an attempt to update the following fields although they are not allowed to be updated: {}", Line 61: StringUtils.join(props, ",")); Line 62: return failCanDoAction(VdcBllMessages.ERROR_CANNOT_CHANGE_STORAGE_DOMAIN_FIELDS); https://gerrit.ovirt.org/#/c/35277/23/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 Done 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/23/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 Done Line 612: } Line 613: Line 614: private void mockConfigSizeDefaults() { Line 615: int requiredSpaceBufferInGB = 5; https://gerrit.ovirt.org/#/c/35277/23/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 Done 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/23/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 Done Line 55: Line 56: @Mock Line 57: private DiskImageDAO diskImageDao; Line 58: @Mock https://gerrit.ovirt.org/#/c/35277/23/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 Done Line 127: } Line 128: Line 129: private void mockConfigSizeDefaults() { Line 130: int requiredSpaceBufferInGB = 5; https://gerrit.ovirt.org/#/c/35277/23/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 Done Line 57: ); Line 58: Line 59: @Mock Line 60: private VDSBrokerFrontend vdsBrokerFrontend; https://gerrit.ovirt.org/#/c/35277/23/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 50: public static final int VM_SPACE_IN_MB = 2000; Line 51: Line 52: @Rule Line 53: public MockConfigRule mcr = new MockConfigRule( Line 54: mockConfig(ConfigValues.CriticalSpaceThreshold, THRESHOLD_IN_GB)); > The command should now take this value from the domain - hence this mocking Done Line 55: Line 56: @Before Line 57: public void setUp() { Line 58: VmHandler.init(); https://gerrit.ovirt.org/#/c/35277/23/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 Done Line 45: Line 46: @Mock Line 47: private StorageDomainDAO dao; Line 48: https://gerrit.ovirt.org/#/c/35277/23/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 command should now take this value from the domain - hence this mocking Done Line 33: ); Line 34: Line 35: @Before Line 36: public void setUp() { https://gerrit.ovirt.org/#/c/35277/23/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties: Line 571: ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL=Cannot ${action} ${type}. Storage Domain format ${storageFormat} is illegal. Line 572: ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL_HOST=Cannot ${action} ${type}. Storage format ${storageFormat} is not supported on the selected host version. Line 573: ERROR_CANNOT_EXTEND_NON_DATA_DOMAIN=Cannot extend Storage Domain. Extend operation is supported only on Data Storage Domain. Line 574: ERROR_CANNOT_EXTEND_CONNECTION_FAILED=Cannot extend Storage Domain. Storage device ${lun} is unreachable on ${hostName}. Line 575: ERROR_CANNOT_CHANGE_STORAGE_DOMAIN_FIELDS=Cannot ${action} ${type}. Can only update the following fields: name, description, comment, wipe after delete, low space threshold, critical space threshold. > use "and" instead of a comma before the last field in the list American English mandates the use of the comma as well as the coordinating conjunction. Organization standards may apply, but I suppose we don't have any, so I'm using the common American style (meaning I added "and" after the comma, not instead). Line 576: ERROR_CANNOT_UPDATE_STORAGE_POOL_COMPATIBILITY_VERSION_BIGGER_THAN_CLUSTERS=Cannot update Data Center compatibility version to a value that is greater than its Cluster's version. The following clusters should be upgraded ${ClustersList}. Line 577: ERROR_CANNOT_ADD_EXISTING_STORAGE_DOMAIN_CONNECTION_DATA_ILLEGAL=Cannot import Storage Domain. Internal Error: The connection data is illegal. Line 578: ERROR_CANNOT_ADD_DEPRECATED_EXISTING_SAN_EXPORT_STORAGE_DOMAIN=Cannot import SAN Export Storage Domain as it is no longer supported. Line 579: NETWORK_MAC_ADDRESS_IN_USE=MAC Address is already in use. https://gerrit.ovirt.org/#/c/35277/23/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java: Line 1578: Line 1579: @DefaultStringValue("Cannot extend Storage Domain. Storage device ${lun} is unreachable from ${hostName}.") Line 1580: String ERROR_CANNOT_EXTEND_CONNECTION_FAILED(); Line 1581: Line 1582: @DefaultStringValue("Cannot ${action} ${type}. Cannot ${action} ${type}. Can only update the following fields: name, description, comment, wipe after delete, low space threshold, critical space threshold.") > You should use an "and" instead of a comma before the last field in the lis Added and, not replaced, following American English standard. Line 1583: String ERROR_CANNOT_CHANGE_STORAGE_DOMAIN_FIELDS(); Line 1584: Line 1585: @DefaultStringValue("Cannot update Data Center compatibility version to a value that is greater than its Cluster's version. The following clusters should be upgraded ${ClustersList}.") Line 1586: String ERROR_CANNOT_UPDATE_STORAGE_POOL_COMPATIBILITY_VERSION_BIGGER_THAN_CLUSTERS(); https://gerrit.ovirt.org/#/c/35277/23/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties File frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties: Line 525: ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL=Cannot ${action} ${type}. Storage Domain format ${storageFormat} is illegal. Line 526: ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL_HOST=Cannot ${action} ${type}. Storage format ${storageFormat} is not supported on the selected host version. Line 527: ERROR_CANNOT_EXTEND_NON_DATA_DOMAIN=Cannot extend Storage Domain. Extend operation is supported only on Data Storage Domain. Line 528: ERROR_CANNOT_EXTEND_CONNECTION_FAILED=Cannot extend Storage Domain. Storage device ${lun} is unreachable from ${hostName}. Line 529: ERROR_CANNOT_CHANGE_STORAGE_DOMAIN_FIELDS=Cannot ${action} ${type}. Can only update the following fields: name, description, comment, wipe after delete, low space threshold, critical space threshold. > You should use an "and" instead of a comma before the last field in the lis Added and, not replaced, following American English standard. Line 530: ERROR_CANNOT_UPDATE_STORAGE_POOL_COMPATIBILITY_VERSION_BIGGER_THAN_CLUSTERS=Cannot update Data Center compatibility version to a value that is greater than its Cluster's version. The following clusters should be upgraded ${ClustersList}. Line 531: ERROR_CANNOT_ADD_EXISTING_STORAGE_DOMAIN_CONNECTION_DATA_ILLEGAL=Cannot import Storage Domain. Internal Error: The connection data is illegal. Line 532: ERROR_CANNOT_ADD_DEPRECATED_EXISTING_SAN_EXPORT_STORAGE_DOMAIN=Cannot import SAN Export Storage Domain as it is no longer supported. Line 533: ERROR_CANNOT_ADD_EXISTING_STORAGE_DOMAIN_LUNS_PROBLEM=Cannot import Storage Domain. Not all LUNs connected to Storage Domain. https://gerrit.ovirt.org/#/c/35277/23/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties File frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties: Line 576: ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL=Cannot ${action} ${type}. Storage Domain format ${storageFormat} is illegal. Line 577: ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL_HOST=Cannot ${action} ${type}. Storage format ${storageFormat} is not supported on the selected host version. Line 578: ERROR_CANNOT_EXTEND_NON_DATA_DOMAIN=Cannot extend Storage Domain. Extend operation is supported only on Data Storage Domain. Line 579: ERROR_CANNOT_EXTEND_CONNECTION_FAILED=Cannot extend Storage Domain. Storage device ${lun} is unreachable from ${hostName}. Line 580: ERROR_CANNOT_CHANGE_STORAGE_DOMAIN_FIELDS=Cannot ${action} ${type}. Can only update the following fields: name, description, comment, wipe after delete, low space threshold, critical space threshold. > You should use an "and" instead of a comma before the last field in the lis Added and, not replaced, following American English standard. Line 581: ERROR_CANNOT_UPDATE_STORAGE_POOL_COMPATIBILITY_VERSION_BIGGER_THAN_CLUSTERS=Cannot update Data Center compatibility version to a value that is greater than its Cluster's version. The following clusters should be upgraded ${ClustersList}. Line 582: ERROR_CANNOT_ADD_EXISTING_STORAGE_DOMAIN_CONNECTION_DATA_ILLEGAL=Cannot import Storage Domain. Internal Error: The connection data is illegal. Line 583: ERROR_CANNOT_ADD_DEPRECATED_EXISTING_SAN_EXPORT_STORAGE_DOMAIN=Cannot import SAN Export Storage Domain as it is no longer supported. Line 584: NETWORK_MAC_ADDRESS_IN_USE=MAC Address is already in use. https://gerrit.ovirt.org/#/c/35277/23/packaging/dbscripts/inst_sp.sql File packaging/dbscripts/inst_sp.sql: Line 14: Line 15: Line 16: Line 17: -- This function calls insert_server_connections, insertstorage_domain_static,insertstorage_domain_dynamic Line 18: -- Any change to these functions may effect correctness of the installation. > This fix, although correct, is not related to this patch - please do it in Sure I will. Line 19: Line 20: Create or replace FUNCTION inst_add_iso_storage_domain(v_storage_domain_id UUID, v_name VARCHAR(250), v_connection_id uuid, v_connection VARCHAR(250),v_available int, v_used int) Line 21: RETURNS VOID Line 22: AS $procedure$ https://gerrit.ovirt.org/#/c/35277/23/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql File packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql: Line 873: select fn_db_delete_config_value('FreeSpaceCriticalLow','general'); Line 874: select fn_db_delete_config_value('GlusterRefreshRateGeoRepStatus', 'general'); Line 875: select fn_db_delete_config_value('GlusterRefreshRateGeoRepDiscovery', 'general'); Line 876: select fn_db_delete_config_value('FreeSpaceCriticalLowInGB','general'); Line 877: select fn_db_delete_config_value('FreeSpaceLow','general'); > Yes - iiuc, you're logically attempting to rename a conf value. This will r This should stay here, should also add *rename_key*, in later patch. This is confirmed with Eli. Line 878: select fn_db_delete_config_value('HotPlugUnsupportedOsList','general'); Line 879: select fn_db_delete_config_value('HotPlugSupportedOsList','general'); Line 880: select fn_db_delete_config_value('ImagesSyncronizationTimeout','general'); Line 881: select fn_db_delete_config_value('keystorePass','general'); -- 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: 23 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
