Maor Lipchuk has posted comments on this change. Change subject: core: Limitation addition for downgrading DC's compatible version. ......................................................................
Patch Set 13: (3 comments) http://gerrit.ovirt.org/#/c/35790/13/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 221: Line 222: for (StorageDomainStatic domainStatic : poolDomains) { Line 223: StorageDomainToPoolRelationValidator attachDomainValidator = getAttachDomainValidator(domainStatic); Line 224: Line 225: if (!failOnSupportedTypeMixing && !attachDomainValidator.isStorageDomainTypeFitsPoolIfMixed().isValid()) { failOnSupportedTypeMixing will always be false, perhaps it is better to remove it from the if condition Line 226: failOnSupportedTypeMixing = true; Line 227: } Line 228: if (!attachDomainValidator.isStorageDomainTypeSupportedInPool().isValid()) { Line 229: typeProblematicDomains.add(domainStatic.getName()); Line 241: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_MIXED_STORAGE_TYPES_NOT_ALLOWED); Line 242: } Line 243: if (!formatProblematicDomains.isEmpty()) { Line 244: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DECREASING_COMPATIBILITY_VERSION_CAUSES_STORAGE_FORMAT_DOWNGRADING); Line 245: getReturnValue().getCanDoActionMessages().addAll(ReplacementUtils.replaceWith("formatDowngradedDomains", formatProblematicDomains, "," , formatProblematicDomains.size())); Suggestion: I would just use the ...replaceWith("unsupportedVersionDomains", typeProblematicDomains), the separator which is passed here "," is already the default separator used in the ReplacementUtils and perhaps it is more better to keep it consistent with the DEFAULT_MAX_NUMBER_OF_PRINTED_ITEMS which is 5 Line 246: } Line 247: if (!typeProblematicDomains.isEmpty()) { Line 248: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAINS_ARE_NOT_SUPPORTET_IN_DOWNGRADED_VERSION); Line 249: getReturnValue().getCanDoActionMessages().addAll(ReplacementUtils.replaceWith("unsupportedVersionDomains", typeProblematicDomains , ",", typeProblematicDomains.size())); Line 245: getReturnValue().getCanDoActionMessages().addAll(ReplacementUtils.replaceWith("formatDowngradedDomains", formatProblematicDomains, "," , formatProblematicDomains.size())); Line 246: } Line 247: if (!typeProblematicDomains.isEmpty()) { Line 248: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAINS_ARE_NOT_SUPPORTET_IN_DOWNGRADED_VERSION); Line 249: getReturnValue().getCanDoActionMessages().addAll(ReplacementUtils.replaceWith("unsupportedVersionDomains", typeProblematicDomains , ",", typeProblematicDomains.size())); Suggestion: I would just use the ...replaceWith("unsupportedVersionDomains", typeProblematicDomains), the separator which is passed here "," is already the default separator used in the ReplacementUtils and perhaps it is more better to keep it consistent with the DEFAULT_MAX_NUMBER_OF_PRINTED_ITEMS which is 5 Line 250: } Line 251: Line 252: return typeProblematicDomains.isEmpty() && formatProblematicDomains.isEmpty() && !failOnSupportedTypeMixing; Line 253: } -- 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: 13 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@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
