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