Maor Lipchuk has posted comments on this change. Change subject: core: DiskImagesValidator - validate images belong to same Disk ......................................................................
Patch Set 4: (3 comments) http://gerrit.ovirt.org/#/c/26448/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java: Line 196: Guid imageGroupId = null; Line 197: for (DiskImage diskImage : diskImages) { Line 198: if (imageGroupId == null || diskImage.getId().equals(imageGroupId)) { Line 199: imageGroupId = diskImage.getId(); Line 200: continue; How you can be sure that the first disk id is the right one? Line 201: } Line 202: Line 203: return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_SNAPSHOTS_DONT_BELONG_TO_SAME_DISK, Line 204: String.format("diskalias %s", diskImage.getDiskAlias())); Line 200: continue; Line 201: } Line 202: Line 203: return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_SNAPSHOTS_DONT_BELONG_TO_SAME_DISK, Line 204: String.format("diskalias %s", diskImage.getDiskAlias())); You might want to indicate the original disk alias also, since the first disk might be the wrong one Line 205: } Line 206: Line 207: return ValidationResult.VALID; Line 208: } http://gerrit.ovirt.org/#/c/26448/4/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties: Line 11: MAC_POOL_NOT_ENOUGH_MAC_ADDRESSES=Not enough MAC addresses left in MAC Address Pool. Line 12: VMT_CANNOT_REMOVE_DETECTED_DERIVED_VM=Cannot delete Template. Template is being used by the following VMs: ${vmsList}. Line 13: VMT_CANNOT_REMOVE_BASE_WITH_VERSIONS=Cannot delete Base Template that has Template Versions, please first remove all Template Versions for this Template: ${versionsList}. Line 14: ACTION_TYPE_FAILED_DETECTED_DERIVED_DISKS=Cannot ${action} ${type}. The following Disk(s) are based on it: \n ${disksInfo}. Line 15: ACTION_TYPE_FAILED_DISKS_SNAPSHOTS_DONT_BELONG_TO_SAME_DISK="Cannot ${action} ${type}. The specified disks snapshots don't belong to the same Disk: ${diskAlias}." Maybe a more appropriate message should be: Cannot ${action} ${type}. The specified disks snapshots don't share the same Disk: ${diskAlias1} and ${diskAlias2}. Line 16: VMT_CANNOT_REMOVE_DOMAINS_LIST_MISMATCH=Cannot delete Template. The Template does not exist on the following Storage Domains: ${domainsList}.\nEither verify that Template exists on all Storage Domains listed on the domains list,\nor do not send domains list in order to delete all instances of the Template from the system. Line 17: ACTION_TYPE_FAILED_VM_IMAGE_DOES_NOT_EXIST=Cannot ${action} ${type}. VM's Image does not exist. Line 18: ACTION_TYPE_FAILED_VM_SNAPSHOT_DOES_NOT_EXIST=Cannot ${action} ${type}. VM's Snapshot does not exist. Line 19: ACTION_TYPE_FAILED_VM_SNAPSHOT_TYPE_NOT_ALLOWED=Cannot ${action} ${type}. The Snapshot type is ${snapshotType} while the operation is supported for Snapshots of the following type(s): ${supportedSnapshotTypes}. -- To view, visit http://gerrit.ovirt.org/26448 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I52c4a31cbe3d93f582b710910dd9cd1a51ab50b2 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[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
