Sergey Gotliv has uploaded a new change for review. Change subject: engine: Clearing address of vm disk device if it's interface was changed. ......................................................................
engine: Clearing address of vm disk device if it's interface was changed. Updating disk interface its like detach and attach disk again, therefore address of the vm device representing this disk must be cleared. Change-Id: I62605c490da909447f77513e7691d76ddd24ff26 Bug-Url: https://bugzilla.redhat.com/994247 Signed-off-by: Sergey Gotliv <sgot...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java 2 files changed, 53 insertions(+), 10 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/29/17729/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java index 2b8b908..a433fb2 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java @@ -272,7 +272,7 @@ @Override public Object runInTransaction() { getVmStaticDAO().incrementDbGeneration(getVm().getId()); - clearAddressOnInterfaceChange(disk, getNewDisk()); + clearAddressOnInterfaceChange(); getBaseDiskDao().update(disk); if (disk.getDiskStorageType() == DiskStorageType.IMAGE) { DiskImage diskImage = (DiskImage) disk; @@ -288,11 +288,11 @@ return null; } - private void clearAddressOnInterfaceChange(Disk diskToUpdate, Disk diskWithUserChanges) { + private void clearAddressOnInterfaceChange() { // clear the disk address if the type has changed - if (diskToUpdate.getDiskInterface() != diskWithUserChanges.getDiskInterface()) { - getVmDeviceDao().clearDeviceAddress(getVmDeviceDao().get(new VmDeviceId(diskToUpdate.getId(), - getVmId())).getDeviceId()); + if (getOldDisk().getDiskInterface() != getNewDisk().getDiskInterface()) { + VmDeviceId deviceId = new VmDeviceId(getOldDisk().getId(), getVmId()); + getVmDeviceDao().clearDeviceAddress(getVmDeviceDao().get(deviceId).getDeviceId()); } } }); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java index ce36b46..378ae5e 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java @@ -8,6 +8,8 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.ovirt.engine.core.utils.MockConfigRule.mockConfig; @@ -24,15 +26,20 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.invocation.InvocationOnMock; import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.common.action.UpdateVmDiskParameters; import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.DiskInterface; import org.ovirt.engine.core.common.businessentities.StoragePool; import org.ovirt.engine.core.common.businessentities.VDS; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; +import org.ovirt.engine.core.common.businessentities.VmDevice; +import org.ovirt.engine.core.common.businessentities.VmDeviceId; import org.ovirt.engine.core.common.businessentities.VolumeFormat; import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.errors.VdcBllMessages; @@ -48,6 +55,7 @@ import org.ovirt.engine.core.dao.StoragePoolDAO; import org.ovirt.engine.core.dao.VdsDAO; import org.ovirt.engine.core.dao.VmDAO; +import org.ovirt.engine.core.dao.VmDeviceDAO; import org.ovirt.engine.core.dao.VmStaticDAO; import org.ovirt.engine.core.dao.network.VmNetworkInterfaceDao; import org.ovirt.engine.core.utils.MockConfigRule; @@ -79,6 +87,8 @@ private SnapshotDao snapshotDao; @Mock private DiskImageDAO diskImageDao; + @Mock + private VmDeviceDAO vmDeviceDAO; @Mock private StoragePoolDAO storagePoolDao; @Mock @@ -237,6 +247,40 @@ assertEquals(!boot, command.canDoAction()); } + @Test + public void clearAddressOnInterfaceChange() { + final UpdateVmDiskParameters parameters = createParameters(); + // update new disk interface so it will be different than the old one + parameters.getDiskInfo().setDiskInterface(DiskInterface.VirtIO_SCSI); + + // creating old disk with interface different than interface of disk from parameters + // have to return original disk on each request to dao, + // since the command updates retrieved instance of disk + when(diskDao.get(diskImageGuid)).thenAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocationOnMock) throws Throwable { + final DiskImage oldDisk = createDiskImage(); + oldDisk.setDiskInterface(DiskInterface.VirtIO); + assert(oldDisk.getDiskInterface() != parameters.getDiskInfo().getDiskInterface()); + return oldDisk; + } + }); + + initializeCommand(parameters); + mockVmStatusDown(); + + VmDeviceId vmDeviceId = new VmDeviceId(diskImageGuid, vmId); + VmDevice device = new VmDevice(); + device.setId(vmDeviceId); + + doReturn(device).when(vmDeviceDAO).get(vmDeviceId); + + command.executeVmCommand(); + + // verify that device addressed was cleared exactly once + verify(vmDeviceDAO, times(1)).clearDeviceAddress(device.getDeviceId()); + } + private void initializeCommand() { initializeCommand(createParameters()); } @@ -259,7 +303,9 @@ doReturn(vmStaticDAO).when(command).getVmStaticDAO(); doReturn(baseDiskDao).when(command).getBaseDiskDao(); doReturn(imageDao).when(command).getImageDao(); + doReturn(vmDeviceDAO).when(command).getVmDeviceDao(); doNothing().when(command).updateVmDisksAndDevice(); + doNothing().when(vmStaticDAO).incrementDbGeneration(any(Guid.class)); ejbRule.mockResource(ContainerManagedResourceType.TRANSACTION_MANAGER, new DummyTransactionManager()); DbFacadeLocator.setDbFacade(dbFacade); @@ -280,9 +326,6 @@ when(vmDAO.get(command.getParameters().getVmId())).thenReturn(null); } - /** - * Mock a VM in status Up - */ protected VM mockVmStatusDown(VM... otherPluggedVMs) { VM vm = new VM(); vm.setStatus(VMStatus.Down); @@ -346,8 +389,7 @@ * @return Valid parameters for the command. */ protected UpdateVmDiskParameters createParameters() { - DiskImage diskInfo = new DiskImage(); - diskInfo.setId(diskImageGuid); + DiskImage diskInfo = createDiskImage(); return new UpdateVmDiskParameters(vmId, diskImageGuid, diskInfo); } @@ -364,6 +406,7 @@ private DiskImage createDiskImage() { DiskImage disk = new DiskImage(); disk.setId(diskImageGuid); + disk.setSize(100000L); return disk; } -- To view, visit http://gerrit.ovirt.org/17729 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I62605c490da909447f77513e7691d76ddd24ff26 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv <sgot...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
