Arik Hadas has posted comments on this change. Change subject: core: Fix NPE on ChangeCD with 'Down' VM ......................................................................
Patch Set 3: (3 comments) http://gerrit.ovirt.org/#/c/31614/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeDiskCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeDiskCommand.java: Line 40: setSucceeded no need for the setSucceeded(false) - it can be removed from all the places in this method (it is false by default) Line 50: } Line 51: Line 52: if (!canRunActionOnNonManagedVm()) { Line 53: return false; Line 54: } please move this check (52-54) to be before line 39 so in case of non-managed VM the user will get the proper error Line 55: Line 56: if ((IsoDomainListSyncronizer.getInstance().findActiveISODomain(getVm().getStoragePoolId()) == null) Line 57: && !StringUtils.isEmpty(cdImagePath)) { Line 58: setSucceeded(false); Line 65: addCanDoActionMessage(VdcBllMessages.VAR__ACTION__CHANGE_CD); Line 66: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_INVALID_CDROM_DISK_FORMAT); Line 67: } Line 68: Line 69: cdImagePath = ImagesHandler.cdPathWindowsToLinux(getParameters().getCdImagePath(), getVm().getStoragePoolId(), getVm().getRunOnVds()); so can we move it now to be in the execute method? it is not really related to the can-do-action section anymore.. Line 70: Line 71: return true; Line 72: } Line 73: -- To view, visit http://gerrit.ovirt.org/31614 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie3bf10dbeb8dd645a515dc140bd9081dc1d1acab Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Betak <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Martin Betak <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
