Sergey Gotliv has posted comments on this change.
Change subject: engine: Disk interface validation
......................................................................
Patch Set 7:
(6 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllAttachableDisksQuery.java
Line 19:
Line 20: public class GetAllAttachableDisksQuery<P extends
GetAllAttachableDisks> extends QueriesCommandBase<P> {
Line 21:
Line 22: private static final Log LOG =
LogFactory.getLog(GetAllAttachableDisksQuery.class);
Line 23:
Same as in DiskValidator
Line 24: public GetAllAttachableDisksQuery(P parameters) {
Line 25: super(parameters);
Line 26: }
Line 27:
Line 55: } catch (IllegalArgumentException e) {
Line 56: // ignore if we can't find the enum value.
Line 57: LOG.errorFormat("Can't find the value {0} in the enum
DiskInterface. Error: {1}",
Line 58: ((interfaceName == null) ? "null" :
interfaceName),
Line 59: e.toString());
Same as in DiskValidator
Line 60: }
Line 61: }
Line 62:
Line 63: List<Disk> filteredDiskList = new ArrayList<Disk>();
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
Line 152: }
Line 153:
Line 154: DiskValidator diskValidator = getDiskValidator(getNewDisk());
Line 155: return validateCanUpdateShareable() &&
validate(diskValidator.isVirtIoScsiValid(getVm())) &&
Line 156:
validate(diskValidator.isDiskInterfaceSupported(getVm()));
Are you rebased? The code here is looking different:
DiskValidator diskValidator = getDiskValidator(getNewDisk());
return validateCanUpdateShareable() && validateCanUpdateReadOnly() &&
validate(diskValidator.isVirtIoScsiValid(getVm()));
Line 157: }
Line 158:
Line 159: @Override
Line 160: protected void setActionMessageParameters() {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java
Line 26: public class DiskValidator {
Line 27:
Line 28: private Disk disk;
Line 29: private static final Log LOG =
LogFactory.getLog(DiskValidator.class);
Line 30:
It's just a matter of style but please change to log.
Line 31: public DiskValidator(Disk disk) {
Line 32: this.disk = disk;
Line 33: }
Line 34:
Line 112: } catch (IllegalArgumentException e) {
Line 113: // ignore if we can't find the enum value.
Line 114: LOG.errorFormat("Can't find the value {0} in the enum
DiskInterface. Error: {1}",
Line 115: ((interfaceName == null) ? "null" :
interfaceName),
Line 116: e.toString());
Can interfaceName be a null?
What is the added value of e.toString here? The message already contains the
problematic value and the enum name and we know which exception is thrown.
Line 117: }
Line 118: }
Line 119:
Line 120: return supportedDiskInterfaces;
....................................................
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
Line 145: ACTION_TYPE_FAILED_VM_HAS_PLUGGED_DISK_SNAPSHOT=Cannot ${action}
${type}. The following VM's activated disks are disk snapshots:\n\
Line 146: ${disksInfo}. \n\
Line 147: Please deactivate them and try again.
Line 148: ACTION_TYPE_FAILED_DISKS_LOCKED=Cannot ${action} ${type}: The
following disks are locked: ${diskAliases}. Please try again in a few minutes.
Line 149: ACTION_TYPE_DISK_INTERFACE_UNSUPPORTED=Cannot ${action} ${type}: The
disk interface is not supported by the VM.
The disk interface is not supported by VM OS, probably message should contain
the OS name.
Line 150: ACTION_TYPE_FAILED_DISKS_ILLEGAL=Cannot ${action} ${type}. The
following attached disks are in ILLEGAL status: ${diskAliases} - please remove
them and try again.
Line 151: ACTION_TYPE_FAILED_MOVE_DISKS_MIXED_PLUGGED_STATUS=Cannot ${action}
${type}. The following disks could not be moved: ${diskAliases}. Please make
sure that all disks are active or inactive in the VM.
Line 152: ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST=Cannot ${action}
${type}. The following disks already exist: ${diskAliases}. Please import as a
clone.
Line 153: ACTION_TYPE_FAILED_VM_IS_LOCKED=Cannot ${action} ${type}: VM is
locked. Please try again in a few minutes.
--
To view, visit http://gerrit.ovirt.org/18648
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe095557089aa5670c50eaa120eac9f60e13aea0
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gustavo Frederico Temple Pedrosa <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Gustavo Frederico Temple Pedrosa
<[email protected]>
Gerrit-Reviewer: Leonardo Bianconi <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
Gerrit-Reviewer: Vitor de Lima <[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
