Liron Ar has posted comments on this change.
Change subject: core: add the image block alignment scan
......................................................................
Patch Set 7: (7 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDiskAlignmentCommand.java
Line 98: if (getDisk() == null) {
Line 99: return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DISK_NOT_EXIST);
Line 100: }
Line 101:
Line 102: if (getDiskType() == DiskStorageType.IMAGE) {
perhaps use here DiskImageValidator that encapsulate this logic/messages.
Line 103: if (((DiskImage) getDisk()).getImageStatus() ==
ImageStatus.LOCKED) {
Line 104: return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_LOCKED);
Line 105: } else if (((DiskImage) getDisk()).getImageStatus() !=
ImageStatus.OK) {
Line 106: return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_ILLEGAL);
Line 159: } else {
Line 160: throw new VdcBLLException(VdcBllErrors.ENGINE, "Unknown
DiskStorageType: " +
Line 161: getDiskType().toString() + " Disk id: " +
getDisk().getId().toString());
Line 162: }
Line 163:
please remember to add comptabillity check prior to executing
Line 164: Boolean isDiskAligned = (Boolean) runVdsCommand(
Line 165: VDSCommandType.GetDiskAlignment,
parameters).getReturnValue();
Line 166:
Line 167: getDisk().setAlignment(isDiskAligned ? DiskAlignment.Aligned
: DiskAlignment.Misaligned);
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/GetDiskAlignmentParameters.java
Line 5: public class GetDiskAlignmentParameters extends VdcActionParametersBase
{
Line 6: private static final long serialVersionUID = -6587274019503875891L;
Line 7:
Line 8: public GetDiskAlignmentParameters(Guid diskId) {
Line 9: setEntityId(diskId);
entity id is generally used for async tasks, I'd suggest to avoid using it.
Line 10: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DiskAlignment.java
Line 9: NotApplicable(1), // future use, e.g. disks with no partition table
Line 10: Misaligned(2),
Line 11: Aligned(3);
Line 12:
Line 13: private int intValue;
please eliminate the "int" from the name
Line 14: private static final Map<Integer, DiskAlignment> mappings;
Line 15:
Line 16: static {
Line 17: mappings = new HashMap<Integer, DiskAlignment>();
Line 13: private int intValue;
Line 14: private static final Map<Integer, DiskAlignment> mappings;
Line 15:
Line 16: static {
Line 17: mappings = new HashMap<Integer, DiskAlignment>();
please change to EnumMap
Line 18:
Line 19: for (DiskAlignment enumValue : values()) {
Line 20: mappings.put(enumValue.getValue(), enumValue);
Line 21: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/GetDiskLunAlignmentVDSCommandParameters.java
Line 6: import org.ovirt.engine.core.compat.Guid;
Line 7:
Line 8:
Line 9: public class GetDiskLunAlignmentVDSCommandParameters extends
GetDiskAlignmentVDSCommandParameters {
Line 10: private String lunGuid;
perhaps change to lunId
Line 11:
Line 12: public GetDiskLunAlignmentVDSCommandParameters(Guid vdsId, Guid
vmId) {
Line 13: super(vdsId, vmId);
Line 14: }
....................................................
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
Line 529: ACTION_TYPE_FAILED_DISK_IS_USED_FOR_CREATE_VM=Cannot ${action}
${type}. This disk is currently in use to create VM ${VmName}.
Line 530: ACTION_TYPE_FAILED_DISK_IS_BEING_REMOVED=Cannot ${action} ${type}.
Disk ${DiskName} is being removed.
Line 531: ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_EXPORTED=Cannot ${action}
${type}. Template ${TemplateName} is being exported.
Line 532: ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_REMOVED=Cannot ${action}
${type}. Template ${TemplateName} is being removed.
Line 533: ACTION_TYPE_FAILED_DISK_IS_USED_BY_GET_ALIGNMENT=Cannot ${action}
${type}. Disk ${DiskName} is in use to determine the alignment.
1. I'd change the phrasing, perhaps :
Disk ${DiskName} alignment is currently being determined.
2. perhaps change to DiskAlias
Line 534: NETWORK_BOND_HAVE_ATTACHED_VLANS=Bond attached to vlan, remove bonds
vlan first
Line 535: NETWORK_INTERFACE_CONNECT_TO_VLAN=Cannot attach non vlan network to
vlan interface
Line 536: NETWORK_CANNOT_REMOVE_NETWORK_IN_USE_BY_VM=Cannot remove network
'${NetworkName}', it's in use by a VM
Line 537: ACTION_TYPE_FAILED_DISK_MAX_SIZE_EXCEEDED=Cannot create disk more
than ${max}_disk_size GB
--
To view, visit http://gerrit.ovirt.org/11946
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4858b7bbfa453230fcafecfbc5358c715d5d825b
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
