Arik Hadas has uploaded a new change for review. Change subject: core: remove unneeded code which might produce NPE ......................................................................
core: remove unneeded code which might produce NPE VdsEventListener#processOnVmPoweringUp contained block of code in a if clause that could never be executed as all the commands that implement the IVdsAsyncCommand interface returned null in their getAutoStartVdsId method, thus the condition in the if statement was always false. Because of a race which is similar to the one which was fixed by Icaca2a3, the check inside the if-condition could produce NPE. So in this patch this block of code is removed. The signature of VdsEventListener#processOnVmPoweringUp and IVdsAsyncCommand interface were cleaned up by removing things which are not needed anymore. Change-Id: Iffef34726c7feb87be1436c003eded2e226d7e5d Bug-Url: https://bugzilla.redhat.com/1134701 Signed-off-by: Arik Hadas <aha...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/IVdsAsyncCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/IVdsEventListener.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java 5 files changed, 3 insertions(+), 46 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/07/32107/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java index dcc195d..67b584d 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java @@ -277,16 +277,6 @@ return vds != null ? vds.getId() : null; } - @Override - public boolean getAutoStart() { - return getVm().isAutoStartup(); - } - - @Override - public Guid getAutoStartVdsId() { - return null; - } - protected boolean connectLunDisks(Guid hostId) { if (getVm().getDiskMap().isEmpty()) { VmHandler.updateDisksFromDb(getVm()); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java index fa4d32e..9707ca4 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java @@ -59,8 +59,6 @@ import org.ovirt.engine.core.common.locks.LockingGroup; import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.vdscommands.DisconnectStoragePoolVDSCommandParameters; -import org.ovirt.engine.core.common.vdscommands.SetVmTicketVDSCommandParameters; -import org.ovirt.engine.core.common.vdscommands.StartSpiceVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.UpdateVmPolicyVDSParams; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.compat.Guid; @@ -68,7 +66,6 @@ import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase; -import org.ovirt.engine.core.utils.Ticketing; import org.ovirt.engine.core.utils.ejb.BeanProxyType; import org.ovirt.engine.core.utils.ejb.BeanType; import org.ovirt.engine.core.utils.ejb.EjbUtils; @@ -359,7 +356,7 @@ } @Override - public void processOnVmPoweringUp(Guid vds_id, Guid vmid, String display_ip, int display_port) { + public void processOnVmPoweringUp(Guid vmid) { IVdsAsyncCommand command = Backend.getInstance().getResourceManager().GetAsyncCommandForVm(vmid); /* @@ -370,30 +367,6 @@ */ if (command != null) { command.onPowerringUp(); - if (command.getAutoStart() && command.getAutoStartVdsId() != null) { - try { - String otp64 = Ticketing.generateOTP(); - Backend.getInstance() - .getResourceManager() - .RunVdsCommand(VDSCommandType.SetVmTicket, - new SetVmTicketVDSCommandParameters(vds_id, vmid, otp64, 60, "", Guid.Empty)); - log.infoFormat( - "VdsEventListener.ProcessOnVmPoweringUp - Auto start logic, starting spice to vm - {0} ", - vmid); - Backend.getInstance() - .getResourceManager() - .RunVdsCommand( - VDSCommandType.StartSpice, - new StartSpiceVDSCommandParameters(command.getAutoStartVdsId(), display_ip, - display_port, otp64)); - } catch (RuntimeException ex) { - log.errorFormat( - "VdsEventListener.ProcessOnVmPoweringUp - failed to start spice on VM - {0} - {1} - {2}", - vmid, - ex.getMessage(), - ex.getStackTrace()); - } - } } } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/IVdsAsyncCommand.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/IVdsAsyncCommand.java index f21e4d5..9d169f9 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/IVdsAsyncCommand.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/IVdsAsyncCommand.java @@ -1,15 +1,10 @@ package org.ovirt.engine.core.common.businessentities; -import org.ovirt.engine.core.compat.Guid; public interface IVdsAsyncCommand { void rerun(); void runningSucceded(); - - boolean getAutoStart(); - - Guid getAutoStartVdsId(); /** * Assures the Job/Step are completed. diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/IVdsEventListener.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/IVdsEventListener.java index 1e97598..1ab4471 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/IVdsEventListener.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/IVdsEventListener.java @@ -34,7 +34,7 @@ void processOnCpuFlagsChange(Guid vdsId); - void processOnVmPoweringUp(Guid vds_id, Guid vmid, String display_ip, int display_port); + void processOnVmPoweringUp(Guid vmid); void handleVdsVersion(Guid vdsId); diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java index 680830d..e47103e 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java @@ -580,8 +580,7 @@ // process all vms that powering up. for (VmDynamic runningVm : _poweringUpVms) { - getVdsEventListener().processOnVmPoweringUp(_vds.getId(), runningVm.getId(), - runningVm.getDisplayIp(), runningVm.getDisplay()); + getVdsEventListener().processOnVmPoweringUp(runningVm.getId()); } // process all vms that went down -- To view, visit http://gerrit.ovirt.org/32107 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iffef34726c7feb87be1436c003eded2e226d7e5d Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <aha...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
