Maor Lipchuk has posted comments on this change.

Change subject: core: add clear message for live snapshot fail
......................................................................


Patch Set 5: (5 inline comments)

If you plan to keep the message perhaps consider to make it more general, so it 
can be used in other commands in the future (Maybe move the notion of LIVE 
SNPASHOT in the enum and in the message)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
Line 39: import java.util.ArrayList;
Line 40: import java.util.Arrays;
Line 41: import java.util.Collections;
Line 42: import java.util.List;
Line 43: import java.util.Map;
I don't think there is a reason to change the imports location here.
I assume that the imports were changed due to intelij default organize imports, 
but it does not related to that patch.
Line 44: 
Line 45: @DisableInPrepareMode
Line 46: @LockIdNameAttribute
Line 47: public class CreateAllSnapshotsFromVmCommand<T extends 
CreateAllSnapshotsFromVmParameters> extends VmCommand<T> implements 
QuotaStorageDependent{


Line 159:         }
Line 160: 
Line 161:         endActionOnDisks();
Line 162: 
Line 163:         updateVmInSpm(getVm().getStoragePoolId(), Arrays.asList(new 
VM[]{getVm()}));
No need to change that line, it was already formatted
Line 164: 
Line 165:         setSucceeded(getParameters().getTaskGroupSuccess());
Line 166:         getReturnValue().setEndActionTryAgain(false);
Line 167:     }


Line 270: 
Line 271:     private boolean canDoSnapshot(VM vm) {
Line 272:         // if live snapshot is not available, then if vm is up - 
snapshot is not possible so it needs to be
Line 273:         // checked if vm up or not
Line 274:         // if live snapshot is enabled, there is no need to check if 
vm is up since in any case snapshot is possible
Consider put relevant part of this comment as a java doc for this method
Line 275:         if (!isLiveSnapshotEnabled() && !ImagesHandler.isVmDown(vm)) {
Line 276:             // if there is no live snapshot and the vm is up - 
snapshot is not possible
Line 277:             return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DATA_CENTER_VERSION_DOESNT_SUPPORT_LIVE_SNAPSHOT);
Line 278:         }


Line 279:         return true;
Line 280:     }
Line 281: 
Line 282:     /**
Line 283:      * @return Check for VM down only if DC level does not support 
live snapshots.
Java doc here is not related to the method (...Check for VM down...)
Line 284:      */
Line 285:     private boolean isLiveSnapshotEnabled() {
Line 286:         return Config.<Boolean> GetValue(
Line 287:                 ConfigValues.LiveSnapshotEnabled, 
getStoragePool().getcompatibility_version().getValue());


....................................................
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
Line 479: ACTION_TYPE_FAILED_ILLEGAL_NUM_OF_MONITORS=Cannot ${action} ${type}. 
Illegal number of monitors is provided, max allowed number of monitors is 1 for 
VNC and the max number in the ValidNumOfMonitors configuration variable for 
Spice.
Line 480: ACTION_TYPE_FAILED_ILLEGAL_DOMAIN_NAME=Cannot ${action} ${type}. 
Illegal Domain name: ${Domain}. Domain name has unsupported special character 
${Char}.
Line 481: ACTION_TYPE_FAILED_CANNOT_DECREASE_COMPATIBILITY_VERSION=Cannot 
${action} ${type}. Compatibility version is invalid, previous value restored.
Line 482: ACTION_TYPE_FAILED_GIVEN_VERSION_NOT_SUPPORTED=Cannot ${action} 
${type}. Selected Compatibility Version is not supported.
Line 483: 
ACTION_TYPE_FAILED_DATA_CENTER_VERSION_DOESNT_SUPPORT_LIVE_SNAPSHOT=Cannot 
${action} ${type}. Selected data center compatibility version does not support 
live snapshot.
/s/data center/Data Center

Data Center reflects an entity in the setup
Line 484: NETWORK_ADDR_MANDATORY_IN_STATIC_IP=Netwrok address must be specify 
when using static ip
Line 485: ACTION_TYPE_FAILED_OBJECT_LOCKED=Cannot ${action} ${type}. Related 
operation is currently in progress. Please try again later.
Line 486: NETWORK_BOND_HAVE_ATTACHED_VLANS=Bond attached to vlan, remove bonds 
vlan first
Line 487: NETWORK_INTERFACE_CONNECT_TO_VLAN=Cannot attach non vlan network to 
vlan interface


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7b6dfc138951baddc991a82a4c9e64f60499428a
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Sharad Mishra <snmis...@linux.vnet.ibm.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to