Liron Ar has uploaded a new change for review. Change subject: core: avoid update disk dynamic data gathered from disk snapshot ......................................................................
core: avoid update disk dynamic data gathered from disk snapshot When a disk snapshot is plugged to the vm, a transient image is created on the host local storage to allow read/write to that plugged 'disk'. Therefore when the disk related stats are recieved for a VM that a disk snapshot is plugged to, they shouldn't be updated in the disc_image_dynamic table. Change-Id: Iaa25d040327b4cfe9b049fa2ec2638893190e8ef Signed-off-by: Liron Aravot <[email protected]> --- M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDynamicDAO.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDynamicDAODbFacadeImpl.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskImageDynamicDAOTest.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/FixturesTool.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDeviceDAOTest.java M backend/manager/modules/dal/src/test/resources/fixtures.xml M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java M packaging/dbscripts/disk_image_dynamic_sp.sql 9 files changed, 90 insertions(+), 26 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/88/28888/1 diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDynamicDAO.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDynamicDAO.java index b690392..01c78b9 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDynamicDAO.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDynamicDAO.java @@ -3,10 +3,11 @@ import java.util.Collection; import org.ovirt.engine.core.common.businessentities.DiskImageDynamic; +import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.compat.Guid; public interface DiskImageDynamicDAO extends GenericDao<DiskImageDynamic, Guid>, MassOperationsDao<DiskImageDynamic, Guid> { - public void updateAllDiskImageDynamicWithDiskId(Collection<DiskImageDynamic> diskImageDynamic); + public void updateAllDiskImageDynamicWithDiskIdByVmId(Collection<Pair<Guid, DiskImageDynamic>> diskImageDynamic); } diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDynamicDAODbFacadeImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDynamicDAODbFacadeImpl.java index f45b868..5cc81ce 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDynamicDAODbFacadeImpl.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskImageDynamicDAODbFacadeImpl.java @@ -5,6 +5,7 @@ import java.util.Collection; import org.ovirt.engine.core.common.businessentities.DiskImageDynamic; +import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.MapSqlParameterMapper; import org.springframework.jdbc.core.RowMapper; @@ -86,19 +87,22 @@ }; } - public MapSqlParameterMapper<DiskImageDynamic> getBatchImageGroupMapper() { - return new MapSqlParameterMapper<DiskImageDynamic>() { + public MapSqlParameterMapper<Pair<Guid, DiskImageDynamic>> getBatchImageGroupMapper() { + return new MapSqlParameterMapper<Pair<Guid, DiskImageDynamic>>() { @Override - public MapSqlParameterSource map(DiskImageDynamic entity) { + public MapSqlParameterSource map(Pair<Guid, DiskImageDynamic> entity) { + Guid vmId = entity.getFirst(); + DiskImageDynamic diskImageDynamic = entity.getSecond(); MapSqlParameterSource paramValue = new MapSqlParameterSource() - .addValue("image_group_id", entity.getId()) - .addValue("read_rate", entity.getread_rate()) - .addValue("write_rate", entity.getwrite_rate()) - .addValue("actual_size", entity.getactual_size()) - .addValue("read_latency_seconds", entity.getReadLatency()) - .addValue("write_latency_seconds", entity.getWriteLatency()) - .addValue("flush_latency_seconds", entity.getFlushLatency()); + .addValue("vm_id", vmId) + .addValue("image_group_id", diskImageDynamic.getId()) + .addValue("read_rate", diskImageDynamic.getread_rate()) + .addValue("write_rate", diskImageDynamic.getwrite_rate()) + .addValue("actual_size", diskImageDynamic.getactual_size()) + .addValue("read_latency_seconds", diskImageDynamic.getReadLatency()) + .addValue("write_latency_seconds", diskImageDynamic.getWriteLatency()) + .addValue("flush_latency_seconds", diskImageDynamic.getFlushLatency()); return paramValue; } @@ -106,7 +110,8 @@ } @Override - public void updateAllDiskImageDynamicWithDiskId(Collection<DiskImageDynamic> diskImageDynamic) { - updateAllInBatch("Updatedisk_image_dynamic_by_disk_id", diskImageDynamic, getBatchImageGroupMapper()); + public void updateAllDiskImageDynamicWithDiskIdByVmId(Collection<Pair<Guid, DiskImageDynamic>> diskImageDynamicForVm) { + getCallsHandler().executeStoredProcAsBatch("Updatedisk_image_dynamic_by_disk_id_and_vm_id", + diskImageDynamicForVm, getBatchImageGroupMapper()); } } diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java index 71936e6..bd0e372 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java @@ -190,7 +190,7 @@ * The result to check */ private static void assertFullGetAllForVMResult(List<Disk> disks) { - assertEquals("VM should have five disks", 5, disks.size()); + assertEquals("VM should have five disks", 6, disks.size()); } /** diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskImageDynamicDAOTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskImageDynamicDAOTest.java index 81d82bc..af48a57 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskImageDynamicDAOTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskImageDynamicDAOTest.java @@ -11,14 +11,17 @@ import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.DiskImageDynamic; import org.ovirt.engine.core.common.businessentities.DiskInterface; +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.businessentities.VolumeType; +import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.compat.Guid; public class DiskImageDynamicDAOTest extends BaseDAOTestCase{ private static final Guid EXISTING_IMAGE_ID = new Guid("42058975-3d5e-484a-80c1-01c31207f578"); - private static final int TOTAL_DYNAMIC_DISK_IMAGES = 4; + private static final int TOTAL_DYNAMIC_DISK_IMAGES = 5; private static final Guid EXISTING_IMAGE_DISK_TEMPLATE = new Guid("42058975-3d5e-484a-80c1-01c31207f578"); @@ -154,15 +157,29 @@ @Test public void testUpdateWithDiskId() throws Exception { - Guid imageId = new Guid("52058975-3d5e-484a-80c1-01c31207f578"); - Guid imageGroupId = new Guid("1b26a52b-b60f-44cb-9f46-3ef333b04a34"); + Guid imageId = FixturesTool.IMAGE_ID_2; + Guid imageGroupId = FixturesTool.IMAGE_GROUP_ID_2; + DiskImageDynamic existingDynamic2 = dao.get(imageId); + assertFalse(existingDynamic2.getread_rate().equals(120)); + + VmDevice device = dbFacade.getVmDeviceDao().get(new VmDeviceId(imageGroupId, FixturesTool.VM_RHEL5_POOL_57)); + assertNull(device.getSnapshotId()); + existingDynamic2.setId(imageGroupId); existingDynamic2.setread_rate(120); - existingDynamic2.setReadLatency(0.00001d); - dao.updateAllDiskImageDynamicWithDiskId(Arrays.asList(new DiskImageDynamic[] { existingDynamic2 })); + + dao.updateAllDiskImageDynamicWithDiskIdByVmId(Arrays.<Pair<Guid, DiskImageDynamic>>asList(new Pair(FixturesTool.VM_RHEL5_POOL_57, existingDynamic2))); + existingDynamic2.setId(imageId); assertEquals(existingDynamic2, dao.get(imageId)); + + device.setSnapshotId(FixturesTool.EXISTING_SNAPSHOT_ID); + dbFacade.getVmDeviceDao().update(device); + + existingDynamic2.setread_rate(150); + dao.updateAllDiskImageDynamicWithDiskIdByVmId(Arrays.<Pair<Guid, DiskImageDynamic>>asList(new Pair(FixturesTool.VM_RHEL5_POOL_57, existingDynamic2))); + assertFalse(existingDynamic2.getread_rate().equals(dao.get(imageId).getread_rate())); } } diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/FixturesTool.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/FixturesTool.java index ae47bd6..41a9844 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/FixturesTool.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/FixturesTool.java @@ -302,6 +302,14 @@ protected static final Guid IMAGE_ID = new Guid("42058975-3d5e-484a-80c1-01c31207f578"); /** + * Predefined image with the following properties : + * <ul> + * <li>disk id: 1b26a52b-b60f-44cb-9f46-3ef333b04a38</li> + * </ul> + */ + protected static final Guid IMAGE_ID_2 = new Guid("c9a559d9-8666-40d1-9967-759502b19f0c"); + + /** * Predefined disk for testing. */ protected static final Guid DISK_ID = new Guid("1b26a52b-b60f-44cb-9f46-3ef333b04a34"); @@ -327,6 +335,8 @@ */ protected static final Guid IMAGE_GROUP_ID = new Guid("1b26a52b-b60f-44cb-9f46-3ef333b04a35"); + protected static final Guid IMAGE_GROUP_ID_2 = new Guid("1b26a52b-b60f-44cb-9f46-3ef333b04a38"); + /** * Predefined floating disk for testing. */ diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDeviceDAOTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDeviceDAOTest.java index 241741c..dc8c98b 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDeviceDAOTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDeviceDAOTest.java @@ -25,8 +25,8 @@ private static final Guid EXISTING_VM_ID = new Guid("77296e00-0cad-4e5a-9299-008a7b6f4355"); private static final Guid EXISTING_DEVICE_ID = new Guid("e14ed6f0-3b12-11e1-b614-63d00126418d"); - private static final int TOTAL_DEVICES = 11; - private static final int TOTAL_DEVICES_FOR_EXISTING_VM = 4; + private static final int TOTAL_DEVICES = 12; + private static final int TOTAL_DEVICES_FOR_EXISTING_VM = 5; @Override protected VmDeviceId generateNonExistingId() { diff --git a/backend/manager/modules/dal/src/test/resources/fixtures.xml b/backend/manager/modules/dal/src/test/resources/fixtures.xml index d431603..3b0f660 100644 --- a/backend/manager/modules/dal/src/test/resources/fixtures.xml +++ b/backend/manager/modules/dal/src/test/resources/fixtures.xml @@ -4529,6 +4529,13 @@ <null /> </row> <row> + <value>c9a559d9-8666-40d1-9967-759502b19f0c</value> + <value>0</value> + <value>0</value> + <value>1073741824</value> + <null /> + </row> + <row> <value>42058975-3d5e-484a-80c1-01c31207f579</value> <value>0</value> <value>0</value> @@ -5523,6 +5530,21 @@ <null /> </row> <row> + <value>1b26a52b-b60f-44cb-9f46-3ef333b04a38</value> + <value>77296e00-0cad-4e5a-9299-008a7b6f4355</value> + <value>disk</value> + <value>disk</value> + <value>sample</value> + <value>1</value> + <value></value> + <value>true</value> + <value>false</value> + <value>false</value> + <value>alias</value> + <value>{ "prop1" : "true", "prop2" : "123456" }</value> + <null /> + </row> + <row> <value>52058975-3d5e-484a-80c1-01c31207f578</value> <value>1b85420c-b84c-4f29-997e-0eb674b40b79</value> <value>disk</value> diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java index b76e156..c53b72b 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java @@ -2,6 +2,7 @@ import java.lang.reflect.Field; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.HashMap; @@ -57,6 +58,7 @@ import org.ovirt.engine.core.common.businessentities.network.VmNetworkStatistics; 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.common.utils.VmDeviceCommonUtils; import org.ovirt.engine.core.common.utils.VmDeviceType; import org.ovirt.engine.core.common.vdscommands.DestroyVmVDSCommandParameters; @@ -96,7 +98,7 @@ private final Map<Guid, VmDynamic> _vmDynamicToSave = new HashMap<>(); private final Map<Guid, VmStatistics> _vmStatisticsToSave = new HashMap<>(); private final Map<Guid, List<VmNetworkInterface>> _vmInterfaceStatisticsToSave = new HashMap<>(); - private final Set<DiskImageDynamic> _vmDiskImageDynamicToSave = new HashSet<>(); + private final Collection<Pair<Guid, DiskImageDynamic>> _vmDiskImageDynamicToSave = new LinkedList<>(); private final List<LUNs> vmLunDisksToSave = new ArrayList<>(); private final Map<VmDeviceId, VmDevice> vmDeviceToSave = new HashMap<>(); private final List<VmDevice> newVmDevices = new ArrayList<>(); @@ -192,7 +194,7 @@ getDbFacade().getVmNetworkStatisticsDao().updateAllInBatch(allVmInterfaceStatistics); - getDbFacade().getDiskImageDynamicDao().updateAllDiskImageDynamicWithDiskId(_vmDiskImageDynamicToSave); + getDbFacade().getDiskImageDynamicDao().updateAllDiskImageDynamicWithDiskIdByVmId(_vmDiskImageDynamicToSave); getDbFacade().getLunDao().updateAllInBatch(vmLunDisksToSave); saveVmDevicesToDb(); saveVmJobsToDb(); @@ -2039,7 +2041,11 @@ addVmStatisticsToList(vmToUpdate.getStatisticsData()); updateInterfaceStatistics(vmToUpdate, vmStatistics); - _vmDiskImageDynamicToSave.addAll(_runningVms.get(vmToUpdate.getId()).getVmDynamic().getDisks()); + Guid vmId = vmToUpdate.getId(); + Collection<DiskImageDynamic> vmDisksDynamic = _runningVms.get(vmId).getVmDynamic().getDisks(); + for (DiskImageDynamic diskImageDynamic : vmDisksDynamic) { + _vmDiskImageDynamicToSave.add(new Pair<>(vmId, diskImageDynamic)); + } } } diff --git a/packaging/dbscripts/disk_image_dynamic_sp.sql b/packaging/dbscripts/disk_image_dynamic_sp.sql index e94880c..817e9b1 100644 --- a/packaging/dbscripts/disk_image_dynamic_sp.sql +++ b/packaging/dbscripts/disk_image_dynamic_sp.sql @@ -46,7 +46,8 @@ LANGUAGE plpgsql; -Create or replace FUNCTION Updatedisk_image_dynamic_by_disk_id(v_image_group_id UUID, +Create or replace FUNCTION Updatedisk_image_dynamic_by_disk_id_and_vm_id(v_image_group_id UUID, + v_vm_id UUID, v_read_rate INTEGER , v_write_rate INTEGER , v_actual_size BIGINT , @@ -62,7 +63,9 @@ SET read_rate = v_read_rate,write_rate = v_write_rate,actual_size = v_actual_size,read_latency_seconds = v_read_latency_seconds,write_latency_seconds = v_write_latency_seconds,flush_latency_seconds = v_flush_latency_seconds, _update_date = LOCALTIMESTAMP WHERE image_id in (SELECT distinct image_guid FROM images - WHERE image_group_id = v_image_group_id and active = true); + WHERE image_group_id = v_image_group_id and active = true) + AND EXISTS (SELECT 1 FROM vm_device vmd + WHERE vmd.vm_id = v_vm_id AND vmd.device_id = v_image_group_id AND vmd.snapshot_id is NULL); END; $procedure$ LANGUAGE plpgsql; -- To view, visit http://gerrit.ovirt.org/28888 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iaa25d040327b4cfe9b049fa2ec2638893190e8ef Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Ar <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
