Allon Mureinik has posted comments on this change.

Change subject: engine: Validate that IDE disk is not read-only
......................................................................


Patch Set 3:

(2 comments)

@Sergey - basically, +1, see comments inline.

@Cheryn - can you take a look at 
backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties 
please?

http://gerrit.ovirt.org/#/c/23953/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java:

Line 98:         return ValidationResult.VALID;
Line 99: 
Line 100:     }
Line 101: 
Line 102:     public ValidationResult isDiskInterfaceSupportReadOnly() {
This name is confusing - any disk that is r/w would return a VALID result here 
- which is fine on its own right, but not if the method is called 
isDiskInterfaceSupportReadOnly :-)

Perhaps - isReadOnlyPropertyCompatibleWithInterface() (or something shorter to 
that effect)?
Line 103:         if (Boolean.TRUE.equals(disk.getReadOnly()) && 
disk.getDiskInterface() == DiskInterface.IDE) {
Line 104:             return new 
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR);
Line 105:         }
Line 106: 


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

Line 1148: 
Line 1149: ISCSI_BOND_NOT_EXIST=Cannot ${action} ${type}. The specified iSCSI 
bond doesn't exist.
Line 1150: ISCSI_BOND_WITH_SAME_NAME_EXIST_IN_DATA_CENTER=Cannot ${action} 
${type}. iSCSI bond with the same name already exists in the Data Center.
Line 1151: 
Line 1152: ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR=IDE 
disk can't be read-only.
IMHO, either "IDE disks" or "An IDE disk".
Cheryn?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I441362adf2f4833b034ede4093fc8195debd2ed5
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Cheryn Tan <cheryn...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
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

Reply via email to