Allon Mureinik has posted comments on this change. Change subject: core: OvfDataUpdater - removal calls to updateVm/TemplateInSpm ......................................................................
Patch Set 16: I would prefer that you didn't submit this (12 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromScratchCommand.java Line 114: Line 115: VmHandler.LockVm(getParameters().getVmStaticData().getId()); Line 116: } else { Line 117: // if no disks send update vm here Line 118: getVmStaticDao().incrementDbGeneration(getVm().getId()); shouldn't you also increment the DB version if there /ARE/ disks in the VM? Line 119: } Line 120: Line 121: return ret; Line 122: } .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmCommand.java Line 407: OvfDataUpdater.getInstance().buildMetadataDictionaryForVm(getVm(), metaDictionary); Line 408: VmHandler.updateDisksFromDb(getVm()); Line 409: return OvfDataUpdater.getInstance().executeUpdateVmInSpmCommand(getVm().getstorage_pool_id(), Line 410: metaDictionary, Line 411: getParameters().getStorageDomainId()); can you explain this change? Line 412: } Line 413: Line 414: @Override Line 415: protected void endSuccessfully() { .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmTemplateCommand.java Line 141: OvfDataUpdater.getInstance().buildMetadataDictionaryForTemplate(getVmTemplate(), metaDictionary); Line 142: OvfDataUpdater.getInstance().executeUpdateVmInSpmCommand(getVmTemplate().getstorage_pool_id().getValue(), Line 143: metaDictionary, Line 144: getParameters().getStorageDomainId()); Line 145: } this one too? Line 146: Line 147: @Override Line 148: public Map<String, String> getJobMessageProperties() { Line 149: if (jobProperties == null) { .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java Line 256: log.warn("MoveOrCopyTemplateCommand::EndMoveOrCopyCommand: VmTemplate is null, not performing full EndAction"); Line 257: } Line 258: } Line 259: Line 260: protected void updateTemplateInSpm() { please rename the method, as it no longer updates the OVF in the SPM. Line 261: VmTemplate vmt = getVmTemplate(); Line 262: getVmStaticDAO().incrementDbGeneration(vmt.getId()); Line 263: } Line 264: .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveVmCommand.java Line 176: setSucceeded(true); Line 177: Line 178: } Line 179: Line 180: protected void updateVmImSpm() { please rename the method, as it changed it's function Line 181: getVmStaticDAO().incrementDbGeneration(getVm().getId()); Line 182: } Line 183: Line 184: @Override .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java Line 80: } Line 81: Line 82: @Override Line 83: protected void endSuccessfully() { Line 84: getVmStaticDAO().incrementDbGeneration(getVm().getId()); I assume the entire method is run in a single transaction, right? Line 85: endActionOnDisks(); Line 86: Line 87: if (getVm() != null) { Line 88: VmHandler.unlockVm(getVm(), getCompensationContext()); .................................................... File backend/manager/modules/dal/src/main/jdbc-resources/engine-daos.properties Line 54: GlusterVolumeDao=org.ovirt.engine.core.dao.gluster.GlusterVolumeDaoDbFacadeImpl Line 55: GlusterBrickDao=org.ovirt.engine.core.dao.gluster.GlusterBrickDaoDbFacadeImpl Line 56: GlusterOptionDao=org.ovirt.engine.core.dao.gluster.GlusterOptionDaoDbFacadeImpl Line 57: ImageStorageDomainMapDao=org.ovirt.engine.core.dao.ImageStorageDomainMapDaoDbFacadeImpl Line 58: VmAndTemplatesGenerationsDAO=org.ovirt.engine.core.dao.VmAndTemplatesGenerationsDbFacadeImpl This should be a part of the previous patch. .................................................... File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/FixturesTool.java Line 4: Line 5: /** Line 6: * A utility class for DAO testing which maps the fixtures entities to constants, for easy testing. Line 7: */ Line 8: public class FixturesTool { All the change to this file should be a part of the previous patch. Line 9: /** Line 10: * Predefined NFS storage pool. Line 11: */ Line 12: protected static final Guid STORAGE_POOL_NFS = new Guid("72b9e200-f48b-4687-83f2-62828f249a47"); .................................................... File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmStaticDAOTest.java Line 1: /** Line 2: * All the change to this file should be a part of the previous patch. Line 3: */ Line 4: package org.ovirt.engine.core.dao; Line 5: Line 6: import static org.junit.Assert.assertEquals; .................................................... File backend/manager/modules/dal/src/test/resources/fixtures.xml Line 847: <value>2010-12-01 09:52:57</value> Line 848: <value>4</value> Line 849: </row> Line 850: </table> Line 851: All the change to this file should be a part of the previous patch. Line 852: <table name="vm_ovf_generations"> Line 853: <column>vm_guid</column> Line 854: <column>storage_pool_id</column> Line 855: <column>ovf_generation</column> Line 896: <row> Line 897: <value>77296e00-0cad-4e5a-9299-008a7b6f4360</value> Line 898: <value>72b9e200-f48b-4687-83f2-62828f249a47</value> Line 899: <value>1</value> Line 900: </row> replace tabs with spaces Line 901: </table> Line 902: Line 903: <table name="vm_static"> Line 904: <column>vm_guid</column> .................................................... Commit Message Line 3: AuthorDate: 2012-12-02 09:59:28 +0200 Line 4: Commit: Liron Aravot <lara...@redhat.com> Line 5: CommitDate: 2012-12-02 13:38:26 +0200 Line 6: Line 7: core: OvfDataUpdater - removal calls to updateVm/TemplateInSpm s/removal/remove/ Also, missing an explanation why and how this is done. Line 8: Line 9: Change-Id: Iedebceb9809dc0b11c0bbe8a2d4af63b0d848df1 -- To view, visit http://gerrit.ovirt.org/9340 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iedebceb9809dc0b11c0bbe8a2d4af63b0d848df1 Gerrit-PatchSet: 16 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@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: Vered Volansky <vvola...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches