Martin Betak 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 36: Line 37: cdImagePath = getParameters().getCdImagePath(); Line 38: Line 39: if (!getVm().isRunningOrPaused()) { Line 40: setSucceeded(false); > no need for the setSucceeded(false) - it can be removed from all the places Done Line 41: addCanDoActionMessage(VdcBllMessages.VAR__TYPE__VM); Line 42: Line 43: // An empty 'cdImagePath' means eject CD Line 44: if (!StringUtils.isEmpty(cdImagePath)) { 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-manag Done 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 Done 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
