Allon Mureinik has uploaded a new change for review. Change subject: core: Fix storage validations for extending a disk ......................................................................
core: Fix storage validations for extending a disk Use StorageDomainValidator.hasSpaceForNewDisk(DiskImage) method, which is sensitive to the type and format of the disk. Although a new disk is NOT added, the extension is essentially empty, so sans some overhead, it can be considered as a new disk. This patch includes fixing the command and adding some basic unit tests to the flow. Change-Id: I76d38c41321495b9e12087dc4698f67830d7a1d4 Bug-Url: https://bugzilla.redhat.com/1053740 Signed-off-by: Allon Mureinik <amure...@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, 51 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/62/24062/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 1d94e42..5443e46 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 @@ -263,7 +263,7 @@ return true; } - private boolean validateCanResizeDisk() { + protected boolean validateCanResizeDisk() { DiskImage newDiskImage = (DiskImage) getNewDisk(); if (vmDeviceForVm.getSnapshotId() != null) { @@ -286,13 +286,21 @@ } } - long additionalDiskSpaceInGB = newDiskImage.getSizeInGigabytes() - oldDiskImage.getSizeInGigabytes(); StorageDomain storageDomain = getStorageDomainDAO().getForStoragePool( newDiskImage.getStorageIds().get(0), newDiskImage.getStoragePoolId()); StorageDomainValidator storageDomainValidator = new StorageDomainValidator(storageDomain); - return validate(storageDomainValidator.isDomainExistAndActive()) && - validate(storageDomainValidator.isDomainHasSpaceForRequest(additionalDiskSpaceInGB)); + if (!validate(storageDomainValidator.isDomainExistAndActive())) { + return false; + } + + // For size allocation validation, we'll create a dummy with the additional size required. + // That way, the validator can hold all the logic about storage types. + long additionalDiskSpaceInGB = newDiskImage.getSizeInGigabytes() - oldDiskImage.getSizeInGigabytes(); + DiskImage dummyForValidation = DiskImage.copyOf(newDiskImage); + dummyForValidation.setSizeInGigabytes(additionalDiskSpaceInGB); + + return validate(storageDomainValidator.hasSpaceForNewDisk(dummyForValidation)); } return true; 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 d092b08..76319fa 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 @@ -39,6 +39,7 @@ import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.DiskInterface; import org.ovirt.engine.core.common.businessentities.StorageDomain; +import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.StoragePool; import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.VDS; @@ -62,6 +63,7 @@ import org.ovirt.engine.core.dao.DiskImageDAO; import org.ovirt.engine.core.dao.ImageDao; import org.ovirt.engine.core.dao.SnapshotDao; +import org.ovirt.engine.core.dao.StorageDomainDAO; import org.ovirt.engine.core.dao.StorageDomainStaticDAO; import org.ovirt.engine.core.dao.StoragePoolDAO; import org.ovirt.engine.core.dao.VdsDAO; @@ -78,6 +80,8 @@ private Guid diskImageGuid = Guid.newGuid(); private Guid vmId = Guid.newGuid(); + private Guid sdId = Guid.newGuid(); + private Guid spId = Guid.newGuid(); @Mock private VmDAO vmDAO; @@ -101,6 +105,8 @@ private StoragePoolDAO storagePoolDao; @Mock private StorageDomainStaticDAO storageDomainStaticDao; + @Mock + private StorageDomainDAO storageDomainDao; @Mock private DbFacade dbFacade; @Mock @@ -387,6 +393,36 @@ verify(vmDeviceDAO, never()).update(any(VmDevice.class)); } + @Test + public void testResize() { + DiskImage oldDisk = createDiskImage(); + when(diskDao.get(diskImageGuid)).thenReturn(oldDisk); + + StorageDomain sd = new StorageDomain(); + sd.setAvailableDiskSize(Integer.MAX_VALUE); + sd.setStatus(StorageDomainStatus.Active); + when(storageDomainDao.getForStoragePool(sdId, spId)).thenReturn(sd); + + UpdateVmDiskParameters parameters = createParameters(); + ((DiskImage) parameters.getDiskInfo()).setSize(oldDisk.getSize() * 2); + initializeCommand(parameters); + + assertTrue(command.validateCanResizeDisk()); + } + + @Test + public void testFaultyResize() { + when(diskDao.get(diskImageGuid)).thenReturn(createDiskImage()); + + UpdateVmDiskParameters parameters = createParameters(); + ((DiskImage) parameters.getDiskInfo()).setSize(parameters.getDiskInfo().getSize() / 2); + initializeCommand(parameters); + + assertFalse(command.validateCanResizeDisk()); + CanDoActionTestUtils.assertCanDoActionMessages + ("wrong failure", command, VdcBllMessages.ACTION_TYPE_FAILED_REQUESTED_DISK_SIZE_IS_TOO_SMALL); + } + private void initializeCommand(UpdateVmDiskParameters params) { initializeCommand(params, Collections.singletonList(createVmStatusDown())); } @@ -413,6 +449,7 @@ doReturn(diskImageDao).when(command).getDiskImageDao(); doReturn(storagePoolDao).when(command).getStoragePoolDAO(); doReturn(storageDomainStaticDao).when(command).getStorageDomainStaticDAO(); + doReturn(storageDomainDao).when(command).getStorageDomainDAO(); doReturn(vmStaticDAO).when(command).getVmStaticDAO(); doReturn(baseDiskDao).when(command).getBaseDiskDao(); doReturn(imageDao).when(command).getImageDao(); @@ -559,6 +596,8 @@ disk.setId(diskImageGuid); disk.setSize(100000L); disk.setDiskInterface(DiskInterface.VirtIO); + disk.setStorageIds(new ArrayList<>(Collections.singleton(sdId))); + disk.setStoragePoolId(spId); return disk; } -- To view, visit http://gerrit.ovirt.org/24062 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I76d38c41321495b9e12087dc4698f67830d7a1d4 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Allon Mureinik <amure...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
