Martin Betak has uploaded a new change for review. Change subject: backend: Set Vm.current_cd on RunOnce ......................................................................
backend: Set Vm.current_cd on RunOnce Modified tracking of current CDs. Before the persistent configuration was stored in VmBase#isoPath and if we got another "runtime" value from VDSM we would store that in VmDynamic#currentCd. Otherwise currentCd would be null and when getting the "current" status one would first have to check if currentCd is null and if not then fall back to isoPath. This was not only difficult to use from other parts of code, but it also didn't work for RunOnce where the VDSM would not report the 'cdrom' in getAllVmStats. Now we set the currentCd directly in RunVmCommand and can always access directly the "current" value of cd rom. Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1065719 Change-Id: I5bfc4fbfae15e902898859dddf3cdffe9301dab2 Signed-off-by: Martin Betak <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeDiskCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendCdRomResource.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java 8 files changed, 24 insertions(+), 20 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/85/25585/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeDiskCommand.java index d1327e7..51bb909 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeDiskCommand.java @@ -6,7 +6,6 @@ import org.ovirt.engine.core.bll.validator.LocalizedVmStatus; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.ChangeDiskCommandParameters; -import org.ovirt.engine.core.common.businessentities.VmDynamic; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.vdscommands.ChangeDiskVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; @@ -72,15 +71,9 @@ .RunVdsCommand(VDSCommandType.ChangeDisk, new ChangeDiskVDSCommandParameters(getVdsId(), getVm().getId(), cdImagePath)) .getReturnValue()); - updateCurrentCd(); + VmHandler.updateCurrentCd(getVm(), getParameters().getCdImagePath()); setSucceeded(true); - } - - private void updateCurrentCd() { - VmDynamic vmDynamic = getVm().getDynamicData(); - vmDynamic.setCurrentCd(getParameters().getCdImagePath()); - getVmDynamicDao().update(vmDynamic); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java index 7ff4c2b..5312376 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java @@ -470,6 +470,7 @@ if (StringUtils.isNotEmpty(cdPath)) { log.infoFormat("Running VM with attached cd {0}", cdPath); } + updateCurrentCd(cdPath); getVm().setCdPath(cdPathWindowsToLinux(cdPath)); if (!StringUtils.isEmpty(getParameters().getFloppyPath())) { @@ -496,6 +497,10 @@ return vmStatus; } + protected void updateCurrentCd(String cdPath) { + VmHandler.updateCurrentCd(getVm(), cdPath); + } + /** * Initialize the parameters for the VDSM command of VM creation diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java index 37e2046..182a928 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java @@ -58,6 +58,7 @@ 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.dao.VmDynamicDAO; import org.ovirt.engine.core.dao.VmInitDAO; import org.ovirt.engine.core.utils.ObjectIdentityChecker; import org.ovirt.engine.core.utils.linq.LinqUtils; @@ -734,4 +735,11 @@ return validationResult; } + public static void updateCurrentCd(VM vm, String currentCd) { + VmDynamicDAO vmDynamicDao = DbFacade.getInstance().getVmDynamicDao(); + VmDynamic vmDynamic = vm.getDynamicData(); + vmDynamic.setCurrentCd(currentCd); + vmDynamicDao.update(vmDynamic); + } + } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java index a765299..fa2b038 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java @@ -156,9 +156,7 @@ List<String> vmNames = new LinkedList<>(); for (VmStatic vmStatic : vms) { VmDynamic vmDynamic = getVmDynamicDAO().get(vmStatic.getId()); - if (vmDynamic.getStatus() != VMStatus.Down - && !StringUtils.isEmpty(vmDynamic.getCurrentCd() != null ? vmDynamic.getCurrentCd() - : vmStatic.getIsoPath())) { + if (vmDynamic.getStatus() != VMStatus.Down && !StringUtils.isEmpty(vmDynamic.getCurrentCd())) { vmNames.add(vmStatic.getName()); } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java index 46c99c6..b9340b7 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java @@ -297,6 +297,8 @@ doReturn(vmDAO).when(command).getVmDAO(); when(vmDAO.get(command.getParameters().getVmId())).thenReturn(vm); doReturn(new VDSGroup()).when(command).getVdsGroup(); + // Avoid referencing the unmockable static VmHandler.updateCurrentCd + doNothing().when(command).updateCurrentCd(any(String.class)); return vm; } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java index 9242d97..e832976 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java @@ -1248,7 +1248,10 @@ setVmHost(vm.getVmHost()); setVmIp(vm.getVmIp()); setVmFQDN(vm.getVmFQDN()); - setCurrentCd(vm.getCurrentCd()); + // update only if vdsm actually provides some value, otherwise engine has more information + if (vm.getCurrentCd() != null) { + setCurrentCd(vm.getCurrentCd()); + } // if (!string.IsNullOrEmpty(vm.app_list)) // { diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendCdRomResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendCdRomResource.java index ec50c68..3d17a40 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendCdRomResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendCdRomResource.java @@ -50,12 +50,9 @@ if (vm == null) { return notFound(); } - // if the CD has changed during the run of VM - if (vm.getCurrentCd() != null) { - // change the iso path so the result of 'map' will contain current cd instead of the - // persistent configuration - vm.setIsoPath(vm.getCurrentCd()); - } + // change the iso path so the result of 'map' will contain current cd instead of the + // persistent configuration + vm.setIsoPath(vm.getCurrentCd()); return addLinks(populate(map(vm), vm)); } else { return super.get(); diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java index 7e11fad..6444718 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java @@ -292,8 +292,6 @@ if (xmlRpcStruct.containsKey(VdsProperties.CDRom)) { String isoName = Paths.get((String) xmlRpcStruct.get(VdsProperties.CDRom)).getFileName().toString(); vm.setCurrentCd(isoName); - } else { - vm.setCurrentCd(null); } } -- To view, visit http://gerrit.ovirt.org/25585 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5bfc4fbfae15e902898859dddf3cdffe9301dab2 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Betak <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
