Liron Aravot has posted comments on this change. Change subject: core: introducing OvfAutoUpdate ......................................................................
Patch Set 20: (29 inline comments) .................................................... File backend/manager/dbscripts/storages_sp.sql Line 97: WHERE vm_id IN ( Line 98: SELECT vm_guid Line 99: FROM vms Line 100: WHERE storage_pool_id = v_id); Line 101: delete FROM vm_static where vm_guid in (select vm_guid from vms where storage_pool_id = v_id); this stored procedure is not related to this patch, a blank line was removed here by mistake. it can be fixed in further patch. Line 102: -- Get (and keep) a shared lock with "right to upgrade to exclusive" Line 103: -- in order to force locking parent before children Line 104: select id INTO v_val FROM storage_pool WHERE id = v_id FOR UPDATE; Line 105: Line 100: WHERE storage_pool_id = v_id); Line 101: delete FROM vm_static where vm_guid in (select vm_guid from vms where storage_pool_id = v_id); Line 102: -- Get (and keep) a shared lock with "right to upgrade to exclusive" Line 103: -- in order to force locking parent before children Line 104: select id INTO v_val FROM storage_pool WHERE id = v_id FOR UPDATE; this stored procedure is not related to this patch, a blank line was removed here by mistake. it can be fixed in further patch. Line 105: Line 106: DELETE FROM storage_pool Line 107: WHERE id = v_id; Line 108: Line 106: DELETE FROM storage_pool Line 107: WHERE id = v_id; Line 108: Line 109: -- delete StoragePool permissions -- Line 110: DELETE FROM permissions where object_id = v_id; this stored procedure is not related to this patch, a blank line was removed here by mistake. it can be fixed in further patch. Line 111: Line 112: END; $procedure$ Line 113: LANGUAGE plpgsql; Line 114: .................................................... File backend/manager/dbscripts/upgrade/03_02_0020_add_vm_generation_columns.sql Line 1: SELECT fn_db_add_column('vm_static', 'db_generation', 'BIGINT default 1'); Line 2: Line 3: -- not added as foreign key so that when vm is removed, its record in vm_ovf_generations record will stay sure, it will be renamed during rebase.. Line 4: CREATE TABLE vm_ovf_generations Line 5: ( Line 6: vm_guid UUID PRIMARY KEY, Line 7: storage_pool_id UUID REFERENCES storage_pool(id) ON DELETE CASCADE, Line 7: storage_pool_id UUID REFERENCES storage_pool(id) ON DELETE CASCADE, Line 8: ovf_generation BIGINT DEFAULT 1 Line 9: ); Line 10: Line 11: INSERT INTo vm_ovf_generations Done Line 12: (SELECT vm.vm_guid, sp.id Line 13: FROM vm_static vm ,storage_pool sp, vds_groups vg Line 14: WHERE vg.storage_pool_id = sp.id AND vm.vds_group_id = vg.vds_group_id); Line 15: Line 14: WHERE vg.storage_pool_id = sp.id AND vm.vds_group_id = vg.vds_group_id); Line 15: Line 16: -- Only pre-existing vms should have ovf_generation set to 1, so after we added Line 17: -- the pre existing vms, the default should be 0. Line 18: SELECT fn_db_change_column_type ('vm_ovf_generations', 'ovf_generation', 'BIGINT DEFAULT 1','BIGINT DEFAULT 0'); Done. Line 19: Line 20: CREATE INDEX IDX_vm_ovf_generations_vm_guid ON vm_ovf_generations(vm_guid); .................................................... File backend/manager/dbscripts/vms_sp.sql Line 102: Create or replace FUNCTION DeleteOvfGenerations(v_vms_ids VARCHAR(5000)) Line 103: RETURNS VOID Line 104: AS $procedure$ Line 105: BEGIN Line 106: DELETE FROM vm_ovf_generations WHERE vm_guid IN (SELECT ID from fnSplitterUuid(v_vms_ids)); Done Line 107: END; $procedure$ Line 108: LANGUAGE plpgsql; Line 109: Line 110: Line 425: AS $procedure$ Line 426: BEGIN Line 427: INSERT INTO vm_static(description, mem_size_mb, os, vds_group_id, vm_guid, VM_NAME, vmt_guid,domain,creation_date,num_of_monitors,allow_console_reconnect,is_initialized,is_auto_suspend,num_of_sockets,cpu_per_socket,usb_policy, time_zone,auto_startup,is_stateless,dedicated_vm_for_vds, fail_back, default_boot_sequence, vm_type, nice_level, default_display_type, priority,iso_path,origin,initrd_url,kernel_url,kernel_params,migration_support,predefined_properties,userdefined_properties,min_allocated_mem, entity_type, quota_id, cpu_pinning, is_smartcard_enabled,is_delete_protected) Line 428: VALUES(v_description, v_mem_size_mb, v_os, v_vds_group_id, v_vm_guid, v_vm_name, v_vmt_guid, v_domain, v_creation_date, v_num_of_monitors, v_allow_console_reconnect, v_is_initialized, v_is_auto_suspend, v_num_of_sockets, v_cpu_per_socket, v_usb_policy, v_time_zone, v_auto_startup,v_is_stateless,v_dedicated_vm_for_vds,v_fail_back, v_default_boot_sequence, v_vm_type, v_nice_level, v_default_display_type, v_priority,v_iso_path,v_origin,v_initrd_url,v_kernel_url,v_kernel_params,v_migration_support,v_predefined_properties,v_userdefined_properties,v_min_allocated_mem, 'VM', v_quota_id, v_cpu_pinning, v_is_smartcard_enabled,v_is_delete_protected); Line 429: INSERT INTO vm_ovf_generations(vm_guid, storage_pool_id) VALUES (v_vm_guid, (select storage_pool_id from vds_groups vg where vg.vds_group_id = v_vds_group_id)); Done Line 430: END; $procedure$ Line 431: LANGUAGE plpgsql; Line 432: Line 433: .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OvfDataUpdater.java Line 297: Map<Guid, KeyValuePairCompat<String, List<Guid>>> metaDictionary) { Line 298: List<DiskImage> allTemplateImages = template.getDiskList(); Line 299: String templateMeta = generateVmTemplateMetadata(template, allTemplateImages); Line 300: metaDictionary.put(template.getId(), new KeyValuePairCompat<String, List<Guid>>( Line 301: templateMeta, LinqUtils.foreach(allTemplateImages, new Function<DiskImage, Guid>() { a new list with the disks guid only should be created here, that's an extracted code..i prefer to leave it like this at the moment. Line 302: @Override Line 303: public Guid eval(DiskImage diskImage) { Line 304: return diskImage.getId().getValue(); Line 305: } .................................................... File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/OvfDataUpdaterTest.java Line 2: Line 3: import static org.junit.Assert.assertEquals; Line 4: import static org.junit.Assert.assertFalse; Line 5: import static org.junit.Assert.assertTrue; Line 6: import static org.mockito.Matchers.*; Done Line 7: import static org.mockito.Mockito.doAnswer; Line 8: import static org.mockito.Mockito.doCallRealMethod; Line 9: import static org.mockito.Mockito.doNothing; Line 10: import static org.mockito.Mockito.doReturn; Line 259: List<storage_pool> pools = new LinkedList<storage_pool>(); Line 260: pools.add(pool1); Line 261: pools.add(pool2); Line 262: pools.add(pool3); Line 263: return pools; Done Line 264: } Line 265: Line 266: private VM createVm(Guid id, VMStatus status) { Line 267: VM vm = new VM(); .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBase.java Line 229: this.deleteProtected = deleteProtected; Line 230: setQuotaId(quotaId); Line 231: } Line 232: Line 233: public Long getDbGeneration() { Done Line 234: return dbGeneration; Line 235: } Line 236: Line 237: public void setDbGeneration(Long db_generation) { Line 233: public Long getDbGeneration() { Line 234: return dbGeneration; Line 235: } Line 236: Line 237: public void setDbGeneration(Long db_generation) { Done Line 238: this.dbGeneration = db_generation; Line 239: } Line 240: Line 241: public List<VmNetworkInterface> getInterfaces() { .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java Line 946: public void setNiceLevel(int value) { Line 947: this.vmStatic.setnice_level(value); Line 948: } Line 949: Line 950: public void setDbGeneration(Long value) { good catch, leftover from when it could have been null. Line 951: this.vmStatic.setDbGeneration(value); Line 952: } Line 953: Line 954: public Long getDbGeneration() { Line 950: public void setDbGeneration(Long value) { Line 951: this.vmStatic.setDbGeneration(value); Line 952: } Line 953: Line 954: public Long getDbGeneration() { same answer Line 955: return vmStatic.getDbGeneration(); Line 956: } Line 957: Line 958: public MigrationSupport getMigrationSupport() { .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/ConfigurationValues.java Line 61: VdsFenceOptionTypes, Line 62: ServerCPUList, Line 63: SupportedClusterLevels(ConfigAuthType.User), Line 64: OvfUpdateIntervalInMinutes, Line 65: OvfItemsCountPerUpdate, why shouldn't they be? Line 66: ProductRPMVersion(ConfigAuthType.User), Line 67: RhevhLocalFSPath, Line 68: CustomPublicConfig_AppsWebSite(ConfigAuthType.User), Line 69: DocsURL(ConfigAuthType.User), .................................................... File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java Line 23: */ Line 24: @SuppressWarnings("synthetic-access") Line 25: public class StoragePoolDAODbFacadeImpl extends BaseDAODbFacade implements StoragePoolDAO { Line 26: Line 27: private static final class StoragePoolRawMapper implements ParameterizedRowMapper<storage_pool> { I changed it as I already changed this class..that's a minor and correct change, i'd prefer to leave it. Line 28: @Override Line 29: public storage_pool mapRow(ResultSet rs, int rowNum) Line 30: throws SQLException { Line 31: storage_pool entity = new storage_pool(); .................................................... File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDAOTest.java Line 108: * Asserts the given VM is the expected one Line 109: */ Line 110: private void assertGetResult(VM result) { Line 111: assertNotNull(result); Line 112: assertEquals("Vm db generation wasn't loaded as expected", new Long(1), result.getDbGeneration()); Done Line 113: assertEquals(result, existingVm); Line 114: } Line 115: Line 116: /** Line 351: assertNotNull(result); Line 352: assertFalse(result.isEmpty()); Line 353: assertEquals(VM_COUNT, result.size()); Line 354: for (VM vm : result) { Line 355: assertEquals("Vm db generation wasn't loaded as expected", new Long(1), vm.getDbGeneration()); Done Line 356: } Line 357: } .................................................... File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmTemplateDAOTest.java Line 271: Line 272: private static void assertGetResult(VmTemplate result) { Line 273: assertNotNull(result); Line 274: assertEquals(EXISTING_TEMPLATE_ID, result.getId()); Line 275: assertEquals("Template generation wasn't loaded as expected", new Long(1), result.getDbGeneration()); Done Line 276: } Line 277: Line 278: private static void assertGetAllResult(List<VmTemplate> result) { Line 279: assertNotNull(result); Line 278: private static void assertGetAllResult(List<VmTemplate> result) { Line 279: assertNotNull(result); Line 280: assertFalse(result.isEmpty()); Line 281: for (VmTemplate template : result){ Line 282: assertEquals("Template generation wasn't loaded as expected", new Long(1), template.getDbGeneration()); Done Line 283: } Line 284: } .................................................... File backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ovf/OvfVmWriterTest.java Line 4: Line 5: import org.junit.Before; Line 6: import org.junit.ClassRule; Line 7: import org.junit.Test; Line 8: import static org.junit.Assert.*; Done Line 9: import org.ovirt.engine.core.common.businessentities.DiskImage; Line 10: import org.ovirt.engine.core.common.businessentities.OriginType; Line 11: import org.ovirt.engine.core.common.businessentities.VM; Line 12: import org.ovirt.engine.core.common.businessentities.VmNetworkInterface; Line 14: import org.ovirt.engine.core.common.config.ConfigValues; Line 15: import org.ovirt.engine.core.compat.Guid; Line 16: import org.ovirt.engine.core.utils.MockConfigRule; Line 17: Line 18: public class OvfVmWriterTest { Done Line 19: Line 20: @ClassRule Line 21: public static MockConfigRule mockConfigRule = Line 22: new MockConfigRule(MockConfigRule.mockConfig(ConfigValues.VdcVersion, "1.0.0.0")); Line 20: @ClassRule Line 21: public static MockConfigRule mockConfigRule = Line 22: new MockConfigRule(MockConfigRule.mockConfig(ConfigValues.VdcVersion, "1.0.0.0")); Line 23: Line 24: OvfManager manager; Done Line 25: Line 26: @Before Line 27: public void setUp() throws Exception{ Line 28: manager = new OvfManager(); Line 24: OvfManager manager; Line 25: Line 26: @Before Line 27: public void setUp() throws Exception{ Line 28: manager = new OvfManager(); I prefer to init what i need in each test Line 29: } Line 30: Line 31: @Test Line 32: public void testVmOvfCreation() throws Exception{ Line 35: assertNotNull(xml); Line 36: final VM newVm = new VM(); Line 37: manager.ImportVm(xml, newVm, new ArrayList<DiskImage>(), new ArrayList<VmNetworkInterface>()); Line 38: assertEquals(vm, newVm); Line 39: assertEquals(newVm.getDbGeneration(), vm.getDbGeneration()); nop, if i edited VM description from "a" to "b" and then edited again from "b" to "a", db generation will be different but the equals should return true. Line 40: } Line 41: Line 42: @Test Line 43: public void testVmOvfImportWithoutDbGeneration() throws Exception{ Line 48: assertTrue(xml.contains("Generation")); Line 49: String replacedXml = xml.replaceAll("Generation", "test_replaced"); Line 50: manager.ImportVm(replacedXml, newVm, new ArrayList<DiskImage>(), new ArrayList<VmNetworkInterface>()); Line 51: assertEquals(vm, newVm); Line 52: assertEquals(new Long(1), newVm.getDbGeneration()); Done Line 53: } Line 54: Line 55: @Test Line 56: public void testTemplateOvfCreation() throws Exception { Line 71: String replacedXml = xml.replaceAll("Generation", "test_replaced"); Line 72: final VmTemplate newtemplate = new VmTemplate(); Line 73: manager.ImportTemplate(replacedXml,newtemplate, new ArrayList<DiskImage>(), new ArrayList<VmNetworkInterface>()); Line 74: assertEquals(template, newtemplate); Line 75: assertEquals(new Long(1), newtemplate.getDbGeneration()); Done Line 76: } Line 77: Line 78: private VM createVM() { Line 79: VM vm = new VM(); Line 84: vm.setDbGeneration(2L); Line 85: return vm; Line 86: } Line 87: Line 88: private VmTemplate createVmTemplate() { Done Line 89: VmTemplate template = new VmTemplate(); Line 90: template.setname("test-template"); Line 91: template.setorigin(OriginType.OVIRT); Line 92: template.setId(new Guid()); -- To view, visit http://gerrit.ovirt.org/9328 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9b5132300fb1f1fd94f771cab15efe5246dbeca8 Gerrit-PatchSet: 20 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: liron aravot <liron.ara...@gmail.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches