Arik Hadas has posted comments on this change.

Change subject: core: don't allow snapshot for suspended vm
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.ovirt.org/#/c/34731/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java:

Line 543: 
Line 544:     protected boolean validateVM(VmValidator vmValidator) {
Line 545:         return canDoSnapshot(getVm()) &&
Line 546:                 validate(vmValidator.vmNotSavingRestoring()) &&
Line 547:                 validate(vmValidator.vmNotSuspended());
isn't it better to use the matrix in VdcActionUtils so the operation will be 
blocked in the UI as well? then we can validate the status like we do in 
RunVmValidator#validateVmStatusUsingMatrix
Line 548:     }
Line 549: 
Line 550:     private boolean isSpecifiedDisksExist(List<DiskImage> disks) {
Line 551:         if (disks == null || disks.isEmpty()) {


http://gerrit.ovirt.org/#/c/34731/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmValidator.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmValidator.java:

Line 106: 
Line 107:     public ValidationResult vmNotSuspended() {
Line 108:         for (VM vm : vms) {
Line 109:             if (vm.getStatus() == VMStatus.Suspended) {
Line 110:                 return new 
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_SUSPENDED);
how about using ACTION_TYPE_FAILED_VM_STATUS_ILLEGAL ?
Line 111:             }
Line 112:         }
Line 113: 
Line 114:         return ValidationResult.VALID;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf2c4c3ec3561df1d35f5e4da964664d63598434
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to