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

Reply via email to