Liron Ar has posted comments on this change. Change subject: core: Disallow RO LUN ISCSI disks ......................................................................
Patch Set 2: (4 comments) 1. please add the same checks in AttachDIskToVm and UpdateVmDiskCommand, otherwise we can get to the same end result. 2. what about existing disks that suffer from that issue? possibly we can add a db script here to change the interface if they can't run with that configuration. http://gerrit.ovirt.org/#/c/27354/2//COMMIT_MSG Commit Message: Line 5: CommitDate: 2014-05-05 08:23:31 +0300 Line 6: Line 7: core: Disallow RO LUN ISCSI disks Line 8: Line 9: Qemu currently does not support Direct-LUN disks connected using /s/Direct-LUN disks/RO Direct-LUN disks Line 10: Virt-IO-SCSI. Line 11: This patch adds a validation to block this in AddDiskCommand CDA. Line 12: Also added a new VdcBllMessage and a test for this case. Line 13: http://gerrit.ovirt.org/#/c/27354/2/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 106: Line 107: return ValidationResult.VALID; Line 108: } Line 109: Line 110: public ValidationResult isReadOnlyPropertyCompatibleWithLunInterface() { 1. please remove the "Lun" from the methods name, as this resides in DiskValidator, if you want you can perform the check when the disk.getDiskStorageType() is LUN, otherwise return valid result 2. this method is very similar to the previous one (isReadOnlyPropertyCompatibleWithInterface), I'd merge them to one method that'll get the interface/message as the parameters. Line 111: if (Boolean.TRUE.equals(disk.getReadOnly()) && disk.getDiskInterface() == DiskInterface.VirtIO_SCSI) { Line 112: return new ValidationResult( Line 113: VdcBllMessages.ACTION_TYPE_FAILED_VIRT_IO_SCSI_INTERFACE_FOR_LUN_TARGETS_DOES_NOT_SUPPORT_READ_ONLY_ATTR); Line 114: } http://gerrit.ovirt.org/#/c/27354/2/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties: Line 1183: ISCSI_BOND_NOT_EXIST=Cannot ${action} ${type}. The specified iSCSI bond doesn't exist. Line 1184: 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 1185: Line 1186: ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR=Cannot ${action} ${type}. An IDE disk can't be read-only. Line 1187: ACTION_TYPE_FAILED_VIRT_IO_SCSI_INTERFACE_FOR_LUN_TARGETS_DOES_NOT_SUPPORT_READ_ONLY_ATTR=Cannot ${action} ${type}. A VirtIO-SCSI LUN target can't be read-only. 1. /s/target/disk 2.we can "merge" it with the previous message. http://gerrit.ovirt.org/#/c/27354/2/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 1011: ISCSI_BOND_NOT_EXIST=Cannot ${action} ${type}. The specified iSCSI bond doesn't exist. Line 1012: 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 1013: Line 1014: ACTION_TYPE_FAILED_IDE_INTERFACE_DOES_NOT_SUPPORT_READ_ONLY_ATTR=Cannot ${action} ${type}. An IDE disk can't be read-only. Line 1015: ACTION_TYPE_FAILED_VIRT_IO_SCSI_INTERFACE_FOR_LUN_TARGETS_DOES_NOT_SUPPORT_READ_ONLY_ATTR=Cannot ${action} ${type}. A VirtIO-SCSI LUN target can't be read-only. we can "merge" it with the previous message. -- To view, visit http://gerrit.ovirt.org/27354 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id84346098d42f20c593c2682dc9ff99a3e77d534 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vered Volansky <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: Vered Volansky <[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
