Allon Mureinik has posted comments on this change.

Change subject: core: Limitation addition for downgrading DC's compatible 
version.
......................................................................


Patch Set 5: Code-Review-1

(12 comments)

http://gerrit.ovirt.org/#/c/35790/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommand.java:

Line 214:         StoragePoolValidator validator = createStoragePoolValidator();
Line 215:         return validate(validator.isNotLocalfsWithDefaultCluster());
Line 216:     }
Line 217: 
Line 218:     private boolean 
isDowngradeEnabledByDomains(List<StorageDomainStatic> poolDomains) {
how about "isDowngradeAllowedForDomains" ?
Line 219:         List<String> formatProblematicDomains = new ArrayList<>();
Line 220:         List<String> typeProblematicDomains = new ArrayList<>();
Line 221:         boolean isMixedSupportFailing = false;
Line 222: 


Line 217: 
Line 218:     private boolean 
isDowngradeEnabledByDomains(List<StorageDomainStatic> poolDomains) {
Line 219:         List<String> formatProblematicDomains = new ArrayList<>();
Line 220:         List<String> typeProblematicDomains = new ArrayList<>();
Line 221:         boolean isMixedSupportFailing = false;
Default should be true - until you find a domain that fails this check
Line 222: 
Line 223:         for (StorageDomainStatic domainStatic : poolDomains) {
Line 224:             StorageDomain domain = new StorageDomain();
Line 225:             domain.setStorageStaticData(domainStatic);


Line 226: 
Line 227:             AttachDomainValidator attachDomainValidator = 
getAttachDomainValidator(domain);
Line 228: 
Line 229:             if 
(!attachDomainValidator.isStorageDomainTypeFitsPoolIfMixed().isValid()) {
Line 230:                 isMixedSupportFailing = true;
Don't you want to remember this name somewhere?
Line 231:             }
Line 232:             if 
(!attachDomainValidator.isStorageDomainTypeSupportedInPool().isValid()) {
Line 233:                 typeProblematicDomains.add(domain.getName());
Line 234:             }


Line 236:                 formatProblematicDomains.add(domain.getName());
Line 237:             }
Line 238:         }
Line 239: 
Line 240:         int numberOfProblematicDomainsToShow = 10;
Isn't this the default of ReplacementUtils?
Line 241:         if (isMixedSupportFailing) {
Line 242:             
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_MIXED_STORAGE_TYPES_NOT_ALLOWED);
Line 243:         }
Line 244:         if (!formatProblematicDomains.isEmpty()) {


http://gerrit.ovirt.org/#/c/35790/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/AttachDomainValidator.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/storage/AttachDomainValidator.java:

Line 109:                     
storageDomain.getStorageType()).compareTo(storageDomain.getStorageFormat()) < 
0) {
Line 110: 
Line 111:                 return new 
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL,
 String.format(
Line 112:                         "$storageFormat %1$s",
Line 113:                         storageDomain.getStorageFormat().toString()));
Formatting changes are unrelated to the patch - please remove them or send them 
in a separate cleanup patch.
Line 114:             }
Line 115:         }
Line 116:         return ValidationResult.VALID;
Line 117:     }


http://gerrit.ovirt.org/#/c/35790/5/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommandTest.java
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/RemoveStorageDomainCommandTest.java:

Line 160:         doReturn(ret).when(command).runVdsCommand
Line 161:                 (eq(VDSCommandType.FormatStorageDomain), 
any(FormatStorageDomainVDSCommandParameters.class));
Line 162:     }
Line 163: 
Line 164:     private class StorageDomainValidatorForTesting extends 
StorageDomainValidator{
please restore the whitespace before "{"
Line 165:         public StorageDomainValidatorForTesting(StorageDomain domain) 
{
Line 166:             super(domain);
Line 167:         }
Line 168: 


http://gerrit.ovirt.org/#/c/35790/5/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommandTest.java
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStoragePoolCommandTest.java:

Line 224:     }
Line 225: 
Line 226:     @Test
Line 227:     public void poolHasDefaultCluster() {
Line 228:         
mcr.mockConfigValue(ConfigValues.AutoRegistrationDefaultVdsGroupID, 
DEFAULT_VDS_GROUP_ID);
huh?
Line 229:         addDefaultClusterToPool();
Line 230:         storagePoolWithLocalFS();
Line 231:         
canDoActionFailed(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_WITH_DEFAULT_VDS_GROUP_CANNOT_BE_LOCALFS);
Line 232:     }


Line 435:         assertFalse(cmd.canDoAction());
Line 436:         
assertTrue(cmd.getReturnValue().getCanDoActionMessages().contains(reason.toString()));
Line 437:     }
Line 438: 
Line 439:     protected class StoragePoolValidatorForTesting extends 
StoragePoolValidator {
Why isn't this just spied?
Line 440:         public StoragePoolValidatorForTesting(StoragePool 
storagePool) {
Line 441:             super(storagePool);
Line 442:         }
Line 443: 


http://gerrit.ovirt.org/#/c/35790/5/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
File 
backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties:

Line 675: ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME=Cannot ${action} ${type}. 
Illegal Domain name: ${Domain}. Domain name has unsupported special character 
${Char}.
Line 676: ACTION_TYPE_FAILED_ILLEGAL_ARCHITECTURE_TYPE_INCOMPATIBLE=Cannot 
${action} ${type}. Architecture does not match the expected value.
Line 677: ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION=Cannot 
decrease data center compatibility version.
Line 678: 
ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION_UNDER_DC=Cannot 
decrease cluster compatibility version beneath data center compatibility 
version.
Line 679: 
ACTION_TYPE_FAILED_DECREASING_COMPATIBILITY_VERSION_CAUSES_STORAGE_FORMAT_DOWNGRADING=Cannot
 ${action} ${type}. this action will cause storage format downgrading which is 
not supported. The following storage domains cannot be downgraded: 
${formatDowngradedDomains}.
s/. this/. This/
Line 680: 
ACTION_TYPE_FAILED_STORAGE_DOMAINS_ARE_NOT_SUPPORTET_IN_DOWNGRADED_VERSION=Cannot
 ${action} ${type}. The following storage domains are not supported in the 
selected version: ${unsupportedVersionDomains}.
Line 681: ACTION_TYPE_FAILED_GIVEN_VERSION_NOT_SUPPORTED=Cannot ${action} 
${type}. Selected Compatibility Version is not supported.
Line 682: 
ACTION_TYPE_FAILED_DATA_CENTER_VERSION_DOESNT_SUPPORT_LIVE_SNAPSHOT=Cannot 
${action} ${type}. Selected data center compatibility version does not support 
live snapshot.
Line 683: NETWORK_ADDR_MANDATORY_IN_STATIC_IP=Network address must be specified 
when using static IP


http://gerrit.ovirt.org/#/c/35790/5/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 1848: 
Line 1849:     @DefaultStringValue("Cannot decrease data center compatibility 
version.")
Line 1850:     String 
ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION();
Line 1851: 
Line 1852:     @DefaultStringValue("Cannot ${action} ${type}. this action will 
cause storage format downgrading which is not supported. The following storage 
domains cannot be downgraded: ${formatDowngradedDomains}.")
s/. this/. This/
Line 1853:     String 
ACTION_TYPE_FAILED_DECREASING_COMPATIBILITY_VERSION_CAUSES_STORAGE_FORMAT_DOWNGRADING();
Line 1854: 
Line 1855:     @DefaultStringValue("Cannot ${action} ${type}. The following 
storage domains are not supported in the selected version: 
${unsupportedVersionDomains}.")
Line 1856:     String 
ACTION_TYPE_FAILED_STORAGE_DOMAINS_ARE_NOT_SUPPORTET_IN_DOWNGRADED_VERSION();


http://gerrit.ovirt.org/#/c/35790/5/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 629: ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME=Cannot ${action} ${type}. 
Illegal Domain name: ${Domain}. Domain name has unsupported special character 
${Char}.
Line 630: ACTION_TYPE_FAILED_ILLEGAL_ARCHITECTURE_TYPE_INCOMPATIBLE=Cannot 
${action} ${type}. Architecture does not match the expected value.
Line 631: ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION=Cannot 
decrease data center compatibility version.
Line 632: 
ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION_UNDER_DC=Cannot 
decrease cluster compatibility version beneath data center compatibility 
version.
Line 633: 
ACTION_TYPE_FAILED_DECREASING_COMPATIBILITY_VERSION_CAUSES_STORAGE_FORMAT_DOWNGRADING=Cannot
 ${action} ${type}. this action will cause storage format downgrading which is 
not supported. The following storage domains cannot be downgraded: 
${formatDowngradedDomains}.
s/. this/. This/
Line 634: 
ACTION_TYPE_FAILED_STORAGE_DOMAINS_ARE_NOT_SUPPORTET_IN_DOWNGRADED_VERSION=Cannot
 ${action} ${type}. The following storage domains are not supported in the 
selected version: ${unsupportedVersionDomains}.
Line 635: ACTION_TYPE_FAILED_GIVEN_VERSION_NOT_SUPPORTED=Cannot ${action} 
${type}. Selected Compatibility Version is not supported.
Line 636: 
ACTION_TYPE_FAILED_DATA_CENTER_VERSION_DOESNT_SUPPORT_LIVE_SNAPSHOT=Cannot 
${action} ${type}. Selected data center compatibility version does not support 
live snapshot.
Line 637: NETWORK_ADDR_MANDATORY_IN_STATIC_IP=Network address must be specified 
when using static ip


http://gerrit.ovirt.org/#/c/35790/5/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 675: ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME=Cannot ${action} ${type}. 
Illegal Domain name: ${Domain}. Domain name has unsupported special character 
${Char}.
Line 676: ACTION_TYPE_FAILED_ILLEGAL_ARCHITECTURE_TYPE_INCOMPATIBLE=Cannot 
${action} ${type}. Architecture does not match the expected value.
Line 677: ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION=Cannot 
decrease data center compatibility version.
Line 678: 
ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION_UNDER_DC=Cannot 
decrease cluster compatibility version beneath data center compatibility 
version.
Line 679: 
ACTION_TYPE_FAILED_DECREASING_COMPATIBILITY_VERSION_CAUSES_STORAGE_FORMAT_DOWNGRADING=Cannot
 ${action} ${type}. this action will cause storage format downgrading which is 
not supported. The following storage domains cannot be downgraded: 
${formatDowngradedDomains}.
s/. this/. This/
Line 680: 
ACTION_TYPE_FAILED_STORAGE_DOMAINS_ARE_NOT_SUPPORTET_IN_DOWNGRADED_VERSION=Cannot
 ${action} ${type}. The following storage domains are not supported in the 
selected version: ${unsupportedVersionDomains}.
Line 681: ACTION_TYPE_FAILED_GIVEN_VERSION_NOT_SUPPORTED=Cannot ${action} 
${type}. Selected Compatibility Version is not supported.
Line 682: 
ACTION_TYPE_FAILED_DATA_CENTER_VERSION_DOESNT_SUPPORT_LIVE_SNAPSHOT=Cannot 
${action} ${type}. Selected data center compatibility version does not support 
live snapshot.
Line 683: NETWORK_ADDR_MANDATORY_IN_STATIC_IP=Network address must be specified 
when using static ip


-- 
To view, visit http://gerrit.ovirt.org/35790
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib97488c705a32fe73b390ef226b1ad19a3784b76
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Amit Aviram <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Amit Aviram <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: [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

Reply via email to