Ayal Baron has posted comments on this change. Change subject: core : WIP: correct message when suspend vm witout image disks(#841504) ......................................................................
Patch Set 1: I would prefer that you didn't submit this (1 inline comment) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java Line 265: if (retValue) { this function is becoming a monster and the added code is just making it more difficult to read. I don't see a way around clearing it up (early returns). This should be done in a separate patch and only then add the new code. Should be something like: @Override protected boolean canDoAction() { if (!performChecks()) { addCanDoActionMessage(VdcBllMessages.VAR__TYPE__VM); addCanDoActionMessage(VdcBllMessages.VAR__ACTION__HIBERNATE); return false; } return true; protected boolean performChecks() { if (getVm() == null) { addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_FOUND); return false; } if (getStorageDomainId().equals(Guid.Empty)) { addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_NOT_EXIST); return false; } if (getVm().getstatus() == VMStatus.WaitForLaunch || getVm().getstatus() == VMStatus.NotResponding) { addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_VM_STATUS_ILLEGAL); return false; } if (getVm().getstatus() != VMStatus.Up) { addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NOT_UP); return false; } if (AsyncTaskManager.getInstance().EntityHasTasks(getVmId())) { addCanDoActionMessage(VdcBllMessages.VM_CANNOT_SUSPENDE_HAS_RUNNING_TASKS); return false; } // check if vm has stateless images in db in case vm was run once as stateless // (then is_stateless is false) if (getVm().getis_stateless() || DbFacade.getInstance().getSnapshotDao().exists(getVmId(), SnapshotType.STATELESS)) { addCanDoActionMessage(VdcBllMessages.VM_CANNOT_SUSPEND_STATELESS_VM); return false; } if (DbFacade.getInstance().getVmPoolDAO().getVmPoolMapByVmGuid(getVmId()) != null) { addCanDoActionMessage(VdcBllMessages.VM_CANNOT_SUSPEND_VM_FROM_POOL); return false; } // Check storage before trying to create Images for hibernation. storage_domains domain = DbFacade.getInstance().getStorageDomainDAO().get(getStorageDomainId().getValue()); if (!StorageDomainSpaceChecker.hasSpaceForRequest(domain, (getImageSizeInBytes() + getMetaDataSizeInBytes())/BYTES_IN_GB)) { addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW); return false; } return True; } -- To view, visit http://gerrit.ovirt.org/6797 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I11fb32b54cf159fb63e172504342861493d2aa77 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <lara...@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: Maor Lipchuk <mlipc...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches