Gustavo Frederico Temple Pedrosa has uploaded a new change for review. Change subject: core: Disk hotplug validation - Patch 1 of 2 ......................................................................
core: Disk hotplug validation - Patch 1 of 2 * In the method isInterfaceSupportedForPlugUnPlug a hard-coded validation was replaced by information from the osinfo file. * Modifies the trimElements method of the OsRepositoryImpl class to filter out empty elements and correctly handle properties with empty lists as values. Change-Id: Ic2e4c792b607429daf077c11820b43f171d376b7 Signed-off-by: Gustavo Pedrosa <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OsRepositoryQuery.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/HotUnPlugDiskFromVmCommandTest.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/osinfo/OsRepository.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/osinfo/OsRepositoryImpl.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/OsQueryParameters.java M backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/osinfo/OsRepositoryImplTest.java M packaging/conf/osinfo-defaults.properties 9 files changed, 78 insertions(+), 8 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/01/19601/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java index 3c6a6dc..fcc5978 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java @@ -1,6 +1,8 @@ package org.ovirt.engine.core.bll; import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Map; @@ -171,11 +173,30 @@ } protected boolean isInterfaceSupportedForPlugUnPlug(Disk disk) { - if (!(DiskInterface.VirtIO.equals(disk.getDiskInterface()) || DiskInterface.VirtIO_SCSI.equals(disk.getDiskInterface()))) { - addCanDoActionMessage(VdcBllMessages.HOT_PLUG_DISK_IS_NOT_VIRTIO); - return false; + + boolean retVal = true; + + List<String> diskHotpluggableInterfaces = osRepository.getDiskHotpluggableInterfaces(getVm().getOs(), + getVm().getVdsGroupCompatibilityVersion()); + if (diskHotpluggableInterfaces == null || diskHotpluggableInterfaces.isEmpty()) { + retVal = false; } - return true; + + if (retVal) { + Collection<DiskInterface> diskInterfaces = new HashSet<DiskInterface>(); + for (String diskHotpluggableInterface : diskHotpluggableInterfaces) { + diskInterfaces.add(DiskInterface.valueOf(diskHotpluggableInterface)); + } + if (!diskInterfaces.contains(disk.getDiskInterface())) { + retVal = false; + } + } + + if (!retVal) { + addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_GUEST_OS_VERSION_IS_NOT_SUPPORTED); + } + + return retVal; } private boolean isDiskExistInVm(Disk disk) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OsRepositoryQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OsRepositoryQuery.java index 5715558..664d9b3 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OsRepositoryQuery.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OsRepositoryQuery.java @@ -48,6 +48,9 @@ case GetNetworkDevices: setReturnValue(osRepository.getNetworkDevices(getParameters().getOsId(), getParameters().getVersion())); break; + case GetDiskHotpluggableInterfaces: + setReturnValue(osRepository.getDiskHotpluggableInterfaces(getParameters().getOsId(), getParameters().getVersion())); + break; } } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommandTest.java index 147b6a0..6bb4304 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommandTest.java @@ -11,6 +11,7 @@ import static org.ovirt.engine.core.utils.MockConfigRule.mockConfig; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -56,6 +57,8 @@ protected Guid vmId = Guid.newGuid(); private final Guid storagePoolId = Guid.newGuid(); private final Guid storageDomainId = Guid.newGuid(); + protected static final ArrayList<String> DISK_HOTPLUGGABLE_INTERFACES = new ArrayList<String>( + Arrays.asList("VirtIO_SCSI", "VirtIO")); @ClassRule public static final MockConfigRule mcr = new MockConfigRule( @@ -140,7 +143,7 @@ assertFalse(command.canDoAction()); assertTrue(command.getReturnValue() .getCanDoActionMessages() - .contains(VdcBllMessages.HOT_PLUG_DISK_IS_NOT_VIRTIO.toString())); + .contains(VdcBllMessages.ACTION_TYPE_FAILED_GUEST_OS_VERSION_IS_NOT_SUPPORTED.toString())); } private void mockSpiceSupportMatrix() { @@ -274,6 +277,8 @@ disk.setDiskInterface(DiskInterface.IDE); doReturn(diskDao).when(command).getDiskDao(); when(diskDao.get(diskImageGuid)).thenReturn(disk); + when(osRepository.getDiskHotpluggableInterfaces(any(Integer.class), + any(Version.class))).thenReturn(DISK_HOTPLUGGABLE_INTERFACES); return disk; } @@ -287,6 +292,8 @@ disk.setActive(true); doReturn(diskDao).when(command).getDiskDao(); when(diskDao.get(diskImageGuid)).thenReturn(disk); + when(osRepository.getDiskHotpluggableInterfaces(any(Integer.class), + any(Version.class))).thenReturn(DISK_HOTPLUGGABLE_INTERFACES); mockVmDevice(false); } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/HotUnPlugDiskFromVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/HotUnPlugDiskFromVmCommandTest.java index 9bb57d6..38b9176 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/HotUnPlugDiskFromVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/HotUnPlugDiskFromVmCommandTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; @@ -11,6 +12,7 @@ import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.DiskInterface; import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.compat.Version; public class HotUnPlugDiskFromVmCommandTest extends HotPlugDiskToVmCommandTest { @@ -42,6 +44,8 @@ disk.setActive(true); doReturn(diskDao).when(command).getDiskDao(); when(diskDao.get(diskImageGuid)).thenReturn(disk); + when(osRepository.getDiskHotpluggableInterfaces(any(Integer.class), + any(Version.class))).thenReturn(DISK_HOTPLUGGABLE_INTERFACES); mockVmDevice(true); } } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/osinfo/OsRepository.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/osinfo/OsRepository.java index 1fa3e06..c82ddff 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/osinfo/OsRepository.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/osinfo/OsRepository.java @@ -125,6 +125,13 @@ /** * @param osId * @param version + * @return list of disk hotpluggable interfaces + */ + ArrayList<String> getDiskHotpluggableInterfaces(int osId, Version version); + + /** + * @param osId + * @param version * @return a specific sound device for the given os. */ String getSoundDevice(int osId, Version version); diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/osinfo/OsRepositoryImpl.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/osinfo/OsRepositoryImpl.java index 0507dc9..e3d1e4c 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/osinfo/OsRepositoryImpl.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/osinfo/OsRepositoryImpl.java @@ -13,6 +13,7 @@ import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.utils.Pair; +import org.ovirt.engine.core.compat.StringHelper; import org.ovirt.engine.core.compat.Version; /** @@ -149,6 +150,14 @@ public ArrayList<String> getNetworkDevices(int osId, Version version) { String devices = getValueByVersion(idToUnameLookup.get(osId), "devices.network", version); + return trimElements(devices.split(",")); + } + + @Override + public ArrayList<String> getDiskHotpluggableInterfaces(int osId, Version version) { + String devices = getValueByVersion(idToUnameLookup.get(osId), + "devices.disk.hotpluggableInterfaces", + version); return trimElements(devices.split(",")); } @@ -302,13 +311,17 @@ * * @param elements * vararg of string elements. - * @return new list where each value its whitespaces trimmed. + * @return new list where each value its whitespaces trimmed, and + * is not added null or empty values. */ - + @SuppressWarnings("deprecation") private ArrayList<String> trimElements(String... elements) { ArrayList<String> list = new ArrayList<String>(elements.length); for (String e : elements) { - list.add(e.trim()); + e = e.trim(); + if (!StringHelper.isNullOrEmpty(e)) { + list.add(e); + } } return list; } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/OsQueryParameters.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/OsQueryParameters.java index 66ea010..185d2ae 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/OsQueryParameters.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/OsQueryParameters.java @@ -46,6 +46,7 @@ GetMinimumOsRam, GetMaxOsRam, GetNetworkDevices, + GetDiskHotpluggableInterfaces, GetWindowsOss, GetUniqueOsNames, GetOsNames diff --git a/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/osinfo/OsRepositoryImplTest.java b/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/osinfo/OsRepositoryImplTest.java index 1f4a88e..0d49c53 100644 --- a/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/osinfo/OsRepositoryImplTest.java +++ b/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/osinfo/OsRepositoryImplTest.java @@ -15,6 +15,7 @@ private static MapBackedPreferences preferences; public static final String NETWORK_DEVICES = "e100,pv"; + public static final String DISK_HOTPLUGGABLE_INTERFACES = "VirtIO_SCSI, VirtIO"; public static final String PATH_TO_SYSPREP = "/path/to/sysprep"; public static final String SOME_PRODUCT_KEY = "some-product-key"; public static final String SOUND_DEVICE = "ac97"; @@ -27,6 +28,7 @@ preferences.node("/os/rhel7/family").put("value", "linux"); preferences.node("/os/rhel7/bus").put("value", "64"); preferences.node("/os/rhel7/devices/network").put("value", NETWORK_DEVICES); + preferences.node("/os/rhel7/devices/disk/hotpluggableInterfaces").put("value", DISK_HOTPLUGGABLE_INTERFACES); preferences.node("/os/rhel7/resources/minimum/ram").put("value", "1024"); preferences.node("/os/rhel7/resources/minimum/ram").put("value.3.1", "512"); preferences.node("/os/rhel7/resources/maximum/ram").put("value", "2048"); @@ -97,6 +99,15 @@ } @Test + public void testGetDiskHotpluggableInterfaces() throws Exception { + ArrayList<String> diskHotpluggableInterfaces = OsRepositoryImpl.INSTANCE.getDiskHotpluggableInterfaces(777, null); + assertTrue(diskHotpluggableInterfaces.size() == 2); + for (String diskHotpluggableInterface : DISK_HOTPLUGGABLE_INTERFACES.split(",")) { + assertTrue(diskHotpluggableInterfaces.contains(diskHotpluggableInterface.trim())); + } + } + + @Test public void testIsLinux() throws Exception { assertTrue(OsRepositoryImpl.INSTANCE.isLinux(777)); } diff --git a/packaging/conf/osinfo-defaults.properties b/packaging/conf/osinfo-defaults.properties index 830cd90..569275d 100644 --- a/packaging/conf/osinfo-defaults.properties +++ b/packaging/conf/osinfo-defaults.properties @@ -45,6 +45,9 @@ os.other.resources.minimum.numberOsCpus.value = 1 os.other.spiceSupport.value = true +os.other.devices.disk.hotpluggableInterfaces.value = VirtIO_SCSI, VirtIO +os.other.devices.disk.hotpluggableInterfaces.value.3.0 = + os.other.devices.audio.value = ich6 # See VmInterfaceType.java os.other.devices.network.value = rtl8139, e1000, pv -- To view, visit http://gerrit.ovirt.org/19601 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic2e4c792b607429daf077c11820b43f171d376b7 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Gustavo Frederico Temple Pedrosa <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
