Daniel Erez has uploaded a new change for review. Change subject: core: clone VM from snapshot - validate VirtIO-SCSI ......................................................................
core: clone VM from snapshot - validate VirtIO-SCSI Validate whether VirtIO-SCSI can be disabled when cloning a VM from snapshot (similar to the validation on UpdateVmCommand): * Added a validation method to VmValidator. * Added appropriate tests. Change-Id: I31d4ebe3a964fb54cb1a905460b9da785a746419 Bug-Url: https://bugzilla.redhat.com/1133475 Signed-off-by: Daniel Erez <de...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmValidator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/VmValidatorTest.java 6 files changed, 141 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/45/31945/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java index 577264f..937d8c6 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java @@ -3,6 +3,7 @@ import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.bll.utils.PermissionSubject; +import org.ovirt.engine.core.bll.validator.VmValidator; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.AddVmFromSnapshotParameters; import org.ovirt.engine.core.common.action.LockProperties; @@ -128,6 +129,12 @@ return false; } + VmValidator vmValidator = createVmValidator(vmFromConfiguration); + if (Boolean.FALSE.equals(getParameters().isVirtioScsiEnabled()) && + !validate(vmValidator.canDisableVirtioScsi(getDiskImagesFromConfiguration()))) { + return false; + } + if (!super.canDoAction()) { return false; } @@ -243,4 +250,8 @@ protected void updateOriginalTemplate(VmStatic vmStatic) { // do not update it - it is already correctly configured from the snapshot } + + public VmValidator createVmValidator(VM vm) { + return new VmValidator(vm); + } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java index ede5e36..1aadd5b 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java @@ -19,6 +19,7 @@ import org.ovirt.engine.core.bll.snapshots.SnapshotsManager; import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.bll.utils.VmDeviceUtils; +import org.ovirt.engine.core.bll.validator.VmValidator; import org.ovirt.engine.core.bll.validator.VmWatchdogValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.FeatureSupported; @@ -34,7 +35,6 @@ import org.ovirt.engine.core.common.action.WatchdogParameters; import org.ovirt.engine.core.common.businessentities.ActionGroup; import org.ovirt.engine.core.common.businessentities.Disk; -import org.ovirt.engine.core.common.businessentities.DiskInterface; import org.ovirt.engine.core.common.businessentities.MigrationSupport; import org.ovirt.engine.core.common.businessentities.Snapshot; import org.ovirt.engine.core.common.businessentities.VM; @@ -539,13 +539,9 @@ } } - if (Boolean.FALSE.equals(getParameters().isVirtioScsiEnabled())) { - List<Disk> allDisks = getDiskDao().getAllForVm(getVmId(), true); - for (Disk disk : allDisks) { - if (disk.getDiskInterface() == DiskInterface.VirtIO_SCSI) { - return failCanDoAction(VdcBllMessages.CANNOT_DISABLE_VIRTIO_SCSI_PLUGGED_DISKS); - } - } + VmValidator vmValidator = createVmValidator(vmFromParams); + if (Boolean.FALSE.equals(getParameters().isVirtioScsiEnabled()) && !validate(vmValidator.canDisableVirtioScsi(null))) { + return false; } if (vmFromParams.getMinAllocatedMem() > vmFromParams.getMemSizeMb()) { @@ -746,4 +742,7 @@ return getParameters().getWatchdog() != null; } + public VmValidator createVmValidator(VM vm) { + return new VmValidator(vm); + } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmValidator.java index b7ca766..8e78830 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmValidator.java @@ -1,20 +1,25 @@ package org.ovirt.engine.core.bll.validator; +import java.util.Collection; import java.util.Collections; import java.util.LinkedList; import java.util.List; +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.collections.Predicate; import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.ImagesHandler; import org.ovirt.engine.core.bll.ValidationResult; import org.ovirt.engine.core.common.businessentities.BaseDisk; 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.Snapshot.SnapshotType; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.dal.dbbroker.DbFacade; +import org.ovirt.engine.core.dao.DiskDao; /** A Validator for various VM canDoAction needs */ public class VmValidator { @@ -170,7 +175,34 @@ return ValidationResult.VALID; } - private DbFacade getDbFacade() { + /** + * Determines whether VirtIO-SCSI can be disabled for the VM + * (can be disabled when no disk uses VirtIO-SCSI interface). + */ + public ValidationResult canDisableVirtioScsi(Collection<? extends Disk> vmDisks) { + if (vmDisks == null) { + vmDisks = getDiskDao().getAllForVm(vms.iterator().next().getId(), true); + } + + boolean isVirtioScsiDiskExists = CollectionUtils.exists(vmDisks, new Predicate() { + @Override + public boolean evaluate(Object disk) { + return ((Disk) disk).getDiskInterface() == DiskInterface.VirtIO_SCSI; + } + }); + + if (isVirtioScsiDiskExists) { + return new ValidationResult(VdcBllMessages.CANNOT_DISABLE_VIRTIO_SCSI_PLUGGED_DISKS); + } + + return ValidationResult.VALID; + } + + public DiskDao getDiskDao() { + return getDbFacade().getDiskDao(); + } + + public DbFacade getDbFacade() { return DbFacade.getInstance(); } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommandTest.java index 67f7d9d..ba66f30 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommandTest.java @@ -8,6 +8,7 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -15,16 +16,22 @@ import org.mockito.stubbing.Answer; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.runners.MockitoJUnitRunner; +import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.bll.validator.StorageDomainValidator; +import org.ovirt.engine.core.bll.validator.VmValidator; import org.ovirt.engine.core.common.action.AddVmFromSnapshotParameters; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.DiskInterface; import org.ovirt.engine.core.common.businessentities.ImageStatus; +import org.ovirt.engine.core.common.businessentities.Snapshot; import org.ovirt.engine.core.common.businessentities.StorageDomain; +import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.compat.Guid; @@ -35,6 +42,8 @@ * The command under test. */ protected AddVmFromSnapshotCommand<AddVmFromSnapshotParameters> command; + + private SnapshotsValidator snapshotsValidator; @Test public void validateSpaceAndThreshold() { @@ -71,6 +80,32 @@ assertFalse(command.validateSpaceRequirements()); } + @Test + public void testCannotDisableVirtioScsi() { + initCommand(); + command.getParameters().setVirtioScsiEnabled(false); + + doReturn(snapshotsValidator).when(command).createSnapshotsValidator(); + doReturn(ValidationResult.VALID).when(snapshotsValidator).snapshotExists(any(Snapshot.class)); + doReturn(ValidationResult.VALID).when(snapshotsValidator).vmNotDuringSnapshot(any(Guid.class)); + + VM vm = new VM(); + Snapshot snapshot = new Snapshot(); + doReturn(vm).when(command).getVmFromConfiguration(); + doReturn(snapshot).when(command).getSnapshot(); + + DiskImage disk = new DiskImage(); + disk.setDiskInterface(DiskInterface.VirtIO_SCSI); + disk.setPlugged(true); + doReturn(Collections.singletonList(disk)).when(command).getDiskImagesFromConfiguration(); + + VmValidator vmValidator = spy(new VmValidator(vm)); + doReturn(vmValidator).when(command).createVmValidator(vm); + + CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, + VdcBllMessages.CANNOT_DISABLE_VIRTIO_SCSI_PLUGGED_DISKS); + } + @Override protected List<DiskImage> createDiskSnapshot(Guid diskId, int numOfImages) { List<DiskImage> disksList = new ArrayList<>(); @@ -104,5 +139,6 @@ generateStorageToDisksMap(command); initDestSDs(command); storageDomainValidator = mock(StorageDomainValidator.class); + snapshotsValidator = mock(SnapshotsValidator.class); } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java index 71f0292..9cb499f 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java @@ -23,6 +23,7 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import org.ovirt.engine.core.bll.validator.VmValidator; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VmManagementParametersBase; import org.ovirt.engine.core.common.businessentities.ArchitectureType; @@ -283,6 +284,10 @@ mockDiskDaoGetAllForVm(Collections.singletonList(disk), true); + VmValidator vmValidator = spy(new VmValidator(vm)); + doReturn(vmValidator).when(command).createVmValidator(vm); + doReturn(diskDAO).when(vmValidator).getDiskDao(); + CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, VdcBllMessages.CANNOT_DISABLE_VIRTIO_SCSI_PLUGGED_DISKS); } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/VmValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/VmValidatorTest.java new file mode 100644 index 0000000..6022861 --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/VmValidatorTest.java @@ -0,0 +1,49 @@ +package org.ovirt.engine.core.bll.validator; + +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.spy; +import static org.ovirt.engine.core.bll.validator.ValidationResultMatchers.failsWith; +import static org.ovirt.engine.core.bll.validator.ValidationResultMatchers.isValid; + +import java.util.Collections; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.runners.MockitoJUnitRunner; +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.VM; +import org.ovirt.engine.core.common.errors.VdcBllMessages; + +@RunWith(MockitoJUnitRunner.class) +public class VmValidatorTest { + + private VmValidator validator; + + private VM vm; + + @Before + public void setup() { + vm = new VM(); + validator = spy(new VmValidator(vm)); + } + + @Test + public void canDisableVirtioScsiSuccess() { + Disk disk = new DiskImage(); + disk.setDiskInterface(DiskInterface.VirtIO); + + assertThat(validator.canDisableVirtioScsi(Collections.singletonList(disk)), isValid()); + } + + @Test + public void canDisableVirtioScsiFail() { + Disk disk = new DiskImage(); + disk.setDiskInterface(DiskInterface.VirtIO_SCSI); + + assertThat(validator.canDisableVirtioScsi(Collections.singletonList(disk)), + failsWith(VdcBllMessages.CANNOT_DISABLE_VIRTIO_SCSI_PLUGGED_DISKS)); + } +} -- To view, visit http://gerrit.ovirt.org/31945 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I31d4ebe3a964fb54cb1a905460b9da785a746419 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <de...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
