Liron Aravot has posted comments on this change.
Change subject: core: add clear message for live snapshot fail
......................................................................
Patch Set 1: (4 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
Line 241: protected boolean canDoAction() {
Line 242: boolean result = true;
Line 243: List<DiskImage> disksList = getDisksList();
Line 244: if (disksList.size() > 0) {
Line 245: VmValidator vmValidator = new VmValidator(getVm());
the check is performed in wrong place now -
the current code performs the VmIsDown check at the beginning of
ImagesHandler.performImageChecks method and then doesn't continue with any
checks.
what will happen after this change is that we will make unneeded vdsm calls in
this method - (call to checkDiskImages()) which is totally unnecessary if we
can know from the begining that it's irrelevant.
Line 246: result = validate(new
SnapshotsValidator().vmNotDuringSnapshot(getVmId()))
Line 247: && validate(vmValidator.vmNotDuringMigration())
Line 248: && validate(vmValidator.vmNotRunningStateless())
Line 249: && ImagesHandler.PerformImagesChecks(getVm(),
Line 269: addCanDoActionMessage(VdcBllMessages.VAR__ACTION__CREATE);
Line 270: addCanDoActionMessage(VdcBllMessages.VAR__TYPE__SNAPSHOT);
Line 271: }
Line 272:
Line 273: private boolean canDoSnapshot(VM vm) {
please remove the version specific comments..
Line 274: //if version is 3.0 - live snapshot is not available, thus if
vm is up snapshot is not possible so it needs to be
Line 275: //checked if it's up or not
Line 276: //if version is 3.1, there is no need to check if vm is up
since in any case snapshot is possible
Line 277: boolean canSnapshot = true;
Line 273: private boolean canDoSnapshot(VM vm) {
Line 274: //if version is 3.0 - live snapshot is not available, thus if
vm is up snapshot is not possible so it needs to be
Line 275: //checked if it's up or not
Line 276: //if version is 3.1, there is no need to check if vm is up
since in any case snapshot is possible
Line 277: boolean canSnapshot = true;
I agree with Vered
Line 278: if(!isLiveSnapshotEnabled() && !ImagesHandler.isVmDown(vm)) {
Line 279: //if there is no live snapshot and the vm is up -
snapshot is not possible
Line 280:
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DATA_CENTER_VERSION_DOESNT_SUPPORT_LIVE_SNAPSHOT);
Line 281: canSnapshot = false;
Line 275: //checked if it's up or not
Line 276: //if version is 3.1, there is no need to check if vm is up
since in any case snapshot is possible
Line 277: boolean canSnapshot = true;
Line 278: if(!isLiveSnapshotEnabled() && !ImagesHandler.isVmDown(vm)) {
Line 279: //if there is no live snapshot and the vm is up -
snapshot is not possible
please use failCanDoAction instead
Line 280:
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DATA_CENTER_VERSION_DOESNT_SUPPORT_LIVE_SNAPSHOT);
Line 281: canSnapshot = false;
Line 282: }
Line 283: return canSnapshot;
--
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: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
